open-telemetry / open-telemetry/opentelemetry-cpp
[BUG] OnResponse() can call std::terminate() when the response body fails to parse as JSON/protobuf
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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