From: Marcin Siodelski <marcin@isc.org>
Date: Thu, 20 Aug 2020 11:23:22 +0000 (+0200)
Subject: [#1390] Http connection closed on timeout
X-Git-Tag: Kea-1.8.0~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9261ac07fe0f2b1958e9e4a913204e66fdf44d55;p=thirdparty%2Fkea.git

[#1390] Http connection closed on timeout

When the client gets the timeout trying to communicate with the server it
now closes connection and re-establishes it when next request is to be
sent.
---

diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc
index f66f4b3079..63dd8ca1e3 100644
--- a/src/lib/http/client.cc
+++ b/src/lib/http/client.cc
@@ -968,9 +968,9 @@ Connection::terminateInternal(const boost::system::error_code& ec,
         } catch (...) {
         }
 
-        // If we're not requesting connection persistence, we should close the socket.
-        // We're going to reconnect for the next transaction.
-        if (!current_request_->isPersistent()) {
+        // If we're not requesting connection persistence or the connection has timed out,
+        // we should close the socket. We're going to reconnect for the next transaction.
+        if (!current_request_->isPersistent() || (ec == boost::asio::error::timed_out)) {
             closeInternal();
         }
 
diff --git a/src/lib/http/tests/server_client_unittests.cc b/src/lib/http/tests/server_client_unittests.cc
index fa91b150de..6717128a5f 100644
--- a/src/lib/http/tests/server_client_unittests.cc
+++ b/src/lib/http/tests/server_client_unittests.cc
@@ -1480,20 +1480,29 @@ public:
         // rest of the response to be provided and will eventually time out.
         PostHttpRequestJsonPtr request1 = createRequest("partial-response", true);
         HttpResponseJsonPtr response1(new HttpResponseJson());
+        // This value will be set to true if the connection close callback is
+        // invoked upon time out.
+        auto connection_closed = false;
         ASSERT_NO_THROW(client.asyncSendRequest(url, request1, response1,
             [this, &cb_num](const boost::system::error_code& ec,
                             const HttpResponsePtr& response,
                             const std::string&) {
-            if (++cb_num > 1) {
-                io_service_.stop();
-            }
-            // In this particular case we know exactly the type of the
-            // IO error returned, because the client explicitly sets this
-            // error code.
-            EXPECT_TRUE(ec.value() == boost::asio::error::timed_out);
-            // There should be no response returned.
-            EXPECT_FALSE(response);
-        }, HttpClient::RequestTimeout(100)));
+                if (++cb_num > 1) {
+                    io_service_.stop();
+                }
+                // In this particular case we know exactly the type of the
+                // IO error returned, because the client explicitly sets this
+                // error code.
+                EXPECT_TRUE(ec.value() == boost::asio::error::timed_out);
+                // There should be no response returned.
+                EXPECT_FALSE(response);
+            }, HttpClient::RequestTimeout(100), HttpClient::ConnectHandler(),
+            [&connection_closed](const int) {
+                // This callback is called when the connection gets closed
+                // by the client.
+                connection_closed = true;
+            })
+        );
 
         // Create another request after the timeout. It should be handled ok.
         PostHttpRequestJsonPtr request2 = createRequest("sequence", 1);
@@ -1509,6 +1518,8 @@ public:
 
         // Actually trigger the requests.
         ASSERT_NO_THROW(runIOService());
+        // Make sure that the client has closed the connection upon timeout.
+        EXPECT_TRUE(connection_closed);
     }
 
     /// @brief Test that client times out when connection takes too long.
@@ -1527,9 +1538,9 @@ public:
         PostHttpRequestJsonPtr request = createRequest("sequence", 1);
         HttpResponseJsonPtr response(new HttpResponseJson());
         ASSERT_NO_THROW(client.asyncSendRequest(url, request, response,
-            [this, &cb_num](const boost::system::error_code& ec,
-                            const HttpResponsePtr& response,
-                            const std::string&) {
+            [this, &cb_num, &client](const boost::system::error_code& ec,
+                                     const HttpResponsePtr& response,
+                                     const std::string&) {
             if (++cb_num > 1) {
                 io_service_.stop();
             }