diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index a1091caf0..6c080f72b 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -5,7 +5,8 @@ ### Features Added - Add the static libcurl transport adapter. -- Add `NoSignal` option to the `curlTransportAdapter`. +- Add `NoSignal` option to the `CurlTransportAdapter`. +- Add `ConnectionTimeout` option to the `CurlTransportAdapter`. ### Breaking Changes diff --git a/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp b/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp index ff8db0aa4..ec526e4a2 100644 --- a/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp @@ -39,6 +39,13 @@ namespace Azure { namespace Core { namespace Http { */ struct CurlTransportOptions final { + /** + * @brief Default Maximum time in seconds that you allow the connection phase to the server to + * take. + * + */ + AZ_CORE_DLLEXPORT static const long DefaultConnectionTimeout = 300; + /** * @brief The string for the proxy is passed directly to the libcurl handle without any parsing * @@ -98,6 +105,15 @@ namespace Azure { namespace Core { namespace Http { * */ bool NoSignal = false; + + /** + * @brief Contain the maximum time in seconds that you allow the connection phase to the server + * to take. + * + * @details This only limits the connection phase, it has no impact once it has connected. + * + */ + long ConnectionTimeout = DefaultConnectionTimeout; }; /** diff --git a/sdk/core/azure-core/inc/azure/core/http/static_curl_transport.hpp b/sdk/core/azure-core/inc/azure/core/http/static_curl_transport.hpp index 9f7f89436..fedb70b91 100644 --- a/sdk/core/azure-core/inc/azure/core/http/static_curl_transport.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/static_curl_transport.hpp @@ -44,6 +44,14 @@ namespace Azure { namespace Core { namespace Http { */ struct StaticCurlTransportOptions final { + + /** + * @brief Default Maximum time in seconds that you allow the connection phase to the server to + * take. + * + */ + AZ_CORE_DLLEXPORT static const long DefaultConnectionTimeout = 300; + /** * @brief The string for the proxy is passed directly to the libcurl handle without any parsing * @@ -103,6 +111,15 @@ namespace Azure { namespace Core { namespace Http { * */ bool NoSignal = false; + + /** + * @brief Contain the maximum time in seconds that you allow the connection phase to the server + * to take. + * + * @details This only limits the connection phase, it has no impact once it has connected. + * + */ + long ConnectionTimeout = DefaultConnectionTimeout; }; /** diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index 4397e3742..0f76e65a9 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -1231,6 +1231,8 @@ inline std::string GetConnectionKey(std::string const& host, CurlTransportOption key.append(!options.SslOptions.EnableCertificateRevocationListCheck ? "1" : "0"); key.append(options.SslVerifyPeer ? "1" : "0"); key.append(options.NoSignal ? "1" : "0"); + key.append( + options.ConnectionTimeout == CurlTransportOptions::DefaultConnectionTimeout ? "0" : "1"); return key; } @@ -1267,6 +1269,7 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo // created and to discard all current connections in the pool for the host-index. A caller // might request this after getting broken/closed connections multiple-times. hostPoolIndex->second.clear(); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Reset connection pool requested."); } else { @@ -1283,6 +1286,7 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo g_curlConnectionPool.ConnectionPoolIndex.erase(hostPoolIndex); } + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Re-using connection from the pool."); // return connection ref return connection; } @@ -1292,6 +1296,7 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo // Creating a new connection is thread safe. No need to lock mutex here. // No available connection for the pool for the required host. Create one + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Spawn new connection."); CURL* newHandle = curl_easy_init(); if (!newHandle) { @@ -1333,6 +1338,17 @@ std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlCo + std::string(curl_easy_strerror(result))); } + if (options.ConnectionTimeout != CurlTransportOptions::DefaultConnectionTimeout) + { + if (!SetLibcurlOption(newHandle, CURLOPT_CONNECTTIMEOUT, options.ConnectionTimeout, &result)) + { + throw Azure::Core::Http::TransportException( + _detail::DefaultFailedToGetNewConnectionTemplate + host + + ". Fail setting connect timeout to: " + std::to_string(options.ConnectionTimeout) + ". " + + std::string(curl_easy_strerror(result))); + } + } + /******************** Curl handle options apply to all connections created * The keepAlive option is managed by the session directly. */ diff --git a/sdk/core/azure-core/src/http/curl/static_curl.cpp b/sdk/core/azure-core/src/http/curl/static_curl.cpp index 37d8c2a04..40c722147 100644 --- a/sdk/core/azure-core/src/http/curl/static_curl.cpp +++ b/sdk/core/azure-core/src/http/curl/static_curl.cpp @@ -300,6 +300,19 @@ public: } } + if (m_options.ConnectionTimeout + != Azure::Core::Http::StaticCurlTransportOptions::DefaultConnectionTimeout) + { + if (!SetStaticLibcurlOption( + m_libcurlHandle, CURLOPT_CONNECTTIMEOUT, m_options.ConnectionTimeout, &result)) + { + throw Azure::Core::Http::TransportException( + FailedToGetNewConnectionTemplate + host + + ". Fail setting connect timeout to: " + std::to_string(m_options.ConnectionTimeout) + + ". " + std::string(curl_easy_strerror(result))); + } + } + // headers sep-up auto const& headers = request.GetHeaders(); if (headers.size() > 0) diff --git a/sdk/core/azure-core/test/ut/curl_connection_pool_test.cpp b/sdk/core/azure-core/test/ut/curl_connection_pool_test.cpp index e04f35cdf..444d9be5a 100644 --- a/sdk/core/azure-core/test/ut/curl_connection_pool_test.cpp +++ b/sdk/core/azure-core/test/ut/curl_connection_pool_test.cpp @@ -45,7 +45,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::Request req( Azure::Core::Http::HttpMethod::Get, Azure::Core::Url(AzureSdkHttpbinServer::Get())); std::string const expectedConnectionKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "00110"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "001100"; { // Creating a new connection with default options @@ -114,7 +114,7 @@ namespace Azure { namespace Core { namespace Test { // Now test that using a different connection config won't re-use the same connection std::string const secondExpectedKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "00100"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "001000"; { // Creating a new connection with options Azure::Core::Http::CurlTransportOptions options; @@ -394,7 +394,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::Request req( Azure::Core::Http::HttpMethod::Get, Azure::Core::Url(authority)); std::string const expectedConnectionKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "00110"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "001100"; // Creating a new connection with default options auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool @@ -430,7 +430,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::Request req( Azure::Core::Http::HttpMethod::Get, Azure::Core::Url(authority)); std::string const expectedConnectionKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "44300110"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "443001100"; // Creating a new connection with default options auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool @@ -467,7 +467,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::Request req( Azure::Core::Http::HttpMethod::Get, Azure::Core::Url(authority)); std::string const expectedConnectionKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "00110"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "001100"; // Creating a new connection with default options auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool @@ -502,7 +502,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::Request req( Azure::Core::Http::HttpMethod::Get, Azure::Core::Url(authority)); std::string const expectedConnectionKey - = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "44300110"; + = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "443001100"; // Creating a new connection with default options auto connection = Azure::Core::Http::_detail::CurlConnectionPool::g_curlConnectionPool