commit 6ac5cb94d5d2a03a3be45617c5baee115e166ab6
author Marton Szasz <szaszm@apache.org> Fri Sep 01 01:00:38 2023 +0200
committer Ferenc Gerlits <fgerlits@gmail.com> Wed Sep 20 17:31:38 2023 +0200
tree 38331fdb06255b802cfed60157b43327fc63d7be
parent a328a01d288d021a19023facd79a76b2a7d2709f

MINIFICPP-2209 Small HTTPClient refactor: preconditions and exception safety

Signed-off-by: Ferenc Gerlits <fgerlits@gmail.com>
This closes #1648

extensions/http-curl/client/HTTPClient.cpp

diff --git a/extensions/http-curl/client/HTTPClient.cpp b/extensions/http-curl/client/HTTPClient.cpp
index 6e8d411..8d73f62 100644
--- a/extensions/http-curl/client/HTTPClient.cpp
+++ b/extensions/http-curl/client/HTTPClient.cpp
@@ -16,18 +16,15 @@
  */
 #include "HTTPClient.h"
 
-#include <algorithm>
 #include <cinttypes>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
-#include "Exception.h"
 #include "utils/gsl.h"
 #include "utils/StringUtils.h"
 #include "core/Resource.h"
-#include "utils/RegexUtils.h"
 #include "range/v3/algorithm/all_of.hpp"
 #include "range/v3/action/transform.hpp"
 #include "utils/HTTPUtils.h"
@@ -328,18 +325,16 @@
 }
 
 namespace {
-struct CurlSListFreeAll {
-  void operator()(struct curl_slist* slist) const {
-    curl_slist_free_all(slist);
-  }
-};
+using CurlSlistDeleter = decltype([](struct curl_slist* slist) { curl_slist_free_all(slist); });
 
-std::unique_ptr<struct curl_slist, CurlSListFreeAll> getCurlSList(const std::unordered_map<std::string, std::string>& request_headers) {
-  curl_slist* new_list = nullptr;
+std::unique_ptr<struct curl_slist, CurlSlistDeleter> toCurlSlist(const std::unordered_map<std::string, std::string>& request_headers) {
+  gsl::owner<curl_slist*> new_list = nullptr;
+  const auto guard = gsl::finally([&new_list]() { curl_slist_free_all(new_list); });
   for (const auto& [header_key, header_value] : request_headers)
-    new_list = curl_slist_append(new_list, utils::StringUtils::join_pack(header_key, ": ", header_value).c_str());
+    new_list = (utils::optional_from_ptr(curl_slist_append(new_list, utils::StringUtils::join_pack(header_key, ": ", header_value).c_str()))
+        | utils::orElse([]() { throw std::runtime_error{"curl_slist_append failed"}; })).value();
 
-  return {new_list, {}};
+  return {std::exchange(new_list, nullptr), {}};
 }
 }  // namespace
 
@@ -366,7 +361,7 @@
     curl_easy_setopt(http_session_.get(), CURLOPT_NOPROGRESS, 1);
   }
 
-  auto headers = getCurlSList(request_headers_);
+  const auto headers = toCurlSlist(request_headers_);
   if (headers) {
     curl_slist_append(headers.get(), "Expect:");
     curl_easy_setopt(http_session_.get(), CURLOPT_HTTPHEADER, headers.get());
@@ -452,7 +447,8 @@
 }
 
 int HTTPClient::onProgress(void *clientp, curl_off_t /*dltotal*/, curl_off_t dlnow, curl_off_t /*ultotal*/, curl_off_t ulnow) {
-  HTTPClient& client = *reinterpret_cast<HTTPClient*>(clientp);
+  gsl_Expects(clientp);
+  HTTPClient& client = *static_cast<HTTPClient*>(clientp);
   auto now = std::chrono::steady_clock::now();
   auto elapsed = now - client.progress_.last_transferred_;
   if (dlnow != client.progress_.downloaded_data_ || ulnow != client.progress_.uploaded_data_) {
@@ -522,12 +518,10 @@
     return "X-MiNiFi-Empty-Attribute-Name";
   }
 
-  std::string result;
-  result.reserve(field_name.size());
   // RFC822 3.1.2: The  field-name must be composed of printable ASCII characters
   // (i.e., characters that  have  values  between  33.  and  126., decimal, except colon).
-  ranges::actions::transform(field_name, [](char ch) {
-      return (ch >= 33 && ch <= 126 && ch != ':') ? ch : '-';
+  ranges::actions::transform(field_name, [](char ch) {  // NOLINT: false positive: Add #include <algorithm> for transform  [build/include_what_you_use] [4]
+    return (ch >= 33 && ch <= 126 && ch != ':') ? ch : '-';
   });
   return field_name;
 }

extensions/http-curl/client/HTTPClient.h

diff --git a/extensions/http-curl/client/HTTPClient.h b/extensions/http-curl/client/HTTPClient.h
index d6a1894..144edb3 100644
--- a/extensions/http-curl/client/HTTPClient.h
+++ b/extensions/http-curl/client/HTTPClient.h
@@ -207,8 +207,10 @@
  protected:
   static CURLcode configure_ssl_context(CURL* /*curl*/, void *ctx, void *param) {
 #ifdef OPENSSL_SUPPORT
-    auto* ssl_context_service = static_cast<minifi::controllers::SSLContextService*>(param);
-    if (!ssl_context_service->configure_ssl_context(static_cast<SSL_CTX*>(ctx))) {
+    gsl_Expects(ctx);
+    gsl_Expects(param);
+    auto& ssl_context_service = *static_cast<minifi::controllers::SSLContextService*>(param);
+    if (!ssl_context_service.configure_ssl_context(static_cast<SSL_CTX*>(ctx))) {
       return CURLE_FAILED_INIT;
     }
     return CURLE_OK;