fix(trace): reject tracestate keys with vendor part longer than 13 chars - #5599
Draft
sidsri14 wants to merge 2 commits into
Draft
fix(trace): reject tracestate keys with vendor part longer than 13 chars#5599sidsri14 wants to merge 2 commits into
sidsri14 wants to merge 2 commits into
Conversation
TraceState validated keys with a regex whose tenant/vendor alternative allowed
up to 14 characters (`[a-z][_0-9a-z\-\*\/]{0,13}`). Per the W3C tracestate
spec the vendor part (after `@`) MUST be at most 13 characters, so over-long
vendor keys such as `1@nrabcdefghijkl` were incorrectly accepted by `add()`.
Tighten the vendor quantifier to `{0,12}` so 14-character vendors are rejected.
Invalid pairs are discarded (the existing behavior), so `add()` returns the
unchanged TraceState instead of adding the illegal key.
Fixes open-telemetry#5136
Assisted-by: opencode
Signed-off-by: Siddharth Srivastava <128143077+sidsri14@users.noreply.github.com>
Signed-off-by: Siddharth Srivastava <128143077+sidsri14@users.noreply.github.com>
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
1 participant
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.
Which problem is this PR solving?
Fixes #5136.
TraceState.add()(andTraceStateconstruction) did not fullyvalidate keys against the W3C tracestate spec: the vendor part of a key
(after
@) was allowed to be up to 14 characters, but the spec requires it tobe at most 13. As a result, illegal keys such as
1@nrabcdefghijklweresilently accepted.
Short description of the changes
opentelemetry-api/src/opentelemetry/trace/span.py, tightened the vendorquantifier in
_KEY_FORMATfrom{0,13}to{0,12}so the vendor part isat most 13 characters (1 + 12).
add()returnsthe unchanged
TraceStaterather than adding the illegal key.test_tracestate_rejects_illegal_vendor_keycovering a14-char vendor, a long-tenant + 14-char-vendor key, a valid 241-char-tenant
key, and a non-vendor key starting with a digit.
How to verify that this has the expected result
All 11 tests pass. The new test asserts over-long vendor keys are not added
(
len == 0) while valid keys remain accepted.Assisted-by: opencode
Signed-off-by: Siddharth Srivastava 128143077+sidsri14@users.noreply.github.com