From e71f22884d3d46455c0eba5850f1c87ca0ff6ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Sun, 22 Dec 2024 17:15:39 +0100 Subject: [PATCH 1/4] compat/mingw: handle WSA errors in strerror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We map WSAGetLastError() errors to errno errors in winsock_error_to_errno(), but the MSVC strerror() implementation only produces "Unknown error" for most of them. Produce some more meaningful error messages in these cases. Our builds for ARM64 link against the newer UCRT strerror() that does know these errors, so we won't change the strerror() used there. The wording of the messages is copied from glibc strerror() messages. Reported-by: M Hickford Signed-off-by: Matthias Aßhauer Signed-off-by: Johannes Schindelin --- Makefile | 1 + compat/mingw.c | 85 ++++++++++++++++++++++++++++++++++++++++++ compat/mingw.h | 5 +++ t/meson.build | 1 + t/unit-tests/u-mingw.c | 72 +++++++++++++++++++++++++++++++++++ 5 files changed, 164 insertions(+) create mode 100644 t/unit-tests/u-mingw.c diff --git a/Makefile b/Makefile index ab42aaf126936e..924ede41850467 100644 --- a/Makefile +++ b/Makefile @@ -1351,6 +1351,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/% CLAR_TEST_SUITES += u-ctype CLAR_TEST_SUITES += u-strvec +CLAR_TEST_SUITES += u-mingw CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X) CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES)) CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o diff --git a/compat/mingw.c b/compat/mingw.c index b1bd925c701ed0..2373ddfd4a0f6e 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2673,6 +2673,91 @@ static inline int winsock_return(int ret) #define WINSOCK_RETURN(x) do { return winsock_return(x); } while (0) +#undef strerror +char *mingw_strerror(int errnum) +{ + static char buf[41] =""; + switch (errnum) { + case EWOULDBLOCK: + xsnprintf(buf, 41, "%s", "Operation would block"); + break; + case EINPROGRESS: + xsnprintf(buf, 41, "%s", "Operation now in progress"); + break; + case EALREADY: + xsnprintf(buf, 41, "%s", "Operation already in progress"); + break; + case ENOTSOCK: + xsnprintf(buf, 41, "%s", "Socket operation on non-socket"); + break; + case EDESTADDRREQ: + xsnprintf(buf, 41, "%s", "Destination address required"); + break; + case EMSGSIZE: + xsnprintf(buf, 41, "%s", "Message too long"); + break; + case EPROTOTYPE: + xsnprintf(buf, 41, "%s", "Protocol wrong type for socket"); + break; + case ENOPROTOOPT: + xsnprintf(buf, 41, "%s", "Protocol not available"); + break; + case EPROTONOSUPPORT: + xsnprintf(buf, 41, "%s", "Protocol not supported"); + break; + case EOPNOTSUPP: + xsnprintf(buf, 41, "%s", "Operation not supported"); + break; + case EAFNOSUPPORT: + xsnprintf(buf, 41, "%s", "Address family not supported by protocol"); + break; + case EADDRINUSE: + xsnprintf(buf, 41, "%s", "Address already in use"); + break; + case EADDRNOTAVAIL: + xsnprintf(buf, 41, "%s", "Cannot assign requested address"); + break; + case ENETDOWN: + xsnprintf(buf, 41, "%s", "Network is down"); + break; + case ENETUNREACH: + xsnprintf(buf, 41, "%s", "Network is unreachable"); + break; + case ENETRESET: + xsnprintf(buf, 41, "%s", "Network dropped connection on reset"); + break; + case ECONNABORTED: + xsnprintf(buf, 41, "%s", "Software caused connection abort"); + break; + case ECONNRESET: + xsnprintf(buf, 41, "%s", "Connection reset by peer"); + break; + case ENOBUFS: + xsnprintf(buf, 41, "%s", "No buffer space available"); + break; + case EISCONN: + xsnprintf(buf, 41, "%s", "Transport endpoint is already connected"); + break; + case ENOTCONN: + xsnprintf(buf, 41, "%s", "Transport endpoint is not connected"); + break; + case ETIMEDOUT: + xsnprintf(buf, 41, "%s", "Connection timed out"); + break; + case ECONNREFUSED: + xsnprintf(buf, 41, "%s", "Connection refused"); + break; + case ELOOP: + xsnprintf(buf, 41, "%s", "Too many levels of symbolic links"); + break; + case EHOSTUNREACH: + xsnprintf(buf, 41, "%s", "No route to host"); + break; + default: return strerror(errnum); + } + return buf; +} + #undef gethostname int mingw_gethostname(char *name, int namelen) { diff --git a/compat/mingw.h b/compat/mingw.h index 5c5066a668422a..b6f00b539aef9c 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -314,6 +314,11 @@ int mingw_socket(int domain, int type, int protocol); int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz); #define connect mingw_connect +char *mingw_strerror(int errnum); +#ifndef _UCRT +#define strerror mingw_strerror +#endif + int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz); #define bind mingw_bind diff --git a/t/meson.build b/t/meson.build index fad6a3a7fbeb26..1b91eb2d1f28de 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1,5 +1,6 @@ clar_test_suites = [ 'unit-tests/u-ctype.c', + 'unit-tests/u-mingw.c', 'unit-tests/u-strvec.c', ] diff --git a/t/unit-tests/u-mingw.c b/t/unit-tests/u-mingw.c new file mode 100644 index 00000000000000..cb74da5e793a33 --- /dev/null +++ b/t/unit-tests/u-mingw.c @@ -0,0 +1,72 @@ +#include "unit-test.h" + +#if defined(GIT_WINDOWS_NATIVE) && !defined(_UCRT) +#undef strerror +int errnos_contains(int); +static int errnos [53]={ + /* errnos in err_win_to_posix */ + EACCES, EBUSY, EEXIST, ERANGE, EIO, ENODEV, ENXIO, ENOEXEC, EINVAL, ENOENT, + EPIPE, ENAMETOOLONG, ENOSYS, ENOTEMPTY, ENOSPC, EFAULT, EBADF, EPERM, EINTR, + E2BIG, ESPIPE, ENOMEM, EXDEV, EAGAIN, ENFILE, EMFILE, ECHILD, EROFS, + /* errnos only in winsock_error_to_errno */ + EWOULDBLOCK, EINPROGRESS, EALREADY, ENOTSOCK, EDESTADDRREQ, EMSGSIZE, + EPROTOTYPE, ENOPROTOOPT, EPROTONOSUPPORT, EOPNOTSUPP, EAFNOSUPPORT, + EADDRINUSE, EADDRNOTAVAIL, ENETDOWN, ENETUNREACH, ENETRESET, ECONNABORTED, + ECONNRESET, ENOBUFS, EISCONN, ENOTCONN, ETIMEDOUT, ECONNREFUSED, ELOOP, + EHOSTUNREACH + }; + +int errnos_contains(int errnum) +{ + for(int i=0;i<53;i++) + if(errnos[i]==errnum) + return 1; + return 0; +} +#endif + +void test_mingw__no_strerror_shim_on_ucrt(void) +{ +#if defined(GIT_WINDOWS_NATIVE) && defined(_UCRT) + cl_assert_(strerror != mingw_strerror, + "mingw_strerror is unnescessary when building against UCRT"); +#else + cl_skip(); +#endif +} + +void test_mingw__strerror(void) +{ +#if defined(GIT_WINDOWS_NATIVE) && !defined(_UCRT) + for(int i=0;i<53;i++) + { + char *crt; + char *mingw; + mingw = mingw_strerror(errnos[i]); + crt = strerror(errnos[i]); + cl_assert_(!strcasestr(mingw, "unknown error"), + "mingw_strerror should know all errno values we care about"); + if(!strcasestr(crt, "unknown error")) + cl_assert_equal_s(crt,mingw); + } +#else + cl_skip(); +#endif +} + +void test_mingw__errno_translation(void) +{ +#if defined(GIT_WINDOWS_NATIVE) && !defined(_UCRT) + /* GetLastError() return values are currently defined from 0 to 15841, + testing up to 20000 covers some room for future expansion */ + for (int i=0;i<20000;i++) + { + if(i!=ERROR_SUCCESS) + cl_assert_(errnos_contains(err_win_to_posix(i)), + "all err_win_to_posix return values should be tested against mingw_strerror"); + /* ideally we'd test the same for winsock_error_to_errno, but it's static */ + } +#else + cl_skip(); +#endif +} From 86c5a9b4899bd8ed080d22918c50558f160093f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Sun, 22 Dec 2024 17:43:45 +0100 Subject: [PATCH 2/4] compat/mingw: drop outdated comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The part about keeping the original error number hasn't been accurate since commit c11f75c (mingw: make sure errno is set correctly when socket operations fail, 2019-11-25) and the part about strerror() not knowing about these errors is untrue since the previous commit. Signed-off-by: Matthias Aßhauer Signed-off-by: Johannes Schindelin --- compat/mingw.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 2373ddfd4a0f6e..cda75024767787 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2795,15 +2795,6 @@ int mingw_socket(int domain, int type, int protocol) ensure_socket_initialization(); s = WSASocket(domain, type, protocol, NULL, 0, 0); if (s == INVALID_SOCKET) { - /* - * WSAGetLastError() values are regular BSD error codes - * biased by WSABASEERR. - * However, strerror() does not know about networking - * specific errors, which are values beginning at 38 or so. - * Therefore, we choose to leave the biased error code - * in errno so that _if_ someone looks up the code somewhere, - * then it is at least the number that are usually listed. - */ set_wsa_errno(); return -1; } From dfbc6bd00570adb2873fad58dd8e9fc5ed0b9de7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Sun, 29 Dec 2024 11:48:34 +0100 Subject: [PATCH 3/4] t0301: actually test credential-cache on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 2406bf5 (Win32: detect unix socket support at runtime, 2024-04-03) introduced a runtime detection for whether the operating system supports unix sockets for Windows, but a mistake snuck into the tests. When building and testing Git without NO_UNIX_SOCKETS we currently skip t0301-credential-cache on Windows if unix sockets are supported and run the tests if they aren't. Flip that logic to actually work the way it was intended. Signed-off-by: Matthias Aßhauer Signed-off-by: Johannes Schindelin --- t/t0301-credential-cache.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh index dc30289f7539ee..586681c681c3cb 100755 --- a/t/t0301-credential-cache.sh +++ b/t/t0301-credential-cache.sh @@ -12,7 +12,7 @@ test -z "$NO_UNIX_SOCKETS" || { if test_have_prereq MINGW then service_running=$(sc query afunix | grep "4 RUNNING") - test -z "$service_running" || { + test -n "$service_running" || { skip_all='skipping credential-cache tests, unix sockets not available' test_done } From 9a70050492f6aa5bd88358c43d2d9a6d34036163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20A=C3=9Fhauer?= Date: Sun, 22 Dec 2024 17:24:24 +0100 Subject: [PATCH 4/4] credential-cache: handle ECONNREFUSED gracefully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In 245670c (credential-cache: check for windows specific errors, 2021-09-14) we concluded that on Windows we would always encounter ENETDOWN where we would expect ECONNREFUSED on POSIX systems, when connecting to unix sockets. As reported in [1], we do encounter ECONNREFUSED on Windows if the socket file doesn't exist, but the containing directory does and ENETDOWN if neither exists. We should handle this case like we do on non-windows systems. [1] https://github.com/git-for-windows/git/pull/4762#issuecomment-2545498245 This fixes https://github.com/git-for-windows/git/issues/5314 Helped-by: M Hickford Signed-off-by: Matthias Aßhauer Signed-off-by: Johannes Schindelin --- builtin/credential-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c index 7f733cb756e03c..3b8130d3d64f9c 100644 --- a/builtin/credential-cache.c +++ b/builtin/credential-cache.c @@ -23,7 +23,7 @@ static int connection_closed(int error) static int connection_fatally_broken(int error) { - return (error != ENOENT) && (error != ENETDOWN); + return (error != ENOENT) && (error != ENETDOWN) && (error != ECONNREFUSED); } #else