From bd8174ead319e1e3ce367c756398092b3641c6ac Mon Sep 17 00:00:00 2001
From: Daniel Eisenberg <danielei@checkpoint.com>
Date: Mon, 17 Feb 2025 12:20:20 +0200
Subject: [PATCH] fix connection

---
 core/messaging/connection/connection.cc      | 43 ++++----------------
 core/messaging/connection/connection_comp.cc |  2 +-
 2 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/core/messaging/connection/connection.cc b/core/messaging/connection/connection.cc
index df5a272..2be34e1 100644
--- a/core/messaging/connection/connection.cc
+++ b/core/messaging/connection/connection.cc
@@ -184,18 +184,6 @@ public:
     establishConnection()
     {
         dbgFlow(D_CONNECTION) << "Establishing a new connection";
-        // check if connection already established
-        I_MainLoop *i_mainloop = Singleton::Consume<I_MainLoop>::by<Messaging>();
-        while (lock) {
-            i_mainloop->yield(true);
-        }
-        lock = true;
-        auto unlock = make_scope_exit([&] () { lock = false; });
-
-        if (is_connected && !should_close_connection) {
-            dbgTrace(D_CONNECTION) << "Connection already established";
-            return Maybe<void>();
-        }
         auto set_socket = setSocket();
         if (!set_socket.ok()) {
             dbgWarning(D_CONNECTION) << "Failed to set socket: " << set_socket.getErr();
@@ -233,7 +221,6 @@ public:
             << (isOverProxy() ? ", Over proxy: " + settings.getProxyHost() + ":" + to_string(key.getPort()) : "");
         active = Maybe<void, chrono::seconds>();
         should_close_connection = false;
-        is_connected = true;
         return Maybe<void>();
     }
 
@@ -596,13 +583,11 @@ private:
 
         if (BIO_should_retry(bio.get())) return string();
 
-        auto fd = BIO_get_fd(bio.get(), nullptr);
-
         char error_buf[256];
         ERR_error_string(ERR_get_error(), error_buf);
         string error = receive_len == 0 ?
-            "Connection closed by peer (BIO fd: " + to_string(fd) + "). Error: " + string(error_buf) :
-            "Failed to read data from BIO socket (fd: " + to_string(fd) + "). Error: " + string(error_buf);
+            "Connection closed by peer" :
+            "Failed to read data from BIO socket. Error: " + string(error_buf);
         dbgWarning(D_CONNECTION) << error;
         return genError(HTTPResponse(HTTPStatusCode::HTTP_UNKNOWN, error));
     }
@@ -636,28 +621,17 @@ private:
     Maybe<HTTPResponse, HTTPResponse>
     sendAndReceiveData(const string &request, bool is_connect)
     {
-        dbgFlow(D_CONNECTION) << "Sending and receiving data, lock: " << lock;
+        dbgFlow(D_CONNECTION) << "Sending and receiving data";
         I_MainLoop *i_mainloop = Singleton::Consume<I_MainLoop>::by<Messaging>();
-        while (lock && !is_connect) {
+        while (lock) {
             i_mainloop->yield(true);
         }
         lock = true;
-        dbgTrace(D_CONNECTION) << "acquire lock";
-        auto unlock = make_scope_exit([&] () {
-            lock = false;
-        });
+        auto unlock = make_scope_exit([&] () { lock = false; });
 
         if (should_close_connection) {
-            dbgTrace(D_CONNECTION) << "reconnect in progress";
-            while (lock) {
-                i_mainloop->yield(true);
-            }
-            if (!is_connected) {
-                dbgWarning(D_CONNECTION) << close_error.getBody();
-                return genError(close_error);
-            }
-            dbgTrace(D_CONNECTION) << "reconnected by other routine";
-            lock = true;
+            dbgWarning(D_CONNECTION) << close_error.getBody();
+            return genError(close_error);
         }
 
         I_TimeGet *i_time = Singleton::Consume<I_TimeGet>::by<Messaging>();
@@ -677,13 +651,11 @@ private:
         dbgTrace(D_CONNECTION) << "Sent the message, now waiting for response";
         while (!http_parser.hasReachedError()) {
             if (i_time->getMonotonicTime() > receiving_end_time) {
-                is_connected = false;
                 should_close_connection = true;
                 return genError(receving_timeout);
             };
             auto receieved = receiveData();
             if (!receieved.ok()) {
-                is_connected = false;
                 should_close_connection = true;
                 return receieved.passErr();
             }
@@ -734,7 +706,6 @@ private:
     bool lock = false;
     bool should_close_connection = false;
     bool is_dual_auth = false;
-    bool is_connected = false;
     Maybe<string> sni_hostname = genError<string>("Uninitialized");
     Maybe<string> dn_host_name = genError<string>("Uninitialized");
 
diff --git a/core/messaging/connection/connection_comp.cc b/core/messaging/connection/connection_comp.cc
index 44111c5..a033d79 100644
--- a/core/messaging/connection/connection_comp.cc
+++ b/core/messaging/connection/connection_comp.cc
@@ -92,12 +92,12 @@ private:
             << metadata.getPort();
         MessageConnectionKey conn_key(metadata.getHostName(), metadata.getPort(), category);
         Connection conn(conn_key, metadata);
-        persistent_connections.emplace(conn_key, conn);
 
         const auto &external_certificate = metadata.getExternalCertificate();
         if (!external_certificate.empty()) conn.setExternalCertificate(external_certificate);
 
         auto connected = conn.establishConnection();
+        persistent_connections.emplace(conn_key, conn);
 
         if (!connected.ok()) {
             string connection_err = "Failed to establish connection. Error: " + connected.getErr();