diff --git a/contrib/win32/win32compat/socketio.c b/contrib/win32/win32compat/socketio.c index c4eceaea8f71..d3b07e3aec3f 100644 --- a/contrib/win32/win32compat/socketio.c +++ b/contrib/win32/win32compat/socketio.c @@ -271,6 +271,29 @@ socketio_setsockopt(struct w32_io* pio, int level, int optname, const char* optv int socketio_getsockopt(struct w32_io* pio, int level, int optname, char* optval, int* optlen) { + /* + * A failed asynchronous ConnectEx() reports its error through the + * overlapped completion, which socketio_is_io_available()/ + * socketio_finish_connect() capture into write_details.error / + * read_details.error. Once that completion has been consumed the + * underlying socket's SO_ERROR is left at 0, so a plain getsockopt() + * would report success for a connect that actually failed. POSIX code + * relies on getsockopt(SO_ERROR) after a non-blocking connect() to + * detect failure and move on to the next address (e.g. the IPv6 -> IPv4 + * fallback loop in ssh_connect_direct()/timeout_connect()). Surface the + * captured error here so that idiom keeps working on Windows. + */ + if (level == SOL_SOCKET && optname == SO_ERROR && optval != NULL && + optlen != NULL && *optlen >= (int)sizeof(int)) { + DWORD wsa_error = pio->write_details.error ? + pio->write_details.error : pio->read_details.error; + if (wsa_error != 0) { + *(int*)optval = errno_from_WSAError(wsa_error); + *optlen = sizeof(int); + return 0; + } + } + SET_ERRNO_ON_ERROR(getsockopt(pio->sock, level, optname, optval, optlen)); } diff --git a/regress/unittests/win32compat/socket_tests.c b/regress/unittests/win32compat/socket_tests.c index 8169cf4b0822..da4410b53f58 100644 --- a/regress/unittests/win32compat/socket_tests.c +++ b/regress/unittests/win32compat/socket_tests.c @@ -700,6 +700,62 @@ socket_typical_ssh_payload_tests() { freeaddrinfo(servinfo); } +/* + * Regression test: getsockopt(SO_ERROR) must report a failed non-blocking + * connect(). A failed asynchronous ConnectEx() stores its error in the + * w32_io details but leaves the raw socket SO_ERROR at 0; if getsockopt() + * does not surface it, ssh_connect_direct()'s connect loop mistakes the + * failed connect for success and never tries the next address (the + * IPv6 -> IPv4 fallback that lets dual-stack hosts connect). See + * socketio_getsockopt() in contrib/win32/win32compat/socketio.c. + */ +void +socket_connect_error_tests() +{ + int so_error, optlen; + + { + TEST_START("getsockopt SO_ERROR reports refused connect"); + + /* A loopback port with nothing listening -> connect is refused. */ + memset(&hints, 0, sizeof(hints)); + hints.ai_socktype = SOCK_STREAM; + retValue = getaddrinfo("127.0.0.1", "34999", &hints, &servinfo); + ASSERT_INT_EQ(retValue, 0); + connect_fd = socket(servinfo->ai_family, servinfo->ai_socktype, servinfo->ai_protocol); + ASSERT_INT_NE(connect_fd, -1); + ASSERT_INT_EQ(w32_set_nonblock(connect_fd), 0); + + retValue = connect(connect_fd, servinfo->ai_addr, servinfo->ai_addrlen); + if (retValue == -1 && errno == EINPROGRESS) { + /* wait for the async connect to complete (with an error) */ + FD_ZERO(&read_set); + FD_ZERO(&write_set); + FD_SET(connect_fd, &read_set); + FD_SET(connect_fd, &write_set); + time_val.tv_sec = 5; + time_val.tv_usec = 0; + retValue = select(connect_fd + 1, &read_set, &write_set, NULL, &time_val); + ASSERT_INT_NE(retValue, -1); + } + + /* + * The connect was refused, so SO_ERROR must be non-zero. Before the + * fix this returned 0 because the ConnectEx error was not surfaced, + * causing callers to treat the failed connect as a success. + */ + so_error = 0; + optlen = sizeof(so_error); + retValue = getsockopt(connect_fd, SOL_SOCKET, SO_ERROR, (char*)&so_error, &optlen); + ASSERT_INT_EQ(retValue, 0); + ASSERT_INT_NE(so_error, 0); + + close(connect_fd); + freeaddrinfo(servinfo); + TEST_DONE(); + } +} + void socket_tests() { @@ -708,4 +764,5 @@ socket_tests() socket_nonblocking_io_tests(); socket_select_tests(); socket_typical_ssh_payload_tests(); + socket_connect_error_tests(); }