Skip to content

Commit 86e545d

Browse files
committed
fix(core): Simplify TID header parsing error handling
Revert the non-throwing id parsing from #12739: DD64bTraceId.fromOrNull and the LongStringUtils sentinel parser added public API, duplicated the decimal parser, and diverged from DD128bTraceId and DDSpanId. Keep the goal of keeping malformed headers out of telemetry by catching NumberFormatException during Datadog context extraction and logging it with EXCLUDE_TELEMETRY, without the stack trace.
1 parent a9ee2b1 commit 86e545d

9 files changed

Lines changed: 12 additions & 479 deletions

File tree

‎dd-trace-api/src/main/java/datadog/trace/api/DD64bTraceId.java‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,6 @@ public static DD64bTraceId from(String s) throws NumberFormatException {
4646
return DD64bTraceId.create(LongStringUtils.parseUnsignedLong(s), s);
4747
}
4848

49-
/**
50-
* Like {@link #from(String)}, but returns {@code null} instead of throwing when the given {@code
51-
* String} is not a plain decimal unsigned 64 bit id. Stricter than {@link #from(String)}: a
52-
* leading {@code +} and non-ASCII digits are rejected.
53-
*
54-
* <p>Internal API for parsing untrusted input, such as propagation headers, on the request path.
55-
* Not intended for use outside the tracer.
56-
*
57-
* @param s String of unsigned 64 bit id
58-
* @return DDTraceId, or {@code null} if the {@code String} is not a valid id
59-
*/
60-
public static DD64bTraceId fromOrNull(String s) {
61-
int len = s == null ? 0 : s.length();
62-
long id = LongStringUtils.parseUnsignedLongOrSentinel(s, 0, len, 0L);
63-
if (id == 0L && !LongStringUtils.isUnsignedLongZero(s, 0, len)) {
64-
return null;
65-
}
66-
return DD64bTraceId.create(id, s);
67-
}
68-
6949
/**
7050
* Create a new {@code DDTraceId} from the given {@code String} hex representation of the unsigned
7151
* 64 bit id.

‎dd-trace-api/src/main/java/datadog/trace/api/internal/util/LongStringUtils.java‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -73,93 +73,7 @@ private static int firstNonZeroCharacter(CharSequence s, int start) {
7373
return firstNonZero;
7474
}
7575

76-
/**
77-
* Parse the decimal representation of the unsigned 64 bit long from the {@code String}.
78-
*
79-
* @param s String in decimal of unsigned 64-bits long.
80-
* @return long
81-
* @throws NumberFormatException
82-
*/
8376
public static long parseUnsignedLong(String s) throws NumberFormatException {
84-
long result = parseUnsignedLongOrSentinel(s, 0, s == null ? 0 : s.length(), 0L);
85-
if (result != 0L) {
86-
return result;
87-
}
88-
// Zero is ambiguous with the invalid sentinel. Let the strict parser decide: it accepts input
89-
// the non-throwing parser rejects (a leading '+', non-ASCII digits) and builds the exception.
90-
return parseUnsignedLongStrict(s);
91-
}
92-
93-
/**
94-
* Parse the decimal representation of the unsigned 64 bit long from the {@code CharSequence}
95-
* without throwing, for untrusted input such as propagation headers.
96-
*
97-
* <p>Accepts 1 to 20 ASCII digits only: no sign, no whitespace, no non-ASCII digits. An unsigned
98-
* 64 bit value uses every bit, so no return value can mean "invalid" on its own. Callers pick an
99-
* {@code ifInvalid} that is impossible or harmless for their use, e.g. {@code 0} for trace and
100-
* span ids. With {@code 0} as {@code ifInvalid}, {@link #isUnsignedLongZero} tells a real zero
101-
* apart from invalid input.
102-
*
103-
* @param s CharSequence containing the decimal digits
104-
* @param start the start index of the decimal value
105-
* @param len the length of the decimal value
106-
* @param ifInvalid the value returned for null, empty, malformed or out of range input
107-
* @return the parsed long, or {@code ifInvalid}
108-
*/
109-
public static long parseUnsignedLongOrSentinel(
110-
CharSequence s, int start, int len, long ifInvalid) {
111-
if (s == null || len <= 0 || len > 20 || start < 0 || start + len > s.length()) {
112-
return ifInvalid;
113-
}
114-
int end = start + len;
115-
// Signed 64 bits holds any 18 digit number, so the first 18 digits cannot overflow
116-
int fastEnd = Math.min(end, start + 18);
117-
long result = 0;
118-
int i = start;
119-
for (; i < fastEnd; i++) {
120-
int d = s.charAt(i) - '0';
121-
if (d < 0 || d > 9) {
122-
return ifInvalid;
123-
}
124-
result = result * 10 + d;
125-
}
126-
for (; i < end; i++) {
127-
int d = s.charAt(i) - '0';
128-
if (d < 0 || d > 9 || Long.compareUnsigned(result, MAX_FIRST_PART) > 0) {
129-
return ifInvalid;
130-
}
131-
long scaled = result * 10;
132-
result = scaled + d;
133-
if (Long.compareUnsigned(result, scaled) < 0) {
134-
return ifInvalid;
135-
}
136-
}
137-
return result;
138-
}
139-
140-
/**
141-
* Whether {@code s[start, start + len)} is a zero that {@link #parseUnsignedLongOrSentinel}
142-
* accepts: 1 to 20 {@code '0'} characters. Callers that use {@code 0} as the invalid sentinel
143-
* check this only when the parse returned {@code 0}.
144-
*
145-
* @param s CharSequence containing the decimal digits
146-
* @param start the start index of the decimal value
147-
* @param len the length of the decimal value
148-
* @return true if the range parses to zero, false if it is zero-valued only as the sentinel
149-
*/
150-
public static boolean isUnsignedLongZero(CharSequence s, int start, int len) {
151-
if (s == null || len <= 0 || len > 20 || start < 0 || start + len > s.length()) {
152-
return false;
153-
}
154-
for (int i = start, end = start + len; i < end; i++) {
155-
if (s.charAt(i) != '0') {
156-
return false;
157-
}
158-
}
159-
return true;
160-
}
161-
162-
private static long parseUnsignedLongStrict(String s) throws NumberFormatException {
16377
if (s == null) {
16478
throw new NumberFormatException("s can't be null");
16579
}

‎dd-trace-api/src/test/java/datadog/trace/api/DDTraceIdTest.java‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertNotNull;
5-
import static org.junit.jupiter.api.Assertions.assertNull;
65
import static org.junit.jupiter.api.Assertions.assertSame;
76
import static org.junit.jupiter.api.Assertions.assertThrows;
87

@@ -71,46 +70,6 @@ void failParsingIllegal64BitIdStringRepresentation(String stringId) {
7170
assertThrows(NumberFormatException.class, () -> DD64bTraceId.from(stringId));
7271
}
7372

74-
@TableTest({
75-
"scenario | stringId | expectedId ",
76-
"zero | '0' | DD64bTraceId.ZERO ",
77-
"one | '1' | DD64bTraceId.ONE ",
78-
"max | '18446744073709551615' | DD64bTraceId.MAX ",
79-
"long max | '9223372036854775807' | DD64bTraceId.LONG_MAX",
80-
"long max plus one | '9223372036854775808' | DD64bTraceId.LONG_MIN"
81-
})
82-
@ParameterizedTest(name = "fromOrNull parses valid 64-bit id String representation [{index}]")
83-
void fromOrNullParsesValid64BitIdStringRepresentation(String stringId, DD64bTraceId expectedId) {
84-
DD64bTraceId ddid = DD64bTraceId.fromOrNull(stringId);
85-
86-
assertEquals(expectedId, ddid);
87-
assertEquals(stringId, ddid.toString());
88-
}
89-
90-
@Test
91-
void fromOrNullKeepsTheParsedStringForToString() {
92-
String stringId = "1234567890";
93-
94-
assertSame(stringId, DD64bTraceId.fromOrNull(stringId).toString());
95-
}
96-
97-
@ParameterizedTest(name = "fromOrNull returns null for illegal 64-bit id String [{index}]")
98-
@NullSource
99-
@ValueSource(
100-
strings = {
101-
"",
102-
"-1",
103-
"+1",
104-
"18446744073709551616",
105-
"18446744073709551625",
106-
"184467440737095516150",
107-
"18446744073709551a1",
108-
"184467440737095511a"
109-
})
110-
void fromOrNullReturnsNullForIllegal64BitIdString(String stringId) {
111-
assertNull(DD64bTraceId.fromOrNull(stringId));
112-
}
113-
11473
@TableTest({
11574
"scenario | hexId | expectedId ",
11675
"zero | '0' | DD64bTraceId.ZERO ",

‎dd-trace-api/src/test/java/datadog/trace/api/internal/util/LongStringUtilsTest.java‎

Lines changed: 0 additions & 154 deletions
This file was deleted.

0 commit comments

Comments
 (0)