From 8b725c1ecc2c77215353e9005ae53da2f308dce1 Mon Sep 17 00:00:00 2001
From: jfreegman <jfreegman@gmail.com>
Date: Wed, 10 Nov 2021 12:00:18 -0500
Subject: [PATCH] Fix some friend connection issues

- Properly handle crypto_connection_status() failure
- Remove CONNECTION_UNKNOWN and restructure surrounding logic so that
  the API no longer reports erroneous friend connection statuses
---
 toxcore/Messenger.c  | 41 +++++++++++++++++++----------------------
 toxcore/Messenger.h  |  5 ++---
 toxcore/net_crypto.c |  4 +---
 3 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c
index 9d84e7a72d..79629ce169 100644
--- a/toxcore/Messenger.c
+++ b/toxcore/Messenger.c
@@ -444,26 +444,31 @@ int m_get_friend_connectionstatus(const Messenger *m, int32_t friendnumber)
         return -1;
     }
 
-    if (m->friendlist[friendnumber].status == FRIEND_ONLINE) {
-        bool direct_connected = 0;
-        unsigned int num_online_relays = 0;
-        int crypt_conn_id = friend_connection_crypt_connection_id(m->fr_c, m->friendlist[friendnumber].friendcon_id);
+    if (m->friendlist[friendnumber].status != FRIEND_ONLINE) {
+        return CONNECTION_NONE;
+    }
 
-        // FIXME(sudden6): handle return value
-        crypto_connection_status(m->net_crypto, crypt_conn_id, &direct_connected, &num_online_relays);
+    bool direct_connected = 0;
+    unsigned int num_online_relays = 0;
+    int crypt_conn_id = friend_connection_crypt_connection_id(m->fr_c, m->friendlist[friendnumber].friendcon_id);
 
-        if (direct_connected) {
-            return CONNECTION_UDP;
-        }
+    if (!crypto_connection_status(m->net_crypto, crypt_conn_id, &direct_connected, &num_online_relays)) {
+        return CONNECTION_NONE;
+    }
 
-        if (num_online_relays) {
-            return CONNECTION_TCP;
-        }
+    if (direct_connected) {
+        return CONNECTION_UDP;
+    }
 
-        return CONNECTION_UNKNOWN;
+    if (num_online_relays) {
+        return CONNECTION_TCP;
     }
 
-    return CONNECTION_NONE;
+    /* if we have a valid friend connection but do not have an established connection
+     * we leave the connection status unchanged until the friend connection is either
+     * established or dropped.
+     */
+    return m->friendlist[friendnumber].last_connection_udp_tcp;
 }
 
 int m_friend_exists(const Messenger *m, int32_t friendnumber)
@@ -899,14 +904,6 @@ static void check_friend_tcp_udp(Messenger *m, int32_t friendnumber, void *userd
         return;
     }
 
-    if (ret == CONNECTION_UNKNOWN) {
-        if (last_connection_udp_tcp == CONNECTION_UDP) {
-            return;
-        }
-
-        ret = CONNECTION_TCP;
-    }
-
     if (last_connection_udp_tcp != ret) {
         if (m->friend_connectionstatuschange) {
             m->friend_connectionstatuschange(m, friendnumber, ret, userdata);
diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h
index 31526b631d..b9b2f23f23 100644
--- a/toxcore/Messenger.h
+++ b/toxcore/Messenger.h
@@ -109,7 +109,6 @@ typedef enum Connection_Status {
     CONNECTION_NONE,
     CONNECTION_TCP,
     CONNECTION_UDP,
-    CONNECTION_UNKNOWN,
 } Connection_Status;
 
 /* USERSTATUS -
@@ -225,7 +224,7 @@ typedef struct Friend {
     uint32_t message_id; // a semi-unique id used in read receipts.
     uint32_t friendrequest_nospam; // The nospam number used in the friend request.
     uint64_t last_seen_time;
-    uint8_t last_connection_udp_tcp;
+    Connection_Status last_connection_udp_tcp;
     struct File_Transfers file_sending[MAX_CONCURRENT_FILE_PIPES];
     uint32_t num_sending_files;
     struct File_Transfers file_receiving[MAX_CONCURRENT_FILE_PIPES];
@@ -360,7 +359,7 @@ int getfriendcon_id(const Messenger *m, int32_t friendnumber);
  */
 int m_delfriend(Messenger *m, int32_t friendnumber);
 
-/* Checks friend's connecting status.
+/* Checks friend's connection status.
  *
  *  return CONNECTION_UDP (2) if friend is directly connected to us (Online UDP).
  *  return CONNECTION_TCP (1) if friend is connected to us (Online TCP).
diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c
index 0956ea769d..97383a059a 100644
--- a/toxcore/net_crypto.c
+++ b/toxcore/net_crypto.c
@@ -2921,9 +2921,7 @@ bool crypto_connection_status(const Net_Crypto *c, int crypt_connection_id, bool
 
         if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev4) > current_time) {
             *direct_connected = 1;
-        }
-
-        if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev6) > current_time) {
+        } else if ((UDP_DIRECT_TIMEOUT + conn->direct_lastrecv_timev6) > current_time) {
             *direct_connected = 1;
         }
     }