open-telemetry / open-telemetry/opentelemetry-cpp

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

Open Beginner friendly
#4,534 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug help wanted triage/accepted
Dominant language
C++
Stars
1.4k
Forks
632
Avg merge
1d 12h
Merged PRs (30d)
78

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in exporters/otlp/src/otlp_http_client.cc at ResponseHandler::OnResponse and review the existing result and termination flow around response-body copying and parsing. Build with WITH_OTLP_HTTP=ON, then verify malformed or oversized JSON/protobuf responses become ExportResult::kFailure without terminating the process, while the existing stopping_ and Unbind flow still completes.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
observability
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.