Skip to content

Fix trailing element separator rejecting valid dd tracestate #12229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gh-worker-dd-mergequeue-cf854d merged 10 commits into from
Aug 21, 2026
Merged

Fix trailing element separator rejecting valid dd tracestate #12229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
75c589e
Fix trailing element separator rejecting valid dd tracestate
mcculls Aug 18, 2026
3eab2aa
Apply same fix to DatadogPTagsCodec
mcculls Aug 18, 2026
db03948
Fix OWS before comma after trailing separator, align OWC helpers
mcculls Aug 18, 2026
59acd5d
Reject ';' as a dd tracestate key character
mcculls Aug 18, 2026
39acc26
Exclude TAG_KEY_SEPARATOR alongside TAGS_SEPARATOR, mirroring the W3C…
mcculls Aug 18, 2026
5e3562c
Avoid exception when unknown dd tag is followed by trailing-OWS-only …
mcculls Aug 18, 2026
7708b4e
Disallow interior OWS padding inside 'dd' section
mcculls Aug 18, 2026
cd8b76a
Ignore empty elements inside dd tracestate
mcculls Aug 19, 2026
d4f6014
Drop malformed elements from dd tracestate, but retain valid elements
mcculls Aug 20, 2026
2618d35
Still relay elements we couldn't parse, as downstream tracers might h…
mcculls Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/DatadogPTagsCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,8 @@ private static int validateCharsUntilSeparatorOrEnd(
pos++;
if (pos < end) {
c = s.charAt(pos);
// It's not allowed to have the separator as the last character so only check
// if there is something after the separator
if (pos < end - 1 && c == separator) {
break;
if (c == separator) {
break; // trailing separator allowed; caller resumes parsing from here
}
}
} while (pos < end);
Expand All @@ -189,7 +187,10 @@ private static int validateCharsUntilSeparatorOrEnd(

private static boolean isAllowedKeyChar(int c) {
// space (MIN_ALLOWED_CHAR) is not allowed
return c > MIN_ALLOWED_CHAR && c <= MAX_ALLOWED_CHAR && c != TAGS_SEPARATOR;
return c > MIN_ALLOWED_CHAR
&& c <= MAX_ALLOWED_CHAR
&& c != TAG_KEY_SEPARATOR
&& c != TAGS_SEPARATOR;
Comment thread
mcculls marked this conversation as resolved.
}

private static boolean isAllowedValueChar(int c) {
Expand Down
80 changes: 58 additions & 22 deletions dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/W3CPTagsCodec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) {
CharSequence lastParentId = null;
TagValue orgPropagationMarkerTagValue = null;
while (tagPos < ddMemberValueEnd) {
tagPos = skipEmptyElements(value, tagPos, ddMemberValueEnd);
if (tagPos >= ddMemberValueEnd) {
break;
}
int tagKeyEndsAt =
validateCharsUntilSeparatorOrEnd(
value,
Expand All @@ -108,10 +112,11 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) {
KEY_VALUE_SEPARATOR,
false,
W3CPTagsCodec::isAllowedKeyChar);
if (tagKeyEndsAt < 0 || tagKeyEndsAt == ddMemberValueEnd) {
log.warn("Invalid datadog tags header value: '{}' at {}", value, tagPos);
// TODO drop parts?
return empty(tagsFactory, value, firstMemberStart, ddMemberStart, ddMemberValueEnd);
if (tagKeyEndsAt < 0 || tagKeyEndsAt >= ddMemberValueEnd) {
int nextTagPos = skipMalformedElement(value, tagPos, ddMemberValueEnd);
maxUnknownSize += (nextTagPos - tagPos); // still relay malformed elements
tagPos = nextTagPos;
continue;
Comment thread
mcculls marked this conversation as resolved.
}
int tagValuePos = tagKeyEndsAt + 1;
int tagValueEndsAt =
Expand All @@ -123,9 +128,10 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) {
true,
W3CPTagsCodec::isAllowedValueChar);
if (tagValueEndsAt < 0) {
log.warn("Invalid datadog tags header value: '{}' at {}", value, tagKeyEndsAt);
// TODO drop parts?
return empty(tagsFactory, value, firstMemberStart, ddMemberStart, ddMemberValueEnd);
int nextTagPos = skipMalformedElement(value, tagValuePos, ddMemberValueEnd);
maxUnknownSize += (nextTagPos - tagPos); // still relay malformed elements
tagPos = nextTagPos;
continue;
}
int nextTagPos = tagValueEndsAt + 1;
if (tagValueEndsAt == ddMemberValueEnd) {
Expand All @@ -152,7 +158,6 @@ PropagationTags fromHeaderValue(PTagsFactory tagsFactory, String value) {
if (tagKey.equals(TRACE_ID_TAG)) {
return tagsFactory.createInvalid(PROPAGATION_ERROR_MALFORMED_TID + tagValue);
}
// TODO drop parts?
return empty(tagsFactory, value, firstMemberStart, ddMemberStart, ddMemberValueEnd);
}
if (tagKey.equals(DECISION_MAKER_TAG)) {
Expand Down Expand Up @@ -351,10 +356,8 @@ private static int validateCharsUntilSeparatorOrEnd(
pos++;
if (pos < end) {
c = s.charAt(pos);
// It's not allowed to have the separator as the last character so only check
// if there is something after the separator
if (pos < end - 1 && c == separator) {
break;
if (c == separator) {
Comment thread
mcculls marked this conversation as resolved.
break; // trailing separator allowed; caller resumes parsing from here
}
}
} while (pos < end);
Expand All @@ -365,7 +368,10 @@ private static int validateCharsUntilSeparatorOrEnd(
private static boolean isAllowedKeyChar(int c) {
// We already know that the segments have been validated against the valid chars for
// the general tracestate header
return c > MIN_ALLOWED_CHAR && c <= MAX_ALLOWED_CHAR && c != KEY_VALUE_SEPARATOR;
return c > MIN_ALLOWED_CHAR
&& c <= MAX_ALLOWED_CHAR
&& c != KEY_VALUE_SEPARATOR
&& c != ELEMENT_SEPARATOR;
Comment thread
mcculls marked this conversation as resolved.
}

private static boolean isAllowedValueChar(int c) {
Expand Down Expand Up @@ -613,12 +619,37 @@ private static boolean isOWC(char c) {
return c == ' ' || c == '\t';
}

private static int stripTrailingOWC(String original, int start, int end) {
char c = original.charAt(--end);
while (isOWC(c) && end > start) {
c = original.charAt(--end);
private static int stripTrailingOWC(String value, int start, int end) {
while (end > start + 1 && isOWC(value.charAt(end - 1))) {
end--;
}
return ++end;
return end;
}

private static int skipMalformedElement(String value, int start, int end) {
log.warn(
"Invalid datadog tags header value: '{}' dropping malformed element at {}", value, start);
int pos = start;
while (pos < end) {
char c = value.charAt(pos++);
if (c == ELEMENT_SEPARATOR) {
return pos;
}
}
return end;
}

private static int skipEmptyElements(String value, int start, int end) {
int pos = start;
while (pos < end) {
char c = value.charAt(pos++);
if (c == ELEMENT_SEPARATOR) {
start = pos;
} else if (!isOWC(c)) {
return start;
}
}
return end;
}

private static int cleanUpAndAppendUnknown(StringBuilder sb, W3CPTags w3CPTags, int size) {
Expand All @@ -631,16 +662,21 @@ private static int cleanUpAndAppendUnknown(StringBuilder sb, W3CPTags w3CPTags,
int elementStart = w3CPTags.ddMemberStart + EMPTY_SIZE; // skip over 'dd='
int okSize = size;
while (elementStart < w3CPTags.ddMemberValueEnd && size < MAX_HEADER_SIZE) {
elementStart = skipEmptyElements(original, elementStart, w3CPTags.ddMemberValueEnd);
if (elementStart == w3CPTags.ddMemberValueEnd) {
break;
}
okSize = size;
int elementEnd = original.indexOf(ELEMENT_SEPARATOR, elementStart);
if (elementEnd < 0) {
if (elementEnd < 0 || elementEnd > w3CPTags.ddMemberValueEnd) {
elementEnd = w3CPTags.ddMemberValueEnd;
}
if (!original.startsWith(Encoding.W3C.getPrefix(), elementStart)) {
char first = original.charAt(elementStart);
char second = original.charAt(elementStart + 1);
if (second != KEY_VALUE_SEPARATOR || (first != 'o' && first != 's')) {
// only append elements that we don't know about or are not tags
// ignore known o:, s:, and p: elements because we always add them back in appendPrefix
if ((first != 'o' && first != 's' && first != 'p')
|| elementStart + 1 >= elementEnd
|| original.charAt(elementStart + 1) != KEY_VALUE_SEPARATOR) {
if (sb.length() > EMPTY_SIZE) {
sb.append(ELEMENT_SEPARATOR);
size++;
Expand Down
1 change: 1 addition & 0 deletions dd-trace-core/src/test/java/datadog/trace/core/propagation/DatadogPropagationTagsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class DatadogPropagationTagsTest extends DDJavaSpecification {
"valid dm tag 2-digit | '_dd.p.dm=934086a686-10' | '_dd.p.dm=934086a686-10' | [_dd.p.dm: '934086a686-10'] ",
"valid dm tag 3-digit | '_dd.p.dm=934086a686-102' | '_dd.p.dm=934086a686-102' | [_dd.p.dm: '934086a686-102'] ",
"dm tag minus only | '_dd.p.dm=-1' | '_dd.p.dm=-1' | [_dd.p.dm: '-1'] ",
"dm tag with trailing separator | '_dd.p.dm=-4,' | '_dd.p.dm=-4' | [_dd.p.dm: '-4'] ",
"any p tag | '_dd.p.anytag=value' | '_dd.p.anytag=value' | [_dd.p.anytag: 'value'] ",
"non p tag dropped | '_dd.b.somekey=value' | | [:] ",
"upstream services alone dropped | '_dd.p.upstream_services=bWNudWx0eS13ZWI|0|1|0.1' | | [:] ",
Expand Down
25 changes: 16 additions & 9 deletions dd-trace-core/src/test/java/datadog/trace/core/propagation/W3CHttpExtractorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,22 @@ void checkMaxFromW3CTraceIds(@ConvertWith(TraceIdTestConverter.class) DDTraceId
}

@TableTest({
"scenario | traceparent | tracestate | priority | decisionMaker | origin",
"keep empty state | '00-00000000000000000000000000000001-123456789abcdef0-01' | '' | SAMPLER_KEEP | SamplingMechanism.DEFAULT | ",
"drop empty state | '00-00000000000000000000000000000001-123456789abcdef0-00' | '' | SAMPLER_DROP | | ",
"keep with user keep state | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some' | USER_KEEP | | some ",
"keep with user keep state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some;t.dm:-4' | USER_KEEP | SamplingMechanism.MANUAL | some ",
"drop with user keep state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:2;o:some;t.dm:-4' | SAMPLER_DROP | | some ",
"drop with user drop state | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:-1;o:some' | USER_DROP | | some ",
"drop with user drop state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:-1;o:some;t.dm:-4' | USER_DROP | SamplingMechanism.MANUAL | some ",
"keep overrides user drop state with manual dm | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:-1;o:some;t.dm:-4' | SAMPLER_KEEP | SamplingMechanism.DEFAULT | some "
"scenario | traceparent | tracestate | priority | decisionMaker | origin",
"keep empty state | '00-00000000000000000000000000000001-123456789abcdef0-01' | '' | SAMPLER_KEEP | SamplingMechanism.DEFAULT | ",
"drop empty state | '00-00000000000000000000000000000001-123456789abcdef0-00' | '' | SAMPLER_DROP | | ",
"keep with user keep state | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some' | USER_KEEP | | some ",
"keep with trailing element separator | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some;' | USER_KEEP | | some ",
"keep with trailing separator and OWS | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some; \t' | USER_KEEP | | some ",
"keep with trailing separator OWS before comma | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some; ,x=y' | USER_KEEP | | some ",
"skip empty element in middle | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;;o:some' | USER_KEEP | | some ",
"skip leading separator | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=;s:2;o:some' | USER_KEEP | | some ",
"skip bare element in middle | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;flag;o:some' | USER_KEEP | | some ",
"skip multiple bare elements in middle | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;flag1;flag2;o:some' | USER_KEEP | | some ",
"keep with user keep state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:2;o:some;t.dm:-4' | USER_KEEP | SamplingMechanism.MANUAL | some ",
"drop with user keep state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:2;o:some;t.dm:-4' | SAMPLER_DROP | | some ",
"drop with user drop state | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:-1;o:some' | USER_DROP | | some ",
"drop with user drop state and manual dm | '00-00000000000000000000000000000001-123456789abcdef0-00' | 'dd=s:-1;o:some;t.dm:-4' | USER_DROP | SamplingMechanism.MANUAL | some ",
"keep overrides user drop state with manual dm | '00-00000000000000000000000000000001-123456789abcdef0-01' | 'dd=s:-1;o:some;t.dm:-4' | SAMPLER_KEEP | SamplingMechanism.DEFAULT | some "
})
void extractTraceparentTracestateAndHttpHeaders(
String traceparent,
Expand Down
Loading