fix: friend requests with very long messages are no longer dropped

auto_tests/friend_request_test.c

@@ -10,44 +10,57 @@
 #include "../toxcore/tox.h"
 #include "../toxcore/util.h"
 #include "../testing/misc_tools.h"
+
 #include "auto_test_support.h"
 #include "check_compat.h"
 
-#define FR_MESSAGE "Gentoo"
+#define REQUEST_MESSAGE "Hello, I would like to be your friend. Please respond."
+
+typedef struct Callback_Data {
+    Tox      *tox1;  // request sender
+    Tox      *tox2;  // request receiver
+    uint8_t  message[TOX_MAX_FRIEND_REQUEST_LENGTH];
+    uint16_t length;
+} Callback_Data;
 
-static void accept_friend_request(const Tox_Event_Friend_Request *event,
-                                  void *userdata)
+static void accept_friend_request(const Tox_Event_Friend_Request *event, void *userdata)
 {
-    Tox *state_tox = (Tox *)userdata;
+    Callback_Data *cb_data = (Callback_Data *)userdata;
 
     const uint8_t *public_key = tox_event_friend_request_get_public_key(event);
     const uint8_t *data = tox_event_friend_request_get_message(event);
     const size_t length = tox_event_friend_request_get_message_length(event);
 
-    ck_assert_msg(length == sizeof(FR_MESSAGE) && memcmp(FR_MESSAGE, data, sizeof(FR_MESSAGE)) == 0,
+    ck_assert_msg(length == cb_data->length && memcmp(cb_data->message, data, cb_data->length) == 0,
                   "unexpected friend request message");
-    tox_friend_add_norequest(state_tox, public_key, nullptr);
+
+    fprintf(stderr, "Tox2 accepts friend request.\n");
+
+    Tox_Err_Friend_Add err;
+    tox_friend_add_norequest(cb_data->tox2, public_key, &err);
+
+    ck_assert_msg(err == TOX_ERR_FRIEND_ADD_OK, "tox_friend_add_norequest failed: %d", err);
 }
 
-static void iterate2_wait(const Tox_Dispatch *dispatch, Tox *tox1, Tox *tox2)
+static void iterate2_wait(const Tox_Dispatch *dispatch, Callback_Data *cb_data)
 {
     Tox_Err_Events_Iterate err;
     Tox_Events *events;
 
-    events = tox_events_iterate(tox1, true, &err);
+    events = tox_events_iterate(cb_data->tox1, true, &err);
     ck_assert(err == TOX_ERR_EVENTS_ITERATE_OK);
-    tox_dispatch_invoke(dispatch, events, tox1);
+    tox_dispatch_invoke(dispatch, events, cb_data);
     tox_events_free(events);
 
-    events = tox_events_iterate(tox2, true, &err);
+    events = tox_events_iterate(cb_data->tox2, true, &err);
     ck_assert(err == TOX_ERR_EVENTS_ITERATE_OK);
-    tox_dispatch_invoke(dispatch, events, tox2);
+    tox_dispatch_invoke(dispatch, events, cb_data);
     tox_events_free(events);
 
     c_sleep(ITERATION_INTERVAL);
 }
 
-static void test_friend_request(void)
+static void test_friend_request(const uint8_t *message, uint16_t length)
 {
     printf("Initialising 2 toxes.\n");
     uint32_t index[] = { 1, 2 };
@@ -60,7 +73,7 @@ static void test_friend_request(void)
     tox_events_init(tox1);
     tox_events_init(tox2);
 
-    printf("Bootstrapping tox2 off tox1.\n");
+    printf("Bootstrapping Tox2 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);
@@ -70,25 +83,36 @@ static void test_friend_request(void)
     Tox_Dispatch *dispatch = tox_dispatch_new(nullptr);
     ck_assert(dispatch != nullptr);
 
+    Callback_Data cb_data = {nullptr};
+    cb_data.tox1 = tox1;
+    cb_data.tox2 = tox2;
+
+    ck_assert(length <= sizeof(cb_data.message));
+    memcpy(cb_data.message, message, length);
+    cb_data.length = length;
+
     do {
-        iterate2_wait(dispatch, tox1, tox2);
+        iterate2_wait(dispatch, &cb_data);
     } while (tox_self_get_connection_status(tox1) == TOX_CONNECTION_NONE ||
              tox_self_get_connection_status(tox2) == TOX_CONNECTION_NONE);
 
     printf("Toxes are online, took %lu seconds.\n", (unsigned long)(time(nullptr) - cur_time));
     const time_t con_time = time(nullptr);
 
-    printf("Tox1 adds tox2 as friend, tox2 accepts.\n");
+    printf("Tox1 adds Tox2 as friend. Waiting for Tox2 to accept.\n");
     tox_events_callback_friend_request(dispatch, accept_friend_request);
 
     uint8_t address[TOX_ADDRESS_SIZE];
     tox_self_get_address(tox2, address);
 
-    const uint32_t test = tox_friend_add(tox1, address, (const uint8_t *)FR_MESSAGE, sizeof(FR_MESSAGE), nullptr);
+    Tox_Err_Friend_Add err;
+    const uint32_t test = tox_friend_add(tox1, address, message, length, &err);
+
+    ck_assert_msg(err == TOX_ERR_FRIEND_ADD_OK, "tox_friend_add failed: %d", err);
     ck_assert_msg(test == 0, "failed to add friend error code: %u", test);
 
     do {
-        iterate2_wait(dispatch, tox1, tox2);
+        iterate2_wait(dispatch, &cb_data);
     } while (tox_friend_get_connection_status(tox1, 0, nullptr) != TOX_CONNECTION_UDP ||
              tox_friend_get_connection_status(tox2, 0, nullptr) != TOX_CONNECTION_UDP);
 
@@ -104,6 +128,20 @@ int main(void)
 {
     setvbuf(stdout, nullptr, _IONBF, 0);
 
-    test_friend_request();
+    fprintf(stderr, "Testing friend request with the smallest allowed message length.\n");
+    test_friend_request((const uint8_t *)"a", 1);
+
+    fprintf(stderr, "Testing friend request with an average sized message length.\n");
+    test_friend_request((const uint8_t *)REQUEST_MESSAGE, sizeof(REQUEST_MESSAGE) - 1);
+
+    uint8_t long_message[TOX_MAX_FRIEND_REQUEST_LENGTH];
+
+    for (uint16_t i = 0; i < sizeof(long_message); ++i) {
+        long_message[i] = 'a';
+    }
+
+    fprintf(stderr, "Testing friend request with the largest allowed message length.\n");
+    test_friend_request(long_message, sizeof(long_message));
+
     return 0;
 }

toxcore/friend_connection.c

@@ -875,6 +875,10 @@ void set_friend_request_callback(Friend_Connections *fr_c, fr_request_cb *fr_req
 int send_friend_request_packet(Friend_Connections *fr_c, int friendcon_id, uint32_t nospam_num, const uint8_t *data,
                                uint16_t length)
 {
+    // TODO(Jfreegman): This max packet size is too large to be handled by receiving clients
+    // when sent via the onion. We currently limit the length at a higher level, but
+    // this bounds check should be fixed to represent the max size of a packet that
+    // the onion client can handle.
     if (1 + sizeof(nospam_num) + length > ONION_CLIENT_MAX_DATA_SIZE || length == 0) {
         return -1;
     }

toxcore/friend_requests.h

@@ -14,7 +14,8 @@
 #include "attributes.h"
 #include "friend_connection.h"
 
-#define MAX_FRIEND_REQUEST_DATA_SIZE (ONION_CLIENT_MAX_DATA_SIZE - (1 + sizeof(uint32_t)))
+// TODO(Jfreegman): This should be the maximum size that an onion client can handle.
+#define MAX_FRIEND_REQUEST_DATA_SIZE (ONION_CLIENT_MAX_DATA_SIZE - 100)
 
 typedef struct Friend_Requests Friend_Requests;
 

toxcore/onion_client.h

@@ -183,6 +183,8 @@ non_null()
 unsigned int onion_getfriend_dht_pubkey(const Onion_Client *onion_c, int friend_num, uint8_t *dht_key);
 
 #define ONION_DATA_IN_RESPONSE_MIN_SIZE (CRYPTO_PUBLIC_KEY_SIZE + CRYPTO_MAC_SIZE)
+
+// TODO(Jfreegman): This is not the correct value; data this large will be dropped by the onion client.
 #define ONION_CLIENT_MAX_DATA_SIZE (MAX_DATA_REQUEST_SIZE - ONION_DATA_IN_RESPONSE_MIN_SIZE)
 
 /** @brief Send data of length length to friendnum.

toxcore/tox.h

@@ -274,7 +274,7 @@ uint32_t tox_max_status_message_length(void);
  *
  * @deprecated The macro will be removed in 0.3.0. Use the function instead.
  */
-#define TOX_MAX_FRIEND_REQUEST_LENGTH  1016
+#define TOX_MAX_FRIEND_REQUEST_LENGTH  921
 
 uint32_t tox_max_friend_request_length(void);