Skip to content

Commit 380dde9

Browse files
committed
test: Add more logging to TCP connection constructor.
Also add more asserts to the test so we don't do UB.
1 parent 0f12f38 commit 380dde9

File tree

6 files changed

+39
-10
lines changed

6 files changed

+39
-10
lines changed

.cirrus.yml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ bazel-opt_task:
88
configure_script:
99
- git submodule update --init --recursive
1010
- /src/workspace/tools/inject-repo c-toxcore
11-
- cat /src/workspace/.bazelrc.local
1211
test_all_script:
1312
- cd /src/workspace && bazel
1413
test -k
@@ -27,11 +26,9 @@ bazel-dbg_task:
2726
configure_script:
2827
- git submodule update --init --recursive
2928
- /src/workspace/tools/inject-repo c-toxcore
30-
- cat /src/workspace/.bazelrc.local
3129
test_all_script:
3230
- cd /src/workspace && bazel
3331
test -k
34-
--remote_cache=http://$CIRRUS_HTTP_CACHE_HOST
3532
--build_tag_filters=-haskell
3633
--test_tag_filters=-haskell
3734
--
@@ -47,11 +44,9 @@ bazel-msan_task:
4744
configure_script:
4845
- git submodule update --init --recursive
4946
- /src/workspace/tools/inject-repo c-toxcore
50-
- cat /src/workspace/.bazelrc.local
5147
test_all_script:
5248
- cd /src/workspace && bazel
5349
test -k
54-
--remote_cache=http://$CIRRUS_HTTP_CACHE_HOST
5550
--
5651
//c-toxcore/auto_tests:lossless_packet_test
5752

@@ -85,6 +80,7 @@ freebsd_task:
8580
libconfig
8681
libsodium
8782
libvpx
83+
ninja
8884
opus
8985
pkgconf
9086
- git submodule update --init --recursive
@@ -98,6 +94,7 @@ freebsd_task:
9894
-DNON_HERMETIC_TESTS=OFF \
9995
-DTEST_TIMEOUT_SECONDS=50 \
10096
-DUSE_IPV6=OFF \
101-
-DAUTOTEST=ON
97+
-DAUTOTEST=ON \
98+
-GNinja
10299
cmake --build . --target install
103100
ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6

.github/scripts/cmake-freebsd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ RUN "cmake -B_build -Hc-toxcore \
4242
-DCMAKE_EXE_LINKER_FLAGS='$LD_FLAGS' \
4343
-DCMAKE_SHARED_LINKER_FLAGS='$LD_FLAGS' \
4444
-DCMAKE_INSTALL_PREFIX:PATH='_install' \
45+
-DENABLE_SHARED=OFF \
4546
-DMIN_LOGGER_LEVEL=TRACE \
4647
-DMUST_BUILD_TOXAV=ON \
4748
-DNON_HERMETIC_TESTS=ON \

.github/workflows/ci.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ jobs:
127127
libopus
128128
libsodium
129129
libvpx
130+
ninja
130131
pkg-config
131132

132133
run: |
@@ -138,7 +139,8 @@ jobs:
138139
-DNON_HERMETIC_TESTS=ON \
139140
-DTEST_TIMEOUT_SECONDS=90 \
140141
-DUSE_IPV6=OFF \
141-
-DAUTOTEST=ON
142+
-DAUTOTEST=ON \
143+
-GNinja
142144
cmake --build . --target install
143145
ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6
144146
@@ -163,6 +165,7 @@ jobs:
163165
libconfig
164166
libsodium
165167
libvpx
168+
ninja
166169
opus
167170
pkgconf
168171

@@ -175,7 +178,8 @@ jobs:
175178
-DNON_HERMETIC_TESTS=ON \
176179
-DTEST_TIMEOUT_SECONDS=50 \
177180
-DUSE_IPV6=OFF \
178-
-DAUTOTEST=ON
181+
-DAUTOTEST=ON \
182+
-GNinja
179183
cmake --build . --target install
180184
ctest -j50 --output-on-failure --rerun-failed --repeat until-pass:6
181185

auto_tests/TCP_test.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "../toxcore/TCP_server.h"
99
#include "../toxcore/crypto_core.h"
1010
#include "../toxcore/mono_time.h"
11-
#include "../toxcore/util.h"
1211
#include "auto_test_support.h"
1312

1413
#define NUM_PORTS 3
@@ -534,6 +533,7 @@ static void test_client(void)
534533
ip_port_tcp_s.ip = get_loopback();
535534

536535
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);
536+
ck_assert_msg(conn != nullptr, "Failed to create a TCP client connection.");
537537
// TCP sockets might need a moment before they can be written to.
538538
c_sleep(50);
539539
do_tcp_connection(logger, mono_time, conn, nullptr);
@@ -570,6 +570,7 @@ static void test_client(void)
570570
ip_port_tcp_s.port = net_htons(ports[random_u32(rng) % NUM_PORTS]);
571571
TCP_Client_Connection *conn2 = new_tcp_connection(logger, mem, mono_time, rng, ns, &ip_port_tcp_s, self_public_key, f2_public_key,
572572
f2_secret_key, nullptr);
573+
ck_assert_msg(conn2 != nullptr, "Failed to create a second TCP client connection.");
573574
c_sleep(50);
574575

575576
// The client should call this function (defined earlier) during the routing process.
@@ -667,6 +668,7 @@ static void test_client_invalid(void)
667668
ip_port_tcp_s.ip = get_loopback();
668669
TCP_Client_Connection *conn = new_tcp_connection(logger, mem, mono_time, rng, ns, &ip_port_tcp_s,
669670
self_public_key, f_public_key, f_secret_key, nullptr);
671+
ck_assert_msg(conn != nullptr, "Failed to create a TCP client connection.");
670672

671673
// Run the client's main loop but not the server.
672674
mono_time_update(mono_time);
@@ -743,10 +745,12 @@ static void test_tcp_connection(void)
743745
proxy_info.proxy_type = TCP_PROXY_NONE;
744746
crypto_new_keypair(rng, self_public_key, self_secret_key);
745747
TCP_Connections *tc_1 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info);
748+
ck_assert_msg(tc_1 != nullptr, "Failed to create TCP connections");
746749
ck_assert_msg(pk_equal(tcp_connections_public_key(tc_1), self_public_key), "Wrong public key");
747750

748751
crypto_new_keypair(rng, self_public_key, self_secret_key);
749752
TCP_Connections *tc_2 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info);
753+
ck_assert_msg(tc_2 != nullptr, "Failed to create TCP connections");
750754
ck_assert_msg(pk_equal(tcp_connections_public_key(tc_2), self_public_key), "Wrong public key");
751755

752756
IP_Port ip_port_tcp_s;
@@ -858,10 +862,12 @@ static void test_tcp_connection2(void)
858862
proxy_info.proxy_type = TCP_PROXY_NONE;
859863
crypto_new_keypair(rng, self_public_key, self_secret_key);
860864
TCP_Connections *tc_1 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info);
865+
ck_assert_msg(tc_1 != nullptr, "Failed to create TCP connections");
861866
ck_assert_msg(pk_equal(tcp_connections_public_key(tc_1), self_public_key), "Wrong public key");
862867

863868
crypto_new_keypair(rng, self_public_key, self_secret_key);
864869
TCP_Connections *tc_2 = new_tcp_connections(logger, mem, rng, ns, mono_time, self_secret_key, &proxy_info);
870+
ck_assert_msg(tc_2 != nullptr, "Failed to create TCP connections");
865871
ck_assert_msg(pk_equal(tcp_connections_public_key(tc_2), self_public_key), "Wrong public key");
866872

867873
IP_Port ip_port_tcp_s;

auto_tests/tcp_relay_test.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55

66
#include "auto_test_support.h"
77

8+
#ifndef USE_IPV6
9+
#define USE_IPV6 1
10+
#endif
11+
812
int main(void)
913
{
1014
setvbuf(stdout, nullptr, _IONBF, 0);
1115

1216
struct Tox_Options *opts = tox_options_new(nullptr);
1317
tox_options_set_udp_enabled(opts, false);
18+
#if !USE_IPV6
19+
tox_options_set_ipv6_enabled(opts, false);
20+
#endif
1421
Tox *tox_tcp = tox_new_log(opts, nullptr, nullptr);
1522
tox_options_free(opts);
1623

toxcore/TCP_client.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ TCP_Client_Connection *new_tcp_connection(
591591
assert(ns != nullptr);
592592

593593
if (!net_family_is_ipv4(ip_port->ip.family) && !net_family_is_ipv6(ip_port->ip.family)) {
594+
LOGGER_ERROR(logger, "Invalid IP family: %d", ip_port->ip.family.value);
594595
return nullptr;
595596
}
596597

@@ -609,22 +610,34 @@ TCP_Client_Connection *new_tcp_connection(
609610
const Socket sock = net_socket(ns, family, TOX_SOCK_STREAM, TOX_PROTO_TCP);
610611

611612
if (!sock_valid(sock)) {
613+
LOGGER_ERROR(logger, "Failed to create TCP socket with family %d", family.value);
612614
return nullptr;
613615
}
614616

615617
if (!set_socket_nosigpipe(ns, sock)) {
618+
LOGGER_ERROR(logger, "Failed to set TCP socket to ignore SIGPIPE");
616619
kill_sock(ns, sock);
617620
return nullptr;
618621
}
619622

620-
if (!(set_socket_nonblock(ns, sock) && connect_sock_to(ns, logger, mem, sock, ip_port, proxy_info))) {
623+
if (!set_socket_nonblock(ns, sock)) {
624+
LOGGER_ERROR(logger, "Failed to set TCP socket to non-blocking");
625+
kill_sock(ns, sock);
626+
return nullptr;
627+
}
628+
629+
if (!connect_sock_to(ns, logger, mem, sock, ip_port, proxy_info)) {
630+
Ip_Ntoa ip_ntoa;
631+
LOGGER_WARNING(logger, "Failed to connect TCP socket to %s:%u",
632+
net_ip_ntoa(&ip_port->ip, &ip_ntoa), net_ntohs(ip_port->port));
621633
kill_sock(ns, sock);
622634
return nullptr;
623635
}
624636

625637
TCP_Client_Connection *temp = (TCP_Client_Connection *)mem_alloc(mem, sizeof(TCP_Client_Connection));
626638

627639
if (temp == nullptr) {
640+
LOGGER_ERROR(logger, "Failed to allocate memory for TCP_Client_Connection");
628641
kill_sock(ns, sock);
629642
return nullptr;
630643
}
@@ -657,6 +670,7 @@ TCP_Client_Connection *new_tcp_connection(
657670
temp->status = TCP_CLIENT_CONNECTING;
658671

659672
if (generate_handshake(temp) == -1) {
673+
LOGGER_ERROR(logger, "Failed to generate handshake");
660674
kill_sock(ns, sock);
661675
mem_delete(mem, temp);
662676
return nullptr;

0 commit comments

Comments
 (0)