diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index 972cc4fec..b1bb5344c 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -74,8 +74,10 @@ class CurlHttpOperation { void DispatchEvent(HttpStateEvent type) { - if(m_callback != nullptr) + if (m_callback != nullptr) + { m_callback->OnHttpStateEvent(type, static_cast(curl), 0); + } } std::atomic isAborted { false }; // Set to 'true' when async callback is aborted @@ -92,9 +94,11 @@ class CurlHttpOperation { std::string method, std::string url, IHttpResponseCallback* callback, - // Default empty headers and empty request body - const std::map& requestHeaders = std::map(), - const std::vector& requestBody = std::vector(), + // requestHeaders is copied into the curl_slist during construction + // and need not outlive this operation. requestBody is stored by + // reference and read by Send(), so it must outlive this operation. + const std::map& requestHeaders, + const std::vector& requestBody, // Default connectivity and response size options bool rawResponse = false, size_t httpConnTimeout = HTTP_CONN_TIMEOUT, @@ -109,9 +113,9 @@ class CurlHttpOperation { m_callback(callback), m_method(method), m_url(url), + m_sslCaInfo(sslCaInfo), // Local vars - requestHeaders(requestHeaders), requestBody(requestBody) { TRACE("--------------------------------------------------------------------------------------------------\n"); @@ -140,18 +144,18 @@ class CurlHttpOperation { curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, sslVerify ? 1L : 0L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, sslVerify ? 2L : 0L); - if (!sslCaInfo.empty()) { - curl_easy_setopt(curl, CURLOPT_CAINFO, sslCaInfo.c_str()); + if (!m_sslCaInfo.empty()) { + curl_easy_setopt(curl, CURLOPT_CAINFO, m_sslCaInfo.c_str()); } // HTTP/2 please, fallback to HTTP/1.1 if not supported curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0); - // Specify our custom headers - for(auto &kv : this->requestHeaders) + // Headers are copied into m_headersChunk during construction and the + // curl_slist is kept alive until destruction, so the original map does + // not need operation-lifetime storage. + for (const auto& kv : requestHeaders) { - std::string header = kv.first.c_str(); - header += ": "; - header += kv.second.c_str(); + std::string header = kv.first + ": " + kv.second; m_headersChunk = curl_slist_append(m_headersChunk, header.c_str()); } @@ -191,7 +195,7 @@ class CurlHttpOperation { ReleaseResponse(); // Request buffer - const void *request = (requestBody.empty())?NULL:&requestBody[0]; + const void *request = requestBody.empty() ? nullptr : requestBody.data(); const size_t reqSize = requestBody.size(); if(!curl) @@ -263,7 +267,7 @@ class CurlHttpOperation { { // POST curl_easy_setopt(curl, CURLOPT_POST, true); - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, (const char *)request); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, static_cast(request)); curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, reqSize); } else if (m_method.compare("GET") == 0) @@ -340,18 +344,20 @@ class CurlHttpOperation { } /** - * Return a copy of resposne headers + * Return a copy of response headers * * @return */ std::map GetResponseHeaders() { std::map result; - if (respHeaders.size() == 0) + if (respHeaders.empty()) + { return result; + } std::stringstream ss; - std::string headers((const char *)&respHeaders[0], respHeaders.size()); + std::string headers(reinterpret_cast(respHeaders.data()), respHeaders.size()); ss.str(headers); std::string header; @@ -382,8 +388,11 @@ class CurlHttpOperation { std::vector GetRawResponse() { std::vector result; - if ((response.memory!=nullptr)&&(response.size!=0)) - result.insert(result.end(), (const char *)response.memory, ((const char *)response.memory) + response.size); + if ((response.memory != nullptr) && (response.size != 0)) + { + const auto* begin = reinterpret_cast(response.memory); + result.insert(result.end(), begin, begin + response.size); + } return result; } @@ -392,7 +401,7 @@ class CurlHttpOperation { */ void ReleaseResponse() { - if (response.memory!=nullptr) { + if (response.memory != nullptr) { free(response.memory); response.memory = nullptr; response.size = 0; @@ -434,7 +443,11 @@ class CurlHttpOperation { // Request values std::string m_method; std::string m_url; - const std::map& requestHeaders; + std::string m_sslCaInfo; + // The SDK upload path keeps the owning IHttpRequest alive through the + // callback context until Send() completes; copying this body would duplicate + // every upload payload. Unlike CURLOPT_CAINFO, the body pointer is set and + // consumed during Send(), not retained from construction. const std::vector& requestBody; struct curl_slist *m_headersChunk = nullptr; @@ -491,12 +504,13 @@ class CurlHttpOperation { size_t realsize = size * nmemb; struct MemoryStruct *mem = (struct MemoryStruct *)userp; - mem->memory = (char *)(realloc(mem->memory, mem->size + realsize + 1)); - if(mem->memory == NULL) { + auto* memory = static_cast(realloc(mem->memory, mem->size + realsize + 1)); + if(memory == nullptr) { /* out of memory! */ TRACE("not enough memory (realloc returned NULL)\n"); return 0; } + mem->memory = memory; #ifdef HAVE_ONEDS_BOUNDCHECK_METHODS BoundCheckFunctions::oneds_memcpy_s(&(mem->memory[mem->size]), realsize, contents, realsize); #else @@ -519,9 +533,9 @@ class CurlHttpOperation { */ static size_t WriteVectorCallback(void *ptr, size_t size, size_t nmemb, std::vector* data) { - if (data!=nullptr) { - const unsigned char * begin = (unsigned char *)(ptr); - const unsigned char * end = begin + size * nmemb; + if (data != nullptr) { + const auto* begin = static_cast(ptr); + const auto* end = begin + size * nmemb; data->insert( data->end(), begin, end); } return size * nmemb; @@ -534,4 +548,3 @@ class CurlHttpOperation { #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT #endif // HTTPCLIENTCURL_HPP - diff --git a/tests/unittests/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index 889d2ffe7..c9894b90d 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/tests/unittests/HttpClientCurlTests.cpp @@ -19,20 +19,23 @@ class HttpClientCurlTests : public ::testing::Test { protected: HttpClient_Curl m_client; + // Named, fixture-lifetime values so they outlive each CurlHttpOperation. + const std::map m_headers; + const std::vector m_body; }; // --- SetSslVerification wiring --- TEST_F(HttpClientCurlTests, SslVerification_DefaultsToTrue) { - CurlHttpOperation op("GET", "https://example.com", nullptr); + CurlHttpOperation op("GET", "https://example.com", nullptr, m_headers, m_body); ASSERT_NE(op.GetHandle(), nullptr); } TEST_F(HttpClientCurlTests, CurlHttpOperation_ConstructsWithVerifyTrue) { CurlHttpOperation op("GET", "https://example.com", nullptr, - std::map(), std::vector(), + m_headers, m_body, false, 5, true, ""); ASSERT_NE(op.GetHandle(), nullptr); } @@ -40,7 +43,7 @@ TEST_F(HttpClientCurlTests, CurlHttpOperation_ConstructsWithVerifyTrue) TEST_F(HttpClientCurlTests, CurlHttpOperation_ConstructsWithVerifyFalse) { CurlHttpOperation op("GET", "https://example.com", nullptr, - std::map(), std::vector(), + m_headers, m_body, false, 5, false, ""); ASSERT_NE(op.GetHandle(), nullptr); } @@ -48,7 +51,7 @@ TEST_F(HttpClientCurlTests, CurlHttpOperation_ConstructsWithVerifyFalse) TEST_F(HttpClientCurlTests, CurlHttpOperation_ConstructsWithCaInfo) { CurlHttpOperation op("GET", "https://example.com", nullptr, - std::map(), std::vector(), + m_headers, m_body, false, 5, true, "/etc/ssl/certs/ca-certificates.crt"); ASSERT_NE(op.GetHandle(), nullptr); }