Skip to content

Commit

Permalink
credential-cache: handle ECONNREFUSED gracefully (#5329)
Browse files Browse the repository at this point in the history
I should probably add some tests for this.
  • Loading branch information
dscho authored and Git for Windows Build Agent committed Jan 2, 2025
2 parents 9420adc + 22a6d04 commit 81974eb
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 11 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion builtin/credential-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
94 changes: 85 additions & 9 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -2710,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;
}
Expand Down
5 changes: 5 additions & 0 deletions compat/mingw.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions t/meson.build
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
clar_test_suites = [
'unit-tests/u-ctype.c',
'unit-tests/u-mingw.c',
'unit-tests/u-strvec.c',
]

Expand Down
2 changes: 1 addition & 1 deletion t/t0301-credential-cache.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
72 changes: 72 additions & 0 deletions t/unit-tests/u-mingw.c
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 81974eb

Please sign in to comment.