Tolerate invalid UTF-8 and malformed percent-encoding in baggage headers#5689
Conversation
`Baggage#parse_baggage_header` raised `ArgumentError` on two classes of
malformed input, and the exception escaped `Propagation#extract` because
`propagate_baggage` runs outside the rescue block in `propagation.rb`,
contradicting `extract`'s "never raises" docstring contract.
Both paths now record the existing `context_header_style.malformed`
telemetry metric and return `{}`:
1. `baggage_header.valid_encoding? == false` — UTF-8-tagged input with
invalid byte sequences. Triggers raises in `split`, `strip`, and
`URI.decode_www_form_component`. Guard added at the top of the method.
2. Malformed percent encoding (`%XX`, lone `%`) — raises from
`URI.decode_www_form_component`. Wrapped in `rescue ArgumentError`.
ASCII-8BIT input (Rack's default for HTTP headers) is unaffected:
`valid_encoding?` is true for binary regardless of byte values.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Typing analysisNote: Ignored files are excluded from the next sections.
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 84b6652 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-05-06 23:33:56 Comparing candidate commit 84b6652 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.
|
What does this PR do?
Makes
Datadog::Tracing::Distributed::Baggage#parse_baggage_headertreat two classes of malformed input as "malformed header → empty baggage" rather than raisingArgumentErrorinto the caller:baggage_header.valid_encoding? == false— UTF-8-tagged input containing invalid byte sequences. AffectsString#split,String#strip, andURI.decode_www_form_component, all of which raise on invalid UTF-8.%XX, lone%) — raises fromURI.decode_www_form_component.Both paths now record the existing
context_header_style.malformedtelemetry metric and return{}, matching the shape of every other malformed-header branch in this method.Motivation:
This PR was opened in response to review feedback on #5672 (the baggage size/item limits PR), to make the recommendations easier to evaluate as a self-contained change. The findings surfaced during a focused review of how the parser interacts with non-ASCII and malformed input.
Propagation#extractdocuments that it never raises — it logs and returns. That contract holds for every other distributed-trace propagator because the loop inPropagation#extractis wrapped inrescue => e. Baggage is the exception:propagate_baggageis invoked outside that rescue (lib/datadog/tracing/distributed/propagation.rb:146), so any exception raised insideBaggage#extractpropagates to whoever calledPropagation#extract— typically a Rack middleware in customer code.A baggage header containing either invalid UTF-8 bytes (when the upstream hands the propagator a UTF-8-tagged string) or `%XX` is enough to trigger this. Both raises are pre-existing in the parser; this PR closes them at their source.
The change is intentionally narrow:
Change log entry
Yes. Stop raising on baggage headers containing invalid UTF-8 bytes or malformed percent-encoded sequences; treat them as malformed and return empty baggage.
Additional Notes:
Companion to #5672 — both touch `parse_baggage_header` but at different lines (size/item caps vs. encoding/decoding safety) and merge cleanly in either order.
How to test the change?
```
bundle exec rspec spec/datadog/tracing/distributed/baggage_spec.rb
```
Three new specs cover: invalid-UTF-8 input never raises, malformed percent-encoded input never raises, and ASCII-8BIT input continues to parse normally. Verified that the first two specs fail without the production change.