Reset TwoByteMatcher partial match on mismatching byte - #37053
Conversation
|
A note on where this belongs, to help with triage: although the change is in With the default The behaviour dates back to the matcher refactoring in gh-25915 (2020). Happy to adjust the fix or the tests if a different shape is preferred. |
DataBufferUtils.TwoByteMatcher inherited AbstractNestedMatcher.match(byte) without providing the mismatch fallback that its siblings implement (KnuthMorrisPrattMatcher backtracks via its suffix-prefix table, and SingleByteMatcher is stateless). As a result, once the first delimiter byte had matched, the match counter stayed at 1 across any number of intervening non-matching bytes, so a later occurrence of the second delimiter byte falsely completed the match. For a two-byte delimiter such as \r\n this made the matcher report a match across non-contiguous bytes. CompositeMatcher prefers the longest delimiter that matches at a position, so the false \r\n match was chosen over a real single \n, causing StringDecoder to strip two bytes and drop the character preceding a lone \n whenever a line contained a stray \r. TwoByteMatcher now overrides match(byte) to reset the counter to 0 when the incoming byte is not the expected next delimiter byte before delegating to super.match(), mirroring KnuthMorrisPrattMatcher. A genuine contiguous delimiter is unaffected. Signed-off-by: junhyeong9812 <pickjog@gmail.com>
2cd3d8c to
7f1966f
Compare
|
Thanks @junhyeong9812 ! |
|
Thank you for the review and merge, @bclozel. I appreciate you taking the time on it. |
Overview
DataBufferUtils.TwoByteMatcherdoes not reset its partial-match counter when a byte fails to match after the first delimiter byte has matched. As a result the matcher reports a delimiter match across non-contiguous bytes, which causesStringDecoder(and any consumer ofDataBufferUtils.matcher(byte[]...)) to silently drop a character whenever a lone\rappears inside a line. This PR adds the missing reset so the two-byte matcher only matches a contiguous delimiter.Problem
AbstractNestedMatcher.match(byte)deliberately leaves the fallback (what to do on a mismatch mid-match) to its subclasses:KnuthMorrisPrattMatcheroverridesmatch(byte)to backtrack via its suffix-prefix table, andSingleByteMatcheris stateless.TwoByteMatcher, however, inherited the base method without providing any fallback, so after the first delimiter byte matched (matches == 1) the counter stayed at 1 across any number of non-matching bytes, and a later occurrence of the second delimiter byte falsely completed the match.For the default delimiters used by
StringDecoder.allMimeTypes()(\r\nand\n), decoding"a\rXY\nb":a\rXYa\rXCompositeMatcherprefers the longest delimiter that matches at a position, so the false\r\nmatch (length 2) is chosen over the real\n(length 1), and the byte before\nis consumed as part of the delimiter and lost.Fix
TwoByteMatchernow overridesmatch(byte)to reset the counter to0when the incoming byte is not the expected next delimiter byte, then delegates tosuper.match(b)— mirroring the structure ofKnuthMorrisPrattMatcher(itswhileloop collapses to a single reset because a two-byte delimiter can only be in amatches == 1partial state). A genuine contiguous delimiter is unaffected: when the byte is the expected one the reset is skipped and the match completes.Added two tests: a unit test asserting
DataBufferUtils.matcher("\r\n")returns-1for non-contiguous input, and aStringDecodertest asserting the character before a lone\nis preserved.