W3C Baggage propagator uses form encoding instead of percent encoding - #5567
Open
dwin-gharibi wants to merge 2 commits into
Open
W3C Baggage propagator uses form encoding instead of percent encoding#5567dwin-gharibi wants to merge 2 commits into
dwin-gharibi wants to merge 2 commits into
Conversation
The W3C Baggage grammar defines baggage-octet as %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E, so "+" (%x2B) is an ordinary literal that must survive a round trip, while SP (%x20) is excluded and has to be percent-encoded. Add coverage for both directions and point test_encode_baggage_pairs at the real _encode_baggage_pairs helper instead of a local re-implementation that could never fail. Update the two assertions that encoded the form-encoded behaviour. These tests fail against the current implementation.
W3CBaggagePropagator used quote_plus/unquote_plus, which is application/x-www-form-urlencoded: it emits "+" for a space and decodes "+" back to a space. The W3C Baggage grammar lists "+" (%x2B) as an ordinary baggage-octet that must be preserved verbatim, and excludes SP (%x20), which must be percent-encoded. Both directions were affected. On extract, a literal "+" from a compliant peer was silently turned into a space, and a value of "+" decoded to an empty string once the trailing strip() ran. On inject, a value containing a space was emitted as "+", which a compliant peer reads back as a literal "+". Switch to quote/unquote so the wire format matches the specification and values survive a round trip across implementations.
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-23 16:16 UTC Review the latest changes. Status above doesn't look right?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5566.
Description
W3CBaggagePropagatorencodes and decodes baggage withquote_plus/unquote_plus, which isapplication/x-www-form-urlencoded. That form maps a space to+and decodes+back to a space. The W3C Baggage grammar definesbaggage-octetas%x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E, so+(%x2B) is an ordinary literal that must be preserved, and SP (%x20) is excluded and must be percent-encoded as%20.Both directions are wrong, and neither raises: values are silently corrupted as they cross a service boundary.
Root cause
opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.pyimportsquote_plus/unquote_plusand uses them at lines 118-119 (extract) and 167 (_encode_baggage_pairs). The+-for-space substitution is specific to HTML form submission and is not part of RFC 3986 percent-encoding, which is what the Baggage specification requires.Approach
Replace
quote_plus/unquote_pluswithquote/unquote.quoteis called withsafe=""so that every character outside the unreserved set is escaped; over-encoding is lossless and keeps the output unambiguous for any compliant parser.Files changed
opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.pyopentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py.changelog/5561.fixedTesting
Eight new tests cover the contract in both directions: literal
+preserved on extract (including the+-only and repeated-+cases that previously lost data tostrip()),+in keys,%20decoded to a space,%20emitted for a space, and inject/extract round trips.test_encode_baggage_pairspreviously asserted against a local re-implementation of the encoder rather than the module's own_encode_baggage_pairs, so it could never have caught this. It now calls the real helper.Two existing assertions encoded the buggy output (
transaction=string+with+spacesandkey=val+ue) and were updated to%20. Note that the extract direction - the one that corrupts inbound data - had no test at all.Result: 281 passed, 1 skipped across
opentelemetry-api(273 baseline plus 8 new).Risk / compatibility
The wire format changes for values containing spaces or other characters outside the unreserved set. This is the intended correction and brings Python in line with the other OpenTelemetry SDKs, but it is observable: anyone asserting on raw baggage headers in their own tests will see
%20where they previously saw+. Decoding remains backward compatible for every correctly-encoded header.