diff --git a/tests/test_transport_tcp.c b/tests/test_transport_tcp.c index 6dd09f5..2010f14 100644 --- a/tests/test_transport_tcp.c +++ b/tests/test_transport_tcp.c @@ -266,12 +266,36 @@ static int count_open_fds(void) { return count; } +/* True when two sockaddrs name the same endpoint (family, address, and port). + * Comparing only the IP would let a connection to a different port on the same + * host pass, so the port is part of the identity. */ +static bool sockaddr_same_endpoint(const struct sockaddr_storage* a, + const struct sockaddr_storage* b) { + if (a->ss_family != b->ss_family) + return false; + if (a->ss_family == AF_INET) { + const struct sockaddr_in* ia = (const struct sockaddr_in*)a; + const struct sockaddr_in* ib = (const struct sockaddr_in*)b; + return ia->sin_port == ib->sin_port && ia->sin_addr.s_addr == ib->sin_addr.s_addr; + } + if (a->ss_family == AF_INET6) { + const struct sockaddr_in6* ia = (const struct sockaddr_in6*)a; + const struct sockaddr_in6* ib = (const struct sockaddr_in6*)b; + return ia->sin6_port == ib->sin6_port && + memcmp(&ia->sin6_addr, &ib->sin6_addr, sizeof(ia->sin6_addr)) == 0; + } + return false; +} + /* Bind + listen on the SECOND address getaddrinfo returns for "localhost", so * the first candidate is connection-refused and the shared connect loop must - * fall back to a later one. Returns the listener fd and its port, or -1 when - * this host does not resolve localhost to at least two addresses (the test then - * skips rather than claiming coverage it does not have). */ -static int bind_second_localhost_address(int* out_port) { + * fall back to a later one. On success the actual bound endpoint is written to + * out_bound/out_bound_len (the caller asserts the winning connect landed on it). + * Returns the listener fd and its port, or -1 when this host does not resolve + * localhost to at least two addresses (the test then skips rather than claiming + * coverage it does not have). */ +static int bind_second_localhost_address(int* out_port, struct sockaddr_storage* out_bound, + socklen_t* out_bound_len) { struct addrinfo hints; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; @@ -303,6 +327,10 @@ static int bind_second_localhost_address(int* out_port) { freeaddrinfo(res); return -1; } + if (out_bound) + *out_bound = bound; + if (out_bound_len) + *out_bound_len = bound_len; if (bound.ss_family == AF_INET6) *out_port = ntohs(((struct sockaddr_in6*)&bound)->sin6_port); else @@ -316,18 +344,29 @@ static int bind_second_localhost_address(int* out_port) { * (proving the failed attempt's fd was closed before the retry). */ static void test_tcp_connect_falls_back_to_next_address() { int port = 0; - int listener = bind_second_localhost_address(&port); + struct sockaddr_storage bound; + int listener = bind_second_localhost_address(&port, &bound, NULL); if (listener < 0) return; /* localhost is single-address on this host: cannot exercise fallback */ - int before = count_open_fds(); + + /* The /proc/self/fd delta is unreliable under valgrind (its own lazy fd + * activity perturbs the baseline), so only the functional assertions run + * there; the fd-count checks are skipped. */ + bool check_fds = !is_running_under_valgrind(); + int before = check_fds ? count_open_fds() : -1; + Client* c = client_create(); EXPECT_NOT_NULL(c); EXPECT_TRUE(client_connect(c, "localhost", port)); EXPECT_TRUE(c->file_descriptor >= 0); - if (before >= 0) + /* The winning candidate must be the endpoint we bound (the second + * getaddrinfo entry). Without this, a re-resolution that dropped the second + * address would make the test pass without ever exercising fallback. */ + EXPECT_TRUE(sockaddr_same_endpoint(&c->address, &bound)); + if (check_fds && before >= 0) EXPECT_EQ_INT(count_open_fds(), before + 1); client_disconnect(c); - if (before >= 0) + if (check_fds && before >= 0) EXPECT_EQ_INT(count_open_fds(), before); client_delete(c); close(listener); @@ -336,7 +375,13 @@ static void test_tcp_connect_falls_back_to_next_address() { /* #219 AC3: a connect that fails on every candidate leaves at most one * descriptor (the last failed attempt) and none after client_disconnect. */ static void test_tcp_connect_failed_attempts_do_not_leak_fds() { - /* Reserve an ephemeral port, then close it: connecting to it must fail. */ + if (is_running_under_valgrind()) + return; /* /proc/self/fd delta is perturbed by valgrind's own lazy fds */ + + /* Keep an ephemeral loopback port bound (but NOT listening) for the whole + * assertion: the port stays occupied by our own socket, so the kernel + * deterministically refuses a connect() to it. This closes the bind/close/ + * connect TOCTOU window in which a parallel test could claim the port. */ int probe = socket(AF_INET, SOCK_STREAM, 0); EXPECT_TRUE(probe >= 0); struct sockaddr_in addr; @@ -348,18 +393,20 @@ static void test_tcp_connect_failed_attempts_do_not_leak_fds() { socklen_t addr_len = sizeof(addr); EXPECT_EQ_INT(getsockname(probe, (struct sockaddr*)&addr, &addr_len), 0); int port = ntohs(addr.sin_port); - close(probe); int before = count_open_fds(); Client* c = client_create(); EXPECT_NOT_NULL(c); - EXPECT_FALSE(client_connect(c, "localhost", port)); + /* The literal loopback address has a single getaddrinfo candidate -- the one + * our bound socket owns -- so the connect is deterministically refused. */ + EXPECT_FALSE(client_connect(c, "127.0.0.1", port)); if (before >= 0) EXPECT_TRUE(count_open_fds() <= before + 1); client_disconnect(c); if (before >= 0) EXPECT_EQ_INT(count_open_fds(), before); client_delete(c); + close(probe); } /* #219 AC3: the shared tcp_connect_socket_ex() (used by both the plain and TLS diff --git a/tests/test_transport_tls.c b/tests/test_transport_tls.c index 1385ff8..aeab847 100644 --- a/tests/test_transport_tls.c +++ b/tests/test_transport_tls.c @@ -106,12 +106,16 @@ static void test_client_connect_tls_releases_fd_on_setup_failure() { EXPECT_EQ_INT(getsockname(s->file_descriptor, (struct sockaddr*)&bound, &bound_len), 0); int port = ntohs(bound.sin_port); - int before = tls_count_open_fds(); + /* The /proc/self/fd delta is unreliable under valgrind (its own lazy fd + * activity perturbs the baseline); keep the functional assertions and skip + * only the count checks there. */ + bool check_fds = !is_running_under_valgrind(); + int before = check_fds ? tls_count_open_fds() : -1; Client* c = client_create(); EXPECT_NOT_NULL(c); EXPECT_FALSE(client_connect_tls(c, "127.0.0.1", port, NULL, NULL, NULL)); EXPECT_TRUE(c->file_descriptor == -1); - if (before >= 0) + if (check_fds && before >= 0) EXPECT_EQ_INT(tls_count_open_fds(), before); client_delete(c); server_delete(&s);