[BUG] Check what curl_slist_append and curl_multi_init return

Both are allocation points whose failure was treated as success, and both sit next
to a check the same file already makes.

curl_slist_append returns null without freeing the list it was given, and the docs
say so: "To avoid overwriting an existing non-empty list on failure, the new list
should be returned to a temporary variable which can be tested for NULL before
updating the original list pointer." Assigning straight back over headers_chunk did
the opposite. The pointer to everything appended so far went with it, so the two
places that free the list saw null and never ran, and Setup() only sets
CURLOPT_HTTPHEADER when headers_chunk is non-null, so the request went out with none
of the caller's headers rather than not going out. For an OTLP export that means no
Content-Type and a receiver that rejects a request the exporter believes it sent.

It now appends into a temporary. On failure the partial list is released, the
operation reports CreateFailed the same way it already does when curl_easy_init
returns null fifteen lines earlier, and nothing half built is left behind.

curl_multi_init returns null the same way, and its result went straight into
multi_handle_ in both constructors and in resetMultiHandle. Measured against libcurl
8.14.1, curl_multi_add_handle on a null multi handle reports CURLM_BAD_HANDLE, and
that return is discarded today, so such a client would accept every session, add
none of them, and complete none of them. The reset path is the worse of the three:
it runs while recovering from a multi error, so a failure there turns the client
into a black hole for the rest of its life. All three now go through one helper that
logs.

Fixes #4404

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

Author: thc1006

CHANGELOG.md

@@ -34,6 +34,9 @@ Increment the:
   deprecated C headers (`stdint.h`, `stddef.h`, `stdlib.h`, `string.h`,
   `stdio.h`, `ctype.h`, `limits.h`, `assert.h`) with their C++ equivalents
   ([#4349](https://github.com/open-telemetry/opentelemetry-cpp/pull/4349))
+* [BUG] Check what curl_slist_append and curl_multi_init return instead of
+  treating a failed allocation as success
+  [#4404](https://github.com/open-telemetry/opentelemetry-cpp/issues/4404)
 
 * [CONFIGURATION] Add SDK component builder interfaces to the registry
   [#4358](https://github.com/open-telemetry/opentelemetry-cpp/issues/4358)

ext/src/http/client/curl/http_client_curl.cc

@@ -23,6 +23,7 @@
 #include "opentelemetry/ext/http/common/url_parser.h"
 #include "opentelemetry/nostd/shared_ptr.h"
 #include "opentelemetry/nostd/string_view.h"
+#include "opentelemetry/sdk/common/global_log_handler.h"
 #include "opentelemetry/sdk/common/thread_instrumentation.h"
 #include "opentelemetry/version.h"
 
@@ -270,8 +271,20 @@ void Session::FinishOperation()
   }
 }
 
+// A null multi handle makes every curl_multi_add_handle report CURLM_BAD_HANDLE, so a client
+// built on one accepts sessions and never sends any of them. Say so rather than fail quietly.
+static CURLM *initMultiHandle()
+{
+  CURLM *handle = curl_multi_init();
+  if (nullptr == handle)
+  {
+    OTEL_INTERNAL_LOG_ERROR("[HTTP Client Curl] curl_multi_init failed, this client cannot send");
+  }
+  return handle;
+}
+
 HttpClient::HttpClient()
-    : multi_handle_(curl_multi_init()),
+    : multi_handle_(initMultiHandle()),
       next_session_id_{0},
       max_sessions_per_connection_{8},
       background_thread_instrumentation_(nullptr),
@@ -282,7 +295,7 @@ HttpClient::HttpClient()
 
 HttpClient::HttpClient(
     const std::shared_ptr<sdk::common::ThreadInstrumentation> &thread_instrumentation)
-    : multi_handle_(curl_multi_init()),
+    : multi_handle_(initMultiHandle()),
       next_session_id_{0},
       max_sessions_per_connection_{8},
       background_thread_instrumentation_(thread_instrumentation),
@@ -909,7 +922,7 @@ void HttpClient::resetMultiHandle()
   curl_multi_cleanup(multi_handle_);
 
   // Create a another multi handle to continue pending sessions
-  multi_handle_ = curl_multi_init();
+  multi_handle_ = initMultiHandle();
 }
 
 }  // namespace curl

ext/src/http/client/curl/http_operation_curl.cc

@@ -467,8 +467,23 @@ HttpOperation::HttpOperation(opentelemetry::ext::http::client::Method method,
     for (auto &kv : this->request_headers_)
     {
       const auto header = std::string(kv.first).append(": ").append(kv.second);
-      curl_resource_.headers_chunk =
-          curl_slist_append(curl_resource_.headers_chunk, header.c_str());
+
+      // Into a temporary first. curl_slist_append returns null without freeing the list it was
+      // given, so assigning the result straight back would drop the only pointer to everything
+      // appended so far, and Setup() would then send the request with none of these headers
+      // rather than not send it.
+      curl_slist *appended = curl_slist_append(curl_resource_.headers_chunk, header.c_str());
+      if (nullptr == appended)
+      {
+        curl_slist_free_all(curl_resource_.headers_chunk);
+        curl_resource_.headers_chunk = nullptr;
+        last_curl_result_            = CURLE_OUT_OF_MEMORY;
+        DispatchEvent(opentelemetry::ext::http::client::SessionState::CreateFailed,
+                      curl_easy_strerror(last_curl_result_));
+        return;
+      }
+
+      curl_resource_.headers_chunk = appended;
     }
   }