opentelemetry-propagator-jaeger: enforce baggage limits on extract - #5556
Open
serhiy-bzhezytskyy wants to merge 5 commits into
Open
opentelemetry-propagator-jaeger: enforce baggage limits on extract#5556serhiy-bzhezytskyy wants to merge 5 commits into
serhiy-bzhezytskyy wants to merge 5 commits into
Conversation
The Jaeger propagator extracts every uberctx- header into baggage with no bound: _extract_baggage walks all carrier keys and sets each one, so an inbound carrier decides how much baggage enters the context. The Jaeger format defines no limits, so this borrows the W3C Baggage spec limits (180 entries, 4096 bytes per entry, 8192 bytes total, byte-denominated), dropping members once a limit is reached and keeping the earlier ones. opentelemetry-java caps its Jaeger propagator the same way for the same reason (CVE-2026-45292). Adds four extract tests: entry count, per-entry bytes, per-entry bytes measured in bytes rather than characters, and the total-bytes cap. Each fails without the change. Assisted-By: Claude Fable 5
Assisted-By: Claude Fable 5
lzchen
reviewed
Aug 18, 2026
lzchen
reviewed
Aug 18, 2026
Two points from review. The entry limit bounded the entries kept, not the candidates inspected, so a carrier full of oversized uberctx- headers could force unbounded decoding and byte accounting; the limit now bounds the candidates via itertools.islice, which also makes the accepted-entry counter redundant since kept is at most inspected. opentelemetry-java bounds its jaeger baggage parse the same way, in MAX_BAGGAGE_HEADER_TOKENS. The per-entry accounting also omitted the "=" and "," a baggage header carries, so each entry now counts two more bytes. That makes the numbers slightly stricter than opentelemetry-java, which counts key and value only. Assisted-By: Claude Fable 5
Pull request dashboard statusWaiting on maintainers · refreshed 2026-08-21 15:55 UTC Merge when ready. Status above doesn't look right?
|
The tests added here take the class past pylint's max-public-methods of 20, which fails `lint-opentelemetry-propagator-jaeger`. Disable that check on the class, the same way `propagator/opentelemetry-propagator-b3/tests/test_b3_format.py` does, and rename the loop variables the new tests introduced so `invalid-name` stays quiet without a second suppression. Assisted-By: Claude Fable 5
xrmx
reviewed
Aug 20, 2026
Fix the extract-side byte accounting: entry_bytes charged both the "=" and the "," on every entry, but the "," only ever appears between entries. Charge "=" per entry and "," once per already-accepted entry against the running total instead, in a shared _limit_baggage_bytes helper. Cap inject the same way extract is capped, so an in-process baggage map built without going through extract cannot produce an unbounded number or volume of uberctx- headers either. Both paths now build a bounded list of candidate pairs, then filter it through the same byte budget. Update the changelog fragment to cover both directions.
lzchen
approved these changes
Aug 21, 2026
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
3 participants
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.
The Jaeger propagator extracts every
uberctx-header into baggage with no bound:_extract_baggagewalks all carrier keys and sets each one, so an inbound carrier decides how much baggage enters the context.injecthad the same problem in the other direction: it wrote every entry from an in-process baggage map into the carrier with no bound either.The Jaeger format defines no limits, so this borrows the W3C Baggage spec limits — 180 entries, 4096 bytes per entry, 8192 bytes total, byte-denominated — dropping members once a limit is reached and keeping the earlier ones, on both extract and inject.
opentelemetry-javacaps its own Jaeger propagator's extract the same way and for the same reason; the coreW3CBaggagePropagatorin this package already uses the same three numbers.Adds extract tests for entry count, per-entry bytes (the
=inside an entry and the,between accepted entries, pinned separately), and the total-bytes cap; mirrors those with two inject tests. Each fails without the change.Assisted-By: Claude Fable 5