From 1367e825f986c2fbac6cb3b9054ae20137bf86e5 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Thu, 4 Jun 2026 19:37:02 -0500 Subject: [PATCH 1/5] Make Curl CA path ownership safe Split the Curl CA path fix into a smaller review slice. Store CA path strings with stable ownership so Curl options do not keep pointers to transient or externally-owned string data. Files changed: lib/http/HttpClient_Curl.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Curl.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index 972cc4fec..c7a5bdecb 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -109,6 +109,7 @@ class CurlHttpOperation { m_callback(callback), m_method(method), m_url(url), + m_sslCaInfo(sslCaInfo), // Local vars requestHeaders(requestHeaders), @@ -140,8 +141,8 @@ 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); @@ -434,6 +435,7 @@ class CurlHttpOperation { // Request values std::string m_method; std::string m_url; + std::string m_sslCaInfo; const std::map& requestHeaders; const std::vector& requestBody; struct curl_slist *m_headersChunk = nullptr; @@ -534,4 +536,3 @@ class CurlHttpOperation { #endif // HAVE_MAT_DEFAULT_HTTP_CLIENT #endif // HTTPCLIENTCURL_HPP - From efcae8967a70e6c93c45fedf55354a57af0f22d8 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 5 Jun 2026 10:34:33 -0500 Subject: [PATCH 2/5] Document Curl request body lifetime Explain why CurlHttpOperation owns the CA path string but intentionally does not copy the request body. The SDK upload path keeps the request alive until Send completes, and copying the body would duplicate every upload payload. Files changed: lib/http/HttpClient_Curl.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Curl.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index c7a5bdecb..c88a4d0cc 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -436,6 +436,10 @@ class CurlHttpOperation { std::string m_method; std::string m_url; 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::map& requestHeaders; const std::vector& requestBody; struct curl_slist *m_headersChunk = nullptr; From b73be49b7f161b3bc7e8750b6be9f7398a97b5bc Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 5 Jun 2026 10:50:29 -0500 Subject: [PATCH 3/5] Document Curl header lifetime Explain that request headers are copied into the owned curl_slist during construction, so CurlHttpOperation does not need to keep the original header map alive for the operation lifetime. Files changed: lib/http/HttpClient_Curl.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Curl.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index c88a4d0cc..5b6c0447a 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -112,7 +112,6 @@ class CurlHttpOperation { m_sslCaInfo(sslCaInfo), // Local vars - requestHeaders(requestHeaders), requestBody(requestBody) { TRACE("--------------------------------------------------------------------------------------------------\n"); @@ -147,8 +146,10 @@ class CurlHttpOperation { // 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(auto &kv : requestHeaders) { std::string header = kv.first.c_str(); header += ": "; @@ -440,7 +441,6 @@ class CurlHttpOperation { // 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::map& requestHeaders; const std::vector& requestBody; struct curl_slist *m_headersChunk = nullptr; From f7901b20bd6f001e51ff0c2a8266e4c1c02eabea Mon Sep 17 00:00:00 2001 From: bmehta001 Date: Fri, 5 Jun 2026 15:26:54 -0500 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Baiju Meswani --- lib/http/HttpClient_Curl.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index 5b6c0447a..e2756ec04 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -149,11 +149,11 @@ class CurlHttpOperation { // 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(auto &kv : requestHeaders) + for(const auto &kv : requestHeaders) { std::string header = kv.first.c_str(); header += ": "; - header += kv.second.c_str(); +const std::string header = kv.first + ":" + kv.second; m_headersChunk = curl_slist_append(m_headersChunk, header.c_str()); } From a9faff60f23c9048739975a9f3f39db426e6e621 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Fri, 5 Jun 2026 15:41:54 -0500 Subject: [PATCH 5/5] Clean up Curl operation internals Apply the reviewed header-loop cleanup and modernize a few local internals in HttpClient_Curl.hpp without changing behavior. This preserves the existing request-body lifetime model while reducing unnecessary casts/null usage and avoiding loss of the old response buffer if realloc fails. Files changed: lib/http/HttpClient_Curl.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/http/HttpClient_Curl.hpp | 42 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index e2756ec04..126a2c4ce 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 @@ -149,11 +151,9 @@ class CurlHttpOperation { // 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) + for (const auto& kv : requestHeaders) { - std::string header = kv.first.c_str(); - header += ": "; -const std::string header = kv.first + ":" + kv.second; + std::string header = kv.first + ": " + kv.second; m_headersChunk = curl_slist_append(m_headersChunk, header.c_str()); } @@ -193,7 +193,7 @@ const std::string header = kv.first + ":" + kv.second; 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) @@ -265,7 +265,7 @@ const std::string header = kv.first + ":" + kv.second; { // 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) @@ -342,18 +342,20 @@ const std::string header = kv.first + ":" + kv.second; } /** - * 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; @@ -384,8 +386,11 @@ const std::string header = kv.first + ":" + kv.second; 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; } @@ -394,7 +399,7 @@ const std::string header = kv.first + ":" + kv.second; */ void ReleaseResponse() { - if (response.memory!=nullptr) { + if (response.memory != nullptr) { free(response.memory); response.memory = nullptr; response.size = 0; @@ -497,12 +502,13 @@ const std::string header = kv.first + ":" + kv.second; 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 @@ -525,9 +531,9 @@ const std::string header = kv.first + ":" + kv.second; */ 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;