[BUG] The curl client ignores curl_slist_append and curl_multi_init failures

[thc1006]

Describe your environment

main at 768d04de, Debian, gcc 14.2.0, libcurl 8.14.1, plain CMake build with the defaults.

Steps to reproduce

I can't force either allocation to fail, so this is from reading the code against the libcurl docs, plus one measurement of what libcurl actually does with a null multi handle. Two sites of the same shape, and each one sits in a file that already checks the allocation next to it.

1. curl_slist_append at http_operation_curl.cc:470

      curl_resource_.headers_chunk =
          curl_slist_append(curl_resource_.headers_chunk, header.c_str());

The docs call this pattern out by name:

A null pointer is returned if anything went wrong, otherwise the new list pointer is returned. 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.

2. curl_multi_init at http_client_curl.cc:274, :285 and :912

Both constructors take the result straight into multi_handle_, and resetMultiHandle() replaces it the same way while recovering from a multi error. The docs:

If this function returns NULL, something went wrong and you cannot use the other curl functions.

What libcurl 8.14.1 does with one, measured rather than assumed:

multi_add_handle(NULL)    = 1  (Invalid multi handle)
multi_remove_handle(NULL) = 1  (Invalid multi handle)
  CURLM_BAD_HANDLE        = 1
  CURLM_OK                = 0

What is the expected behavior?

The same thing the neighbouring code already does. The HttpOperation constructor checks curl_easy_init() fifteen lines earlier, at http_operation_curl.cc:456, and dispatches CreateFailed when it returns null. HttpClient::wakeupBackgroundThread() already guards multi_handle_ for null at http_client_curl.cc:715 before curl_multi_wakeup. A failure that the library reports should not be turned into a success.

What is the actual behavior?

Both returns are dropped, and in both cases the failure is silent rather than loud.

For the header list, assigning null back over headers_chunk loses the only pointer to whatever was appended before the failure. The two places that free it, Cleanup() at http_operation_curl.cc:589 and doRemoveSessions() at http_client_curl.cc:816, both test for null first, so that chain is never freed. The constructor then carries on and dispatches Created, and Setup() at :1196 only sets CURLOPT_HTTPHEADER when headers_chunk is non-null:

  if (curl_resource_.headers_chunk != nullptr)
  {
    rc = SetCurlListOption(CURLOPT_HTTPHEADER, curl_resource_.headers_chunk);

so the request goes out with none of the caller's headers rather than not going out at all. For an OTLP export that means no Content-Type: application/x-protobuf, and the receiver rejects a request the exporter believes it sent correctly. Failing on the second header is no better than failing on the first, since the assignment discards the ones that already succeeded along with the rest.

For the multi handle, a null one makes doAddSessions() at http_client_curl.cc:752 call curl_multi_add_handle(nullptr, easy_handle), which returns CURLM_BAD_HANDLE as measured above, and that return is discarded. Every session is then accepted, never added to a multi handle, and never completed. :912 is the one I'd worry about most, because that's the recovery path: resetMultiHandle() is reached after a multi error, and if the re-init fails there the client turns into a black hole for the rest of its life with nothing logged.

Additional context

#4395 is open against doAddSessions() and does check curl_multi_add_handle's return, so if that lands the add path stops pretending a rejected handle succeeded. It doesn't cover this: the null handle would still go unnoticed at construction and at reset, and #4395 doesn't touch curl_slist_append at all.

Both of these are out of memory paths, so nobody is hitting them day to day. I'm raising them because the same two files already check the allocations either side, which makes these look like oversights rather than a deliberate decision that the library's failure reports aren't worth reading.

They're one issue rather than two because a single small change covers both sites, but a fix should cover both before this closes. I'm holding the patch until the curl PRs already open on these two files land, so it doesn't add another conflict to that queue.