Skip to content

Fix async parser decoding of short ASCII values split across input feeds - #770

Merged
cowtowncoder merged 2 commits into
2.21from
tatu-claude/2.21/async-smile-short-ascii-chunk
Sep 5, 2026
Merged

Fix async parser decoding of short ASCII values split across input feeds#770
cowtowncoder merged 2 commits into
2.21from
tatu-claude/2.21/async-smile-short-ascii-chunk

Conversation

@cowtowncoder

@cowtowncoder cowtowncoder commented Sep 5, 2026

Copy link
Copy Markdown
Member

Problem

In NonBlockingByteArrayParser._finishToken(), the MINOR_VALUE_STRING_SHORT_ASCII /
MINOR_VALUE_STRING_SHORT_UNICODE case picks its decoder with:

String text = (_minorState == MINOR_FIELD_NAME_SHORT_ASCII)
        ? _decodeASCIIText(_inputCopy, 0, fullLen)
        : _decodeShortUnicodeText(_inputCopy, 0, fullLen);

MINOR_FIELD_NAME_SHORT_ASCII is 5; the two case labels are 16 and 17. The
condition can therefore never be true, and short ASCII values that span a feed
boundary are always decoded as short Unicode.

For well-formed ASCII content both decoders agree, so this is invisible in normal use.
For content that is not valid ASCII, the same document decodes to different Strings
depending on how the caller chunks the input — a 0xC5 byte inside a short-ASCII token
yields aïŋ―cd when fed contiguously but aÅĢd when fed one byte at a time.

The equivalent check for property names (line 233) tests the right constant and is
left unchanged. These are the only two such comparisons in the async parser; the third
(MINOR_HEADER_INLINE, line 396) is reachable and correct. CBOR has no async parser on
2.x, so Smile is the only affected backend.

Fix

Compare against MINOR_VALUE_STRING_SHORT_ASCII.

Test

SimpleStringArrayTest.testShortAsciiValueChunkIndependence writes a short ASCII value,
corrupts one content byte so the two decoders disagree, then asserts that feed sizes of
length+1, 3 and 1 all produce the same String. It fails on the unfixed code with
expected: <aïŋ―cd> but was: <aÅĢd>.

Full smile module suite: 270 tests, 0 failures. CI green on JDK 8/17/21.

Note for merge-forward

3.x already carries this same one-token fix via #767, so the merge-forward will conflict
on that line at 3.x (either side is fine to take). 3.1 and 3.2 still have the bug and do
need it. The test will also need the 3.x API names (SmileReadFeature, no IOException)
when it reaches the 3.x side.

ðŸĪ– Generated with Claude Code

https://claude.ai/code/session_01BbhNqemoB4csempvUS9n5z

cowtowncoder and others added 2 commits September 4, 2026 17:35
`NonBlockingByteArrayParser._finishToken()` compared `_minorState` against
`MINOR_FIELD_NAME_SHORT_ASCII` inside the `MINOR_VALUE_STRING_SHORT_ASCII` /
`MINOR_VALUE_STRING_SHORT_UNICODE` case. Those constants can never be equal,
so the check was always false and short ASCII *values* that span a feed
boundary were always decoded via `_decodeShortUnicodeText()`.

Result: the same document could decode to different Strings depending on how
the caller chunked the input. The equivalent check for property names (line
233) is correct and is left as-is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BbhNqemoB4csempvUS9n5z
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BbhNqemoB4csempvUS9n5z
@cowtowncoder cowtowncoder changed the title (smile) Fix async parser decoding of short ASCII values split across input feeds Fix async parser decoding of short ASCII values split across input feeds Sep 5, 2026
@cowtowncoder cowtowncoder added this to the 2.21.7 milestone Sep 5, 2026
@cowtowncoder
cowtowncoder merged commit 7f6d379 into 2.21 Sep 5, 2026
3 checks passed
@cowtowncoder
cowtowncoder deleted the tatu-claude/2.21/async-smile-short-ascii-chunk branch September 5, 2026 00:43
cowtowncoder added a commit that referenced this pull request Sep 5, 2026
Also add 2.22.3 release notes entry for #770.
cowtowncoder added a commit that referenced this pull request Sep 5, 2026
Resolve #770 conflict in favor of the fix (3.1 still had the always-false
`MINOR_PROPERTY_NAME_SHORT_ASCII` comparison), port the regression test to
3.x API, and add 3.1.7 release notes entry for #770.
cowtowncoder added a commit that referenced this pull request Sep 5, 2026
Also add 3.2.3 release notes entry for #770.
cowtowncoder added a commit that referenced this pull request Sep 5, 2026
#767 had already applied the same one-token fix here, so the parser merged
cleanly. Drop the duplicate copy of `testShortAsciiValueChunkIndependence`
(3.x got one via #767, 3.2 brought another) and trim the #767 release-notes
entry, since the split-feed fix now ships as #770 in 3.2.3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant