Ensure save data unchanged after save and load

auto_tests/save_load_test.c

@@ -16,6 +16,22 @@
 #include "../toxcore/util.h"
 #include "check_compat.h"
 
+/* The Travis-CI container responds poorly to ::1 as a localhost address
+ * You're encouraged to -D FORCE_TESTS_IPV6 on a local test  */
+#ifdef TOX_LOCALHOST
+#undef TOX_LOCALHOST
+#endif
+#ifdef FORCE_TESTS_IPV6
+#define TOX_LOCALHOST "::1"
+#else
+#define TOX_LOCALHOST "127.0.0.1"
+#endif
+
+#ifdef TCP_RELAY_PORT
+#undef TCP_RELAY_PORT
+#endif
+#define TCP_RELAY_PORT 33430
+
 static void accept_friend_request(Tox *m, const uint8_t *public_key, const uint8_t *data, size_t length, void *userdata)
 {
     if (length == 7 && memcmp("Gentoo", data, 7) == 0) {
@@ -35,21 +51,91 @@ static void tox_connection_status(Tox *tox, Tox_Connection connection_status, vo
     connected_t1 = connection_status;
 }
 
+/* validate that:
+ * a) saving stays within the confined space
+ * b) a saved state can be loaded back successfully
+ * c) a second save is of equal size
+ * d) the second save is of equal content */
+static void reload_tox(Tox **tox, struct Tox_Options *const in_opts, void *user_data)
+{
+    const size_t extra = 64;
+    const size_t save_size1 = tox_get_savedata_size(*tox);
+    ck_assert_msg(save_size1 != 0, "save is invalid size %u", (unsigned)save_size1);
+    printf("%u\n", (unsigned)save_size1);
+
+    uint8_t *buffer = (uint8_t *)malloc(save_size1 + 2 * extra);
+    ck_assert_msg(buffer != nullptr, "malloc failed");
+    memset(buffer, 0xCD, extra);
+    memset(buffer + extra + save_size1, 0xCD, extra);
+    tox_get_savedata(*tox, buffer + extra);
+    tox_kill(*tox);
+
+    for (size_t i = 0; i < extra; ++i) {
+        ck_assert_msg(buffer[i] == 0xCD, "Buffer underwritten from tox_get_savedata() @%u", (unsigned)i);
+        ck_assert_msg(buffer[extra + save_size1 + i] == 0xCD, "Buffer overwritten from tox_get_savedata() @%u", (unsigned)i);
+    }
+
+    struct Tox_Options *const options = (in_opts == nullptr) ? tox_options_new(nullptr) : in_opts;
+
+    tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
+
+    tox_options_set_savedata_data(options, buffer + extra, save_size1);
+
+    *tox = tox_new_log(options, nullptr, user_data);
+
+    if (in_opts == nullptr) {
+        tox_options_free(options);
+    }
+
+    ck_assert_msg(*tox != nullptr, "Failed to load back stored buffer");
+
+    const size_t save_size2 = tox_get_savedata_size(*tox);
+
+    ck_assert_msg(save_size1 == save_size2, "Tox save data changed in size from a store/load cycle: %u -> %u",
+                  (unsigned)save_size1, (unsigned)save_size2);
+
+    uint8_t *buffer2 = (uint8_t *)malloc(save_size2);
+
+    ck_assert_msg(buffer2 != nullptr, "malloc failed");
+
+    tox_get_savedata(*tox, buffer2);
+
+    ck_assert_msg(!memcmp(buffer + extra, buffer2, save_size2), "Tox state changed by store/load/store cycle");
+
+    free(buffer2);
+
+    free(buffer);
+}
+
 static void test_few_clients(void)
 {
     uint32_t index[] = { 1, 2, 3 };
     time_t con_time = 0, cur_time = time(nullptr);
-    Tox *tox1 = tox_new_log(nullptr, nullptr, &index[0]);
-    Tox *tox2 = tox_new_log(nullptr, nullptr, &index[1]);
-    Tox *tox3 = tox_new_log(nullptr, nullptr, &index[2]);
+
+    struct Tox_Options *opts1 = tox_options_new(nullptr);
+    tox_options_set_tcp_port(opts1, TCP_RELAY_PORT);
+    Tox *tox1 = tox_new_log(opts1, nullptr, &index[0]);
+    tox_options_free(opts1);
+
+    struct Tox_Options *opts2 = tox_options_new(nullptr);
+    tox_options_set_udp_enabled(opts2, false);
+    tox_options_set_local_discovery_enabled(opts2, false);
+    Tox *tox2 = tox_new_log(opts2, nullptr, &index[1]);
+
+    struct Tox_Options *opts3 = tox_options_new(nullptr);
+    tox_options_set_local_discovery_enabled(opts3, false);
+    Tox *tox3 = tox_new_log(opts3, nullptr, &index[2]);
 
     ck_assert_msg(tox1 && tox2 && tox3, "Failed to create 3 tox instances");
 
-    printf("bootstrapping tox2 and tox3 off tox1\n");
     uint8_t dht_key[TOX_PUBLIC_KEY_SIZE];
     tox_self_get_dht_id(tox1, dht_key);
     const uint16_t dht_port = tox_self_get_udp_port(tox1, nullptr);
 
+    printf("using tox1 as tcp relay for tox2\n");
+    tox_add_tcp_relay(tox2, TOX_LOCALHOST, TCP_RELAY_PORT, dht_key, nullptr);
+
+    printf("bootstrapping toxes off tox1\n");
     tox_bootstrap(tox2, "localhost", dht_port, dht_key, nullptr);
     tox_bootstrap(tox3, "localhost", dht_port, dht_key, nullptr);
 
@@ -76,8 +162,8 @@ static void test_few_clients(void)
                 off = 0;
             }
 
-            if (tox_friend_get_connection_status(tox2, 0, nullptr) == TOX_CONNECTION_UDP
-                    && tox_friend_get_connection_status(tox3, 0, nullptr) == TOX_CONNECTION_UDP) {
+            if (tox_friend_get_connection_status(tox2, 0, nullptr) == TOX_CONNECTION_TCP
+                    && tox_friend_get_connection_status(tox3, 0, nullptr) == TOX_CONNECTION_TCP) {
                 break;
             }
         }
@@ -88,19 +174,12 @@ static void test_few_clients(void)
     ck_assert_msg(connected_t1, "Tox1 isn't connected. %u", connected_t1);
     printf("tox clients connected took %lu seconds\n", (unsigned long)(time(nullptr) - con_time));
 
-    const size_t save_size1 = tox_get_savedata_size(tox2);
-    ck_assert_msg(save_size1 != 0, "save is invalid size %u", (unsigned)save_size1);
-    printf("%u\n", (unsigned)save_size1);
-    VLA(uint8_t, save1, save_size1);
-    tox_get_savedata(tox2, save1);
-    tox_kill(tox2);
+    reload_tox(&tox2, opts2, &index[1]);
+
+    reload_tox(&tox3, opts3, &index[2]);
 
-    struct Tox_Options *const options = tox_options_new(nullptr);
-    tox_options_set_savedata_type(options, TOX_SAVEDATA_TYPE_TOX_SAVE);
-    tox_options_set_savedata_data(options, save1, save_size1);
-    tox_options_set_local_discovery_enabled(options, false);
-    tox2 = tox_new_log(options, nullptr, &index[1]);
     cur_time = time(nullptr);
+
     off = 1;
 
     while (true) {
@@ -116,8 +195,8 @@ static void test_few_clients(void)
                 off = 0;
             }
 
-            if (tox_friend_get_connection_status(tox2, 0, nullptr) == TOX_CONNECTION_UDP
-                    && tox_friend_get_connection_status(tox3, 0, nullptr) == TOX_CONNECTION_UDP) {
+            if (tox_friend_get_connection_status(tox2, 0, nullptr) == TOX_CONNECTION_TCP
+                    && tox_friend_get_connection_status(tox3, 0, nullptr) == TOX_CONNECTION_TCP) {
                 break;
             }
         }
@@ -129,10 +208,12 @@ static void test_few_clients(void)
 
     printf("test_few_clients succeeded, took %lu seconds\n", (unsigned long)(time(nullptr) - cur_time));
 
-    tox_options_free(options);
     tox_kill(tox1);
     tox_kill(tox2);
     tox_kill(tox3);
+
+    tox_options_free(opts2);
+    tox_options_free(opts3);
 }
 
 int main(void)

toxcore/DHT.c

@@ -2803,6 +2803,11 @@ uint32_t dht_size(const DHT *dht)
     uint32_t numv4 = 0;
     uint32_t numv6 = 0;
 
+    for (uint32_t i = 0; i < dht->loaded_num_nodes; ++i) {
+        numv4 += net_family_is_ipv4(dht->loaded_nodes_list[i].ip_port.ip.family);
+        numv6 += net_family_is_ipv6(dht->loaded_nodes_list[i].ip_port.ip.family);
+    }
+
     for (uint32_t i = 0; i < LCLIENT_LIST; ++i) {
         numv4 += (dht->close_clientlist[i].assoc4.timestamp != 0);
         numv6 += (dht->close_clientlist[i].assoc6.timestamp != 0);
@@ -2838,6 +2843,11 @@ void dht_save(const DHT *dht, uint8_t *data)
 
     uint32_t num = 0;
 
+    if (dht->loaded_num_nodes > 0) {
+        memcpy(clients, dht->loaded_nodes_list, sizeof(Node_format) * dht->loaded_num_nodes);
+        num += dht->loaded_num_nodes;
+    }
+
     for (uint32_t i = 0; i < LCLIENT_LIST; ++i) {
         if (dht->close_clientlist[i].assoc4.timestamp != 0) {
             memcpy(clients[num].public_key, dht->close_clientlist[i].public_key, CRYPTO_PUBLIC_KEY_SIZE);

toxcore/Messenger.c

@@ -2593,12 +2593,12 @@ void do_messenger(Messenger *m, void *userdata)
     if (!m->has_added_relays) {
         m->has_added_relays = true;
 
-        int i;
-
-        for (i = 0; i < NUM_SAVED_TCP_RELAYS; ++i) {
+        for (uint16_t i = 0; i < m->num_loaded_relays; ++i) {
             add_tcp_relay(m->net_crypto, m->loaded_relays[i].ip_port, m->loaded_relays[i].public_key);
         }
 
+        m->num_loaded_relays = 0;
+
         if (m->tcp_server) {
             /* Add self tcp server. */
             IP_Port local_ip_port;
@@ -3035,6 +3035,7 @@ static uint8_t *friends_list_save(const Messenger *m, uint8_t *data)
                 temp.info_size = net_htons(m->friendlist[i].info_size);
                 temp.friendrequest_nospam = m->friendlist[i].friendrequest_nospam;
             } else {
+                temp.status = 3;
                 memcpy(temp.name, m->friendlist[i].name, m->friendlist[i].name_length);
                 temp.name_length = net_htons(m->friendlist[i].name_length);
                 memcpy(temp.statusmessage, m->friendlist[i].statusmessage, m->friendlist[i].statusmessage_length);
@@ -3193,7 +3194,13 @@ static uint8_t *save_tcp_relays(const Messenger *m, uint8_t *data)
     Node_format relays[NUM_SAVED_TCP_RELAYS];
     uint8_t *temp_data = data;
     data = state_write_section_header(temp_data, MESSENGER_STATE_COOKIE_TYPE, 0, MESSENGER_STATE_TYPE_TCP_RELAY);
-    unsigned int num = copy_connected_tcp_relays(m->net_crypto, relays, NUM_SAVED_TCP_RELAYS);
+    uint32_t num = copy_connected_tcp_relays(m->net_crypto, relays, NUM_SAVED_TCP_RELAYS);
+
+    if (m->num_loaded_relays > 0) {
+        memcpy(relays, m->loaded_relays, sizeof(Node_format) * m->num_loaded_relays);
+        num = min_u32(num + m->num_loaded_relays, NUM_SAVED_TCP_RELAYS);
+    }
+
     int l = pack_nodes(data, NUM_SAVED_TCP_RELAYS * packed_node_size(net_family_tcp_ipv6), relays, num);
 
     if (l > 0) {
@@ -3208,7 +3215,7 @@ static uint8_t *save_tcp_relays(const Messenger *m, uint8_t *data)
 static State_Load_Status load_tcp_relays(Messenger *m, const uint8_t *data, uint32_t length)
 {
     if (length != 0) {
-        unpack_nodes(m->loaded_relays, NUM_SAVED_TCP_RELAYS, nullptr, data, length, 1);
+        m->num_loaded_relays = unpack_nodes(m->loaded_relays, NUM_SAVED_TCP_RELAYS, nullptr, data, length, 1);
         m->has_added_relays = false;
     }
 

toxcore/Messenger.h

@@ -293,6 +293,8 @@ struct Messenger {
     time_t lastdump;
 
     bool has_added_relays; // If the first connection has occurred in do_messenger
+
+    uint16_t num_loaded_relays;
     Node_format loaded_relays[NUM_SAVED_TCP_RELAYS]; // Relays loaded from config
 
     m_friend_message_cb *friend_message;

toxcore/onion_client.c

@@ -232,27 +232,35 @@ static int onion_add_path_node(Onion_Client *onion_c, IP_Port ip_port, const uin
  */
 uint16_t onion_backup_nodes(const Onion_Client *onion_c, Node_format *nodes, uint16_t max_num)
 {
-    unsigned int i;
-
     if (!max_num) {
         return 0;
     }
 
-    unsigned int num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;
+    const uint16_t num_nodes = (onion_c->path_nodes_index < MAX_PATH_NODES) ? onion_c->path_nodes_index : MAX_PATH_NODES;
+    uint16_t i = 0;
 
-    if (num_nodes == 0) {
-        return 0;
+    while (i < max_num && i < num_nodes) {
+        nodes[i] = onion_c->path_nodes[(onion_c->path_nodes_index - (1 + i)) % num_nodes];
+        ++i;
     }
 
-    if (num_nodes < max_num) {
-        max_num = num_nodes;
-    }
+    for (uint16_t j = 0; i < max_num && j < MAX_PATH_NODES && j < onion_c->path_nodes_index_bs; ++j) {
+        bool already_saved = false;
 
-    for (i = 0; i < max_num; ++i) {
-        nodes[i] = onion_c->path_nodes[(onion_c->path_nodes_index - (1 + i)) % num_nodes];
+        for (uint16_t k = 0; k < num_nodes; ++k) {
+            if (public_key_cmp(nodes[k].public_key, onion_c->path_nodes_bs[j].public_key) == 0) {
+                already_saved = true;
+                break;
+            }
+        }
+
+        if (!already_saved) {
+            nodes[i] = onion_c->path_nodes_bs[j];
+            ++i;
+        }
     }
 
-    return max_num;
+    return i;
 }
 
 /* Put up to max_num random nodes in nodes.