Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

credential-cache: handle ECONNREFUSED gracefully #5329

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,7 @@ THIRD_PARTY_SOURCES += $(UNIT_TEST_DIR)/clar/clar/%

CLAR_TEST_SUITES += ctype
CLAR_TEST_SUITES += strvec
CLAR_TEST_SUITES += 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
Copy link

@hickford hickford Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To distinguish the two identical error messages "unable to connect to cache daemon", consider changing the second to “unable to connect to spawned cache daemon”

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 @@ -2672,6 +2672,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 @@ -2709,15 +2794,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
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/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
}
Loading