Curl keep alive

sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp

@@ -13,9 +13,14 @@
 #include "azure/core/nullable.hpp"
 
 #include <chrono>
+#include <ctime>
 #include <memory>
 #include <string>
-
+#if defined(_azure_TESTING_BUILD)
+namespace Azure { namespace Core { namespace Test {
+  class CurlTransport_VerifyKeepAliveHeaders_Test;
+}}} // namespace Azure::Core::Test
+#endif
 namespace Azure { namespace Core { namespace Http {
   class CurlNetworkConnection;
 
@@ -26,6 +31,32 @@ namespace Azure { namespace Core { namespace Http {
      *
      */
     constexpr std::chrono::milliseconds DefaultConnectionTimeout = std::chrono::minutes(5);
+
+    /**
+     * @brief The configuration options for the keep-alive feature.
+     *
+     * @remark The keep-alive feature allows the SDK to reuse the same connection to the service for
+     * multiple requests.
+     */
+    class KeepAliveOptions {
+    public:
+      /**
+       * @brief The time in seconds that the host will allow an idle connection to remain open
+       * before it is closed.
+       *
+       * @remark A connection is idle if no data is sent or received by a host. A host
+       * may keep an idle connection open for longer than timeout seconds, but the host should
+       * attempt to retain a connection for at least timeout seconds.
+       *
+       */
+      std::chrono::seconds ConnectionTimeout = std::chrono::seconds(0);
+
+      /**
+       * @brief The maximum number of requests that a host will allow over a single connection.
+       *
+       */
+      std::size_t MaxRequests = size_t(1);
+    };
   } // namespace _detail
 
   /**
@@ -151,6 +182,15 @@ namespace Azure { namespace Core { namespace Http {
      */
     bool HttpKeepAlive = true;
 
+    /**
+     * @brief Options specified in the keep-alive request header
+     *
+     * @remark The keep-alive feature allows the SDK to reuse the same connection to the service for
+     * multiple requests. This field is populated if the Keep-Alive header is present in the
+     * request.
+     */
+    Azure::Nullable<Azure::Core::Http::_detail::KeepAliveOptions> KeepAliveOptions;
+
     /**
      * @brief This option determines whether libcurl verifies the authenticity of the peer's
      * certificate.
@@ -200,6 +240,9 @@ namespace Azure { namespace Core { namespace Http {
    * @brief Concrete implementation of an HTTP Transport that uses libcurl.
    */
   class CurlTransport : public HttpTransport {
+#if defined(_azure_TESTING_BUILD)
+    friend class Azure::Core::Test::CurlTransport_VerifyKeepAliveHeaders_Test;
+#endif
   private:
     CurlTransportOptions m_options;
 
@@ -209,6 +252,8 @@ namespace Azure { namespace Core { namespace Http {
      */
     virtual void OnUpgradedConnection(std::unique_ptr<CurlNetworkConnection>&&){};
 
+    void ValidateKeepAliveHeaders(Request& request, std::unique_ptr<RawResponse>& response);
+
   public:
     /**
      * @brief Construct a new CurlTransport object.

sdk/core/azure-core/src/http/curl/curl.cpp

@@ -403,9 +403,48 @@ std::unique_ptr<RawResponse> CurlTransport::Send(Request& request, Context const
   auto response = session->ExtractResponse();
   // Move the ownership of the CurlSession (bodyStream) to the response
   response->SetBodyStream(std::move(session));
+  // check the consistency of the keep alive headers between the request and the response
+  // as suggested by RFC 7230 section 6.3 (https://tools.ietf.org/html/rfc7230#section-6.3)
+  ValidateKeepAliveHeaders(request, response);
+
   return response;
 }
 
+// Check if the server supports keep alive and if the headers are consistent between the request and
+// the response. If they are not consistent, the keep alive header in the request is removed and the
+// calculated options are reset in order to prevent the wrongful caching of the keep alive settings.
+void CurlTransport::ValidateKeepAliveHeaders(
+    Request& request,
+    std::unique_ptr<RawResponse>& response)
+{
+  // if the server supports keep alive the headers should be present in the response. If they are
+  // they should be the same as the request headers.
+  if (response->GetHeaders().find("Connection") != response->GetHeaders().end()
+      && request.GetHeader("Connection").HasValue()
+      && response->GetHeaders().find("Keep-Alive") != response->GetHeaders().end()
+      && request.GetHeader("Keep-Alive").HasValue()
+      // Case sensitivity only applies to the `Key` in the map. Thus. compare `Value` in a case insensitive manor.
+      && Azure::Core::_internal::StringExtensions::ToLower(
+             response->GetHeaders().find("Connection")->second)
+          == Azure::Core::_internal::StringExtensions::ToLower(
+              request.GetHeader("Connection").Value())
+      // just in case the server sends the keep-alive header in a different case
+      && Azure::Core::_internal::StringExtensions::ToLower(
+             response->GetHeaders().find("Keep-Alive")->second)
+          == Azure::Core::_internal::StringExtensions::ToLower(
+              request.GetHeader("Keep-Alive").Value()))
+  {
+    Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Response has same keep-alive settings");
+  }
+  else
+  {
+    // cleanup keep-alive header in the request since they don't match up the response from
+    // the server.
+    request.RemoveHeader("Keep-Alive");
+    m_options.KeepAliveOptions.Reset();
+  }
+}
+
 CURLcode CurlSession::Perform(Context const& context)
 {
   // Set the session state
@@ -2220,10 +2259,18 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
         {
           g_curlConnectionPool.ConnectionPoolIndex.erase(hostPoolIndex);
         }
-
-        Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Re-using connection from the pool.");
-        // return connection ref
-        return connection;
+        // if the connection is expired do not return it, let the code flow and return a new one.
+        if (!connection->IsKeepAliveExpired())
+        {
+          connection->IncreaseUsageCount();
+          Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Re-using connection from the pool.");
+          // return connection ref
+          return connection;
+        }
+        else
+        {
+          Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Connection expired. Discarding.");
+        }
       }
     }
   }
@@ -2235,6 +2282,58 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
   return std::make_unique<CurlConnection>(request, options, hostDisplayName, connectionKey);
 }
 
+Azure::Core::Http::_detail::KeepAliveOptions CurlConnection::ParseKeepAliveHeader(
+    std::string const& keepAlive)
+{
+  // Parse the Keep-Alive header to determine if the connection should be kept alive.
+  // The Keep-Alive header is in the format of:
+  // Keep-Alive: timeout=5, max=1000
+  // The timeout is the number of seconds the connection is allowed to be idle before it is closed.
+  // The max is the maximum number of requests that can be made on the connection before it is
+  // closed.
+  // If the header is not present, the connection will be kept alive for the lifetime of the
+  // application.
+  Azure::Core::Http::_detail::KeepAliveOptions keepAliveOptions;
+  std::string const timeoutKey = "timeout=";
+  std::string const maxKey = "max=";
+  auto timeoutPos = keepAlive.find(timeoutKey);
+  auto maxPos = keepAlive.find(maxKey);
+  try
+  {
+
+    if (timeoutPos != std::string::npos)
+    {
+      auto timeoutEnd = keepAlive.find(',', timeoutPos);
+      if (timeoutEnd == std::string::npos)
+      {
+        timeoutEnd = keepAlive.size();
+      }
+
+      keepAliveOptions.ConnectionTimeout = std::chrono::seconds(std::stoi(keepAlive.substr(
+          timeoutPos + timeoutKey.size(), timeoutEnd - timeoutPos - timeoutKey.size())));
+    }
+
+    if (maxPos != std::string::npos)
+    {
+      auto maxEnd = keepAlive.find(',', maxPos);
+      if (maxEnd == std::string::npos)
+      {
+        maxEnd = keepAlive.size();
+      }
+      keepAliveOptions.MaxRequests
+          = std::stoi(keepAlive.substr(maxPos + maxKey.size(), maxEnd - maxPos - maxKey.size()));
+    }
+  }
+  catch (std::invalid_argument const&)
+  {
+    Log::Write(
+        Logger::Level::Error,
+        "Failed to parse max value / timeout from Keep-Alive header: " + keepAlive);
+    return Azure::Core::Http::_detail::KeepAliveOptions();
+  }
+  return keepAliveOptions;
+}
+
 // Move the connection back to the connection pool. Push it to the front so it becomes the
 // first connection to be picked next time some one ask for a connection to the pool (LIFO)
 void CurlConnectionPool::MoveConnectionBackToPool(
@@ -2246,9 +2345,9 @@ void CurlConnectionPool::MoveConnectionBackToPool(
     return; // The server has asked us to not re-use this connection.
   }
 
-  if (connection->IsShutdown())
+  if (connection->IsShutdown() || connection->IsKeepAliveExpired())
   {
-    // Can't re-used a shut down connection
+    // Can't re-use a shutdown connection or an expired connection
     return;
   }
 
@@ -2309,8 +2408,29 @@ CurlConnection::CurlConnection(
         _detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
         + std::string("curl_easy_init returned Null"));
   }
+
   CURLcode result;
 
+  if (request.GetHeader("connection").HasValue() && request.GetHeader("keep-alive").HasValue())
+  {
+    auto connectionHeader = Azure::Core::_internal::StringExtensions::ToLower(
+        request.GetHeader("connection").Value());
+    auto keepAliveHeader = Azure::Core::_internal::StringExtensions::ToLower(
+        request.GetHeader("keep-alive").Value());
+
+    if (connectionHeader == "keep-alive")
+    {
+      auto keepAliveValue = ParseKeepAliveHeader(keepAliveHeader);
+      // if we have issues parsing this header , or the data in the header is invalid no point in
+      // setting it up
+      if (keepAliveValue.ConnectionTimeout != std::chrono::seconds(0)
+          || keepAliveValue.MaxRequests > 0)
+      {
+        m_keepAliveOptions = keepAliveValue;
+      }
+    }
+  }
+
   if (options.EnableCurlTracing)
   {
     if (!SetLibcurlOption(

sdk/core/azure-core/src/http/curl/curl_connection_private.hpp

@@ -29,7 +29,11 @@
 
 /// From openssl/x509.h.  Avoids needing to include openssl headers
 typedef struct x509_store_ctx_st X509_STORE_CTX;
-
+#if defined(_azure_TESTING_BUILD)
+namespace Azure { namespace Core { namespace Test {
+  class CurlConnectionTest_ParseKeepAliveHeader_Test;
+}}} // namespace Azure::Core::Test
+#endif
 namespace Azure { namespace Core {
   namespace _detail {
     /**
@@ -105,6 +109,12 @@ namespace Azure { namespace Core {
        */
       virtual bool IsExpired() = 0;
 
+      /**
+       * @brief Checks whether this CURL connection is expired due to keep-alive settings.
+       *
+       */
+      virtual bool IsKeepAliveExpired() { return false; };
+
       /**
        * @brief This function is used when working with streams to pull more data from the wire.
        * Function will try to keep pulling data from socket until the buffer is all written or until
@@ -134,22 +144,40 @@ namespace Azure { namespace Core {
        * @return `true` is the connection was shut it down; otherwise, `false`.
        */
       bool IsShutdown() const { return m_isShutDown; }
+
+      /**
+       * @brief Increase the usage count.
+       *
+       */
+      virtual void IncreaseUsageCount(){};
+
+      /**
+       * @brief Get the connection usage count.
+       *
+       * @return The usage count
+       */
+      virtual size_t GetUsageCount() const { return 0; };
     };
 
     /**
      * @brief CURL HTTP connection.
      *
      */
     class CurlConnection final : public CurlNetworkConnection {
+#if defined(_azure_TESTING_BUILD)
+      friend class Azure::Core::Test::CurlConnectionTest_ParseKeepAliveHeader_Test;
+#endif
     private:
       Azure::Core::_internal::UniqueHandle<CURL> m_handle;
       curl_socket_t m_curlSocket;
       std::chrono::steady_clock::time_point m_lastUseTime;
+      std::chrono::steady_clock::time_point m_firstUseTime = std::chrono::steady_clock::now();
       std::string m_connectionKey;
       // CRL validation is disabled by default to be consistent with WinHTTP behavior
       bool m_enableCrlValidation{false};
       // Allow the connection to proceed if retrieving the CRL failed.
       bool m_allowFailedCrlRetrieval{true};
+      size_t m_usedCount = size_t(1);
 
       static int CurlLoggingCallback(
           CURL* handle,
@@ -161,6 +189,9 @@ namespace Azure { namespace Core {
       static int CurlSslCtxCallback(CURL* curl, void* sslctx, void* parm);
       int SslCtxCallback(CURL* curl, void* sslctx);
       int VerifyCertificateError(int ok, X509_STORE_CTX* storeContext);
+      Azure::Nullable<Azure::Core::Http::_detail::KeepAliveOptions> m_keepAliveOptions;
+      Azure::Core::Http::_detail::KeepAliveOptions ParseKeepAliveHeader(
+          std::string const& keepAlive);
 
     public:
       /**
@@ -203,7 +234,34 @@ namespace Azure { namespace Core {
       {
         auto connectionOnWaitingTimeMs = std::chrono::duration_cast<std::chrono::milliseconds>(
             std::chrono::steady_clock::now() - this->m_lastUseTime);
-        return connectionOnWaitingTimeMs.count() >= _detail::DefaultConnectionExpiredMilliseconds;
+        return connectionOnWaitingTimeMs.count() >= _detail::DefaultConnectionExpiredMilliseconds
+            || IsKeepAliveExpired();
+      }
+
+      /**
+       * @brief Checks whether this CURL connection is expired due to keep-alive settings.
+       * @return `true` if this connection is considered expired; otherwise, `false`.
+       */
+      bool IsKeepAliveExpired() override
+      {
+        // if we have keep alive options and we haven reached the max requests declare expired
+        if (m_keepAliveOptions.HasValue() && m_keepAliveOptions.Value().MaxRequests > 0
+            && m_keepAliveOptions.Value().MaxRequests <= m_usedCount)
+        {
+          return true;
+        }
+
+        // if we have keep alive options and we have a connection timeout and the connection time
+        // frame has passed declare expired
+        if (m_keepAliveOptions.HasValue()
+            && m_keepAliveOptions.Value().ConnectionTimeout > std::chrono::seconds(0)
+            && m_firstUseTime + m_keepAliveOptions.Value().ConnectionTimeout
+                < std::chrono::steady_clock::now())
+        {
+          return true;
+        }
+
+        return false;
       }
 
       /**
@@ -231,6 +289,19 @@ namespace Azure { namespace Core {
        */
       CURLcode SendBuffer(uint8_t const* buffer, size_t bufferSize, Context const& context)
           override;
+
+      /**
+       * @brief Increase the usage count.
+       *
+       */
+      void IncreaseUsageCount() override { m_usedCount++; };
+
+      /**
+       * @brief Get the connection usage count.
+       *
+       * @return The usage count
+       */
+      size_t GetUsageCount() const override { return m_usedCount; }
     };
   } // namespace Http
 }} // namespace Azure::Core

sdk/core/azure-core/test/ut/CMakeLists.txt

@@ -30,7 +30,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED True)
 if(BUILD_TRANSPORT_CURL)
   SET(CURL_OPTIONS_TESTS curl_options_test.cpp)
   SET(CURL_SESSION_TESTS curl_session_test_test.cpp curl_session_test.hpp)
-  SET(CURL_CONNECTION_POOL_TESTS curl_connection_pool_test.cpp)
+  SET(CURL_CONNECTION_POOL_TESTS curl_connection_pool_test.cpp curl_connection_tests.cpp)
 endif()
 
 if(RUN_LONG_UNIT_TESTS)