[BUG] OnResponse() can call std::terminate() when the response body fails to parse as JSON/protobuf

[brhenc]

Describe your environment

Reproduced by source read + build on main at 11fa0db0 (also present in v1.28.0, the latest release -- not a regression). exporters/otlp/src/otlp_http_client.cc, ResponseHandler::OnResponse.

Steps to reproduce

OnResponse(http_client::Response &response) noexcept is declared noexcept, but its body has no try/catch anywhere:

void OnResponse(http_client::Response &response) noexcept override
{
  sdk::common::ExportResult result = sdk::common::ExportResult::kSuccess;
  std::string log_message;
  {
    std::unique_lock<std::mutex> lk(mutex_);
    body_ = std::string(response.GetBody().begin(), response.GetBody().end());
    ...
  }

  if (response_ != nullptr && result == sdk::common::ExportResult::kSuccess && !body_.empty())
  {
    if (content_type_ == HttpRequestContentType::kJson)
    {
      if (!google::protobuf::util::JsonStringToMessage(body_, response_).ok())
      { ... }
    }
    else if (!response_->ParseFromString(body_))
    { ... }
  }
  ...
}

Both google::protobuf::util::JsonStringToMessage and Message::ParseFromString can throw (protobuf's own message-building path allocates, and an oversized or maliciously-crafted body can trigger std::bad_alloc or other exceptions depending on build/allocator configuration). The body_ = std::string(...) copy a few lines above can throw for the same reason on an oversized body.

An unhandled exception escaping a function marked noexcept invokes std::terminate() per the C++ standard, regardless of what the exception actually was.

To trigger: point an OTLP HTTP exporter at a collector (or a MITM/misbehaving intermediary proxy) that returns a 2xx response with an extremely large or adversarially crafted JSON/protobuf body.

What is the expected behavior?

A malformed or oversized response body from the configured collector should cause that one export to be reported as a failure (ExportResult::kFailure) -- it shouldn't be able to take down the exporting process.

What is the actual behavior?

The exception escapes noexcept and the process calls std::terminate(), killing the entire application -- not just the telemetry pipeline. Any host application exporting spans/logs/metrics via OTLP/HTTP is exposed to this if its collector (or anything between it and the collector) can be made to return an oversized or malformed response body.

Additional context

Suggested fix -- wrap the body in try/catch, converting any exception into kFailure, and keep the existing termination logic (stopping_.compare_exchange_strong(...) / Unbind(result)) outside the try block so a caught exception can't also leave waitForResponse()-style callers waiting forever (the same class of problem as the lost-wakeup bug fixed in #4298 for the Elasticsearch exporter):

     sdk::common::ExportResult result = sdk::common::ExportResult::kSuccess;
     std::string log_message;
-    // Lock the private members so they can't be read while being modified
+    try
     {
-      std::unique_lock<std::mutex> lk(mutex_);
-
-      // Store the body of the request
-      body_ = std::string(response.GetBody().begin(), response.GetBody().end());
-
-      if (!(response.GetStatusCode() >= 200 && response.GetStatusCode() <= 299))
+      // Lock the private members so they can't be read while being modified
       {
-        log_message = BuildResponseLogMessage(response, body_);
+        std::unique_lock<std::mutex> lk(mutex_);
 
-        OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Export failed, " << log_message);
-        result = sdk::common::ExportResult::kFailure;
-      }
-      else if (console_debug_)
-      {
-        if (log_message.empty())
+        // Store the body of the request
+        body_ = std::string(response.GetBody().begin(), response.GetBody().end());
+
+        if (!(response.GetStatusCode() >= 200 && response.GetStatusCode() <= 299))
         {
           log_message = BuildResponseLogMessage(response, body_);
+
+          OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Export failed, " << log_message);
+          result = sdk::common::ExportResult::kFailure;
+        }
+        else if (console_debug_)
+        {
+          if (log_message.empty())
+          {
+            log_message = BuildResponseLogMessage(response, body_);
+          }
         }
       }
-    }
 
-    // On 2xx with a non-empty body, parse it into the caller-provided typed response
-    if (response_ != nullptr && result == sdk::common::ExportResult::kSuccess && !body_.empty())
-    {
-      if (content_type_ == HttpRequestContentType::kJson)
+      // On 2xx with a non-empty body, parse it into the caller-provided typed response
+      if (response_ != nullptr && result == sdk::common::ExportResult::kSuccess && !body_.empty())
       {
-        if (!google::protobuf::util::JsonStringToMessage(body_, response_).ok())
+        if (content_type_ == HttpRequestContentType::kJson)
+        {
+          if (!google::protobuf::util::JsonStringToMessage(body_, response_).ok())
+          {
+            OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Failed to parse JSON response body");
+            result = sdk::common::ExportResult::kFailure;
+          }
+        }
+        else if (!response_->ParseFromString(body_))
         {
-          OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Failed to parse JSON response body");
+          OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Failed to parse response body");
           result = sdk::common::ExportResult::kFailure;
         }
       }
-      else if (!response_->ParseFromString(body_))
+
+      if (console_debug_ && result == sdk::common::ExportResult::kSuccess)
       {
-        OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Failed to parse response body");
-        result = sdk::common::ExportResult::kFailure;
+        OTEL_INTERNAL_LOG_DEBUG("[OTLP HTTP Client] Export success, " << log_message);
       }
     }
-
-    if (console_debug_ && result == sdk::common::ExportResult::kSuccess)
+    catch (const std::exception &ex)
+    {
+      OTEL_INTERNAL_LOG_ERROR(
+          "[OTLP HTTP Client] Exception while processing response: " << ex.what());
+      result = sdk::common::ExportResult::kFailure;
+    }
+    catch (...)
     {
-      OTEL_INTERNAL_LOG_DEBUG("[OTLP HTTP Client] Export success, " << log_message);
+      OTEL_INTERNAL_LOG_ERROR("[OTLP HTTP Client] Unknown exception while processing response");
+      result = sdk::common::ExportResult::kFailure;
     }
 
     {

Compile-checked against a clean build of this file (WITH_OTLP_HTTP=ON) -- no warnings or errors. Happy to open a PR with this if useful.

[github-actions]

This issue is available for anyone to work on. Make sure to reference this issue in your pull request.
✨ Thank you for your contribution! ✨

[shashankxrm]

Hey @brhenc, I’d be happy to pick up one of these issues you opened today and work on a PR if you’re busy with the SAST backlog and haven’t started on them yet. Let me know if there’s one you’d prefer me to take, otherwise I can pick one up.