KAFKA-20892: SimpleHeaderConverter should not infer a number from a string with trailing tokens - #23496
KAFKA-20892: SimpleHeaderConverter should not infer a number from a string with trailing tokens#23496psiby wants to merge 3 commits into
Conversation
…tring with trailing tokens
| @@ -868,11 +868,15 @@ private SchemaAndValue parseNextToken(boolean embedded, String token) { | |||
| return temporal; | |||
There was a problem hiding this comment.
It may be considered out of scope, but the temporal branch has the same problem: no check that all tokens are consumed. I leave it up to you to add this fix in the same PR or create another PR.
There was a problem hiding this comment.
Good catch — fixed it here with the same guard, so strings like 2020-01-01:foo now parses as a string (test added). While checking, I found arrays/maps had the same issue too ([1,2]foo was read as [1,2], dropping the foo), so I guarded those as well. Happy to split any of it into a separate ticket if you'd prefer, please let me know.
There was a problem hiding this comment.
Perfect!
Personally, I don't think it worth additional overhead of another PR. But someone with commit permissions should have a look anyway.
…ranch Expand the trailing-token test coverage per review, and apply the same `embedded || !parser.hasNext()` guard to the temporal branch so a value like "2020-01-01:foo" is not misread as the date 2020-01-01.
Apply the same consumed-everything guard to the array and map branches so a value like "[1,2]foo" is not misread as the array [1, 2]. Trailing whitespace is still allowed, matching the existing whitespace-tolerant map/array parsing.
|
|
||
| @Test | ||
| public void shouldParseTemporalStringsWithTrailingTokensAsStrings() { | ||
| for (String value : List.of("2020-01-01:foo", "2020-01-01,x")) { |
There was a problem hiding this comment.
I would also add other time formats to test:
2020-02-03T11:12:13.145Z}15:16:17.189Z,
SimpleHeaderConverterdeserializes header values viaValues.parseString, which infers a type from the first token only. A top-level value like1::2was parsed as the number1, silently dropping::2.The parser already guards against this for the
null/true/falseliterals:canParseSingleTokenLiteralonly accepts a literal when it is embedded or consumes the entire input. The number branch inparseNextTokenhad no equivalent guard.Fix: only treat a token as a number when
embedded || !parser.hasNext(), so a non-embedded value that leaves trailing tokens falls back to a string. Embedded values (array/map elements) are unaffected.Tests:
ValuesTest.shouldParseStringsBeginningWithNumberAsStrings—parseString("1::2")returns(STRING, "1::2").SimpleHeaderConverterTest.shouldConvertStringBeginningWithDigitFollowedByDelimiters— round-trips"1::2"as a string.JIRA: https://issues.apache.org/jira/browse/KAFKA-20892
Reviewers: Yuriy Badalyantc lmnet89@gmail.com