From 325f783a84d5946e1750a23e86bbc12cab6e0c34 Mon Sep 17 00:00:00 2001 From: iphydf Date: Fri, 10 Jan 2025 21:44:52 +0000 Subject: [PATCH] test: Add more logging to TCP connection constructor. Also add more asserts to the test so we don't do UB. --- .cirrus.yml | 4 +++- .github/scripts/cmake-freebsd | 1 + .github/workflows/ci.yml | 8 ++++++-- auto_tests/TCP_test.c | 8 +++++++- auto_tests/tcp_relay_test.c | 7 +++++++ toxcore/TCP_client.c | 16 +++++++++++++++- 6 files changed, 39 insertions(+), 5 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index c5c1cdf578..36b982f9b1 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -85,6 +85,7 @@ freebsd_task: libconfig libsodium libvpx + ninja opus pkgconf - git submodule update --init --recursive @@ -98,6 +99,7 @@ freebsd_task: -DNON_HERMETIC_TESTS=OFF \ -DTEST_TIMEOUT_SECONDS=50 \ -DUSE_IPV6=OFF \ - -DAUTOTEST=ON + -DAUTOTEST=ON \ + -GNinja cmake --build . --target install ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6 diff --git a/.github/scripts/cmake-freebsd b/.github/scripts/cmake-freebsd index 13011752ad..568241e8f5 100755 --- a/.github/scripts/cmake-freebsd +++ b/.github/scripts/cmake-freebsd @@ -42,6 +42,7 @@ RUN "cmake -B_build -Hc-toxcore \ -DCMAKE_EXE_LINKER_FLAGS='$LD_FLAGS' \ -DCMAKE_SHARED_LINKER_FLAGS='$LD_FLAGS' \ -DCMAKE_INSTALL_PREFIX:PATH='_install' \ + -DENABLE_SHARED=OFF \ -DMIN_LOGGER_LEVEL=TRACE \ -DMUST_BUILD_TOXAV=ON \ -DNON_HERMETIC_TESTS=ON \ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99cbe7b2d0..0b832d3778 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -127,6 +127,7 @@ jobs: libopus libsodium libvpx + ninja pkg-config run: | @@ -138,7 +139,8 @@ jobs: -DNON_HERMETIC_TESTS=ON \ -DTEST_TIMEOUT_SECONDS=90 \ -DUSE_IPV6=OFF \ - -DAUTOTEST=ON + -DAUTOTEST=ON \ + -GNinja cmake --build . --target install ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6 @@ -163,6 +165,7 @@ jobs: libconfig libsodium libvpx + ninja opus pkgconf @@ -175,7 +178,8 @@ jobs: -DNON_HERMETIC_TESTS=ON \ -DTEST_TIMEOUT_SECONDS=50 \ -DUSE_IPV6=OFF \ - -DAUTOTEST=ON + -DAUTOTEST=ON \ + -GNinja cmake --build . --target install ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6 diff --git a/auto_tests/TCP_test.c b/auto_tests/TCP_test.c index 15c73ccc81..cb27d27ff6 100644 --- a/auto_tests/TCP_test.c +++ b/auto_tests/TCP_test.c @@ -8,7 +8,6 @@ #include "../toxcore/TCP_server.h" #include "../toxcore/crypto_core.h" #include "../toxcore/mono_time.h" -#include "../toxcore/util.h" #include "auto_test_support.h" #define NUM_PORTS 3 @@ -534,6 +533,7 @@ static void test_client(void) ip_port_tcp_s.ip = get_loopback(); TCP_Client_Connection *conn = new_tcp_connection(logger, mem, mono_time, rng, ns, &ip_port_tcp_s, self_public_key, f_public_key, f_secret_key, nullptr); + ck_assert_msg(conn != nullptr, "Failed to create a TCP client connection."); // TCP sockets might need a moment before they can be written to. c_sleep(50); do_tcp_connection(logger, mono_time, conn, nullptr); @@ -570,6 +570,7 @@ static void test_client(void) ip_port_tcp_s.port = net_htons(ports[random_u32(rng) % NUM_PORTS]); TCP_Client_Connection *conn2 = new_tcp_connection(logger, mem, mono_time, rng, ns, &ip_port_tcp_s, self_public_key, f2_public_key, f2_secret_key, nullptr); + ck_assert_msg(conn2 != nullptr, "Failed to create a second TCP client connection."); c_sleep(50); // The client should call this function (defined earlier) during the routing process. @@ -667,6 +668,7 @@ static void test_client_invalid(void) ip_port_tcp_s.ip = get_loopback(); TCP_Client_Connection *conn = new_tcp_connection(logger, mem, mono_time, rng, ns, &ip_port_tcp_s, self_public_key, f_public_key, f_secret_key, nullptr); + ck_assert_msg(conn != nullptr, "Failed to create a TCP client connection."); // Run the client's main loop but not the server. mono_time_update(mono_time); @@ -743,10 +745,12 @@ static void test_tcp_connection(void) proxy_info.proxy_type = TCP_PROXY_NONE; crypto_new_keypair(rng, self_public_key, self_secret_key); TCP_Connections *tc_1 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info); + ck_assert_msg(tc_1 != nullptr, "Failed to create TCP connections"); ck_assert_msg(pk_equal(tcp_connections_public_key(tc_1), self_public_key), "Wrong public key"); crypto_new_keypair(rng, self_public_key, self_secret_key); TCP_Connections *tc_2 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info); + ck_assert_msg(tc_2 != nullptr, "Failed to create TCP connections"); ck_assert_msg(pk_equal(tcp_connections_public_key(tc_2), self_public_key), "Wrong public key"); IP_Port ip_port_tcp_s; @@ -858,10 +862,12 @@ static void test_tcp_connection2(void) proxy_info.proxy_type = TCP_PROXY_NONE; crypto_new_keypair(rng, self_public_key, self_secret_key); TCP_Connections *tc_1 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info); + ck_assert_msg(tc_1 != nullptr, "Failed to create TCP connections"); ck_assert_msg(pk_equal(tcp_connections_public_key(tc_1), self_public_key), "Wrong public key"); crypto_new_keypair(rng, self_public_key, self_secret_key); TCP_Connections *tc_2 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info); + ck_assert_msg(tc_2 != nullptr, "Failed to create TCP connections"); ck_assert_msg(pk_equal(tcp_connections_public_key(tc_2), self_public_key), "Wrong public key"); IP_Port ip_port_tcp_s; diff --git a/auto_tests/tcp_relay_test.c b/auto_tests/tcp_relay_test.c index b098f5cad2..48c24edb6e 100644 --- a/auto_tests/tcp_relay_test.c +++ b/auto_tests/tcp_relay_test.c @@ -5,12 +5,19 @@ #include "auto_test_support.h" +#ifndef USE_IPV6 +#define USE_IPV6 1 +#endif + int main(void) { setvbuf(stdout, nullptr, _IONBF, 0); struct Tox_Options *opts = tox_options_new(nullptr); tox_options_set_udp_enabled(opts, false); +#if !USE_IPV6 + tox_options_set_ipv6_enabled(opts, false); +#endif Tox *tox_tcp = tox_new_log(opts, nullptr, nullptr); tox_options_free(opts); diff --git a/toxcore/TCP_client.c b/toxcore/TCP_client.c index 8cef57a372..5e13134bec 100644 --- a/toxcore/TCP_client.c +++ b/toxcore/TCP_client.c @@ -591,6 +591,7 @@ TCP_Client_Connection *new_tcp_connection( assert(ns != nullptr); if (!net_family_is_ipv4(ip_port->ip.family) && !net_family_is_ipv6(ip_port->ip.family)) { + LOGGER_ERROR(logger, "Invalid IP family: %d", ip_port->ip.family.value); return nullptr; } @@ -609,15 +610,26 @@ TCP_Client_Connection *new_tcp_connection( const Socket sock = net_socket(ns, family, TOX_SOCK_STREAM, TOX_PROTO_TCP); if (!sock_valid(sock)) { + LOGGER_ERROR(logger, "Failed to create TCP socket with family %d", family.value); return nullptr; } if (!set_socket_nosigpipe(ns, sock)) { + LOGGER_ERROR(logger, "Failed to set TCP socket to ignore SIGPIPE"); kill_sock(ns, sock); return nullptr; } - if (!(set_socket_nonblock(ns, sock) && connect_sock_to(ns, logger, mem, sock, ip_port, proxy_info))) { + if (!set_socket_nonblock(ns, sock)) { + LOGGER_ERROR(logger, "Failed to set TCP socket to non-blocking"); + kill_sock(ns, sock); + return nullptr; + } + + if (!connect_sock_to(ns, logger, mem, sock, ip_port, proxy_info)) { + Ip_Ntoa ip_ntoa; + LOGGER_WARNING(logger, "Failed to connect TCP socket to %s:%u", + net_ip_ntoa(&ip_port->ip, &ip_ntoa), net_ntohs(ip_port->port)); kill_sock(ns, sock); return nullptr; } @@ -625,6 +637,7 @@ TCP_Client_Connection *new_tcp_connection( TCP_Client_Connection *temp = (TCP_Client_Connection *)mem_alloc(mem, sizeof(TCP_Client_Connection)); if (temp == nullptr) { + LOGGER_ERROR(logger, "Failed to allocate memory for TCP_Client_Connection"); kill_sock(ns, sock); return nullptr; } @@ -657,6 +670,7 @@ TCP_Client_Connection *new_tcp_connection( temp->status = TCP_CLIENT_CONNECTING; if (generate_handshake(temp) == -1) { + LOGGER_ERROR(logger, "Failed to generate handshake"); kill_sock(ns, sock); mem_delete(mem, temp); return nullptr;