Decode streamed upstream bytes statefully so multi-byte UTF-8 characters split across chunks are not replaced with U+FFFD (#365) - #367
Open
yualice97 wants to merge 1 commit into
Conversation
…cter split across chunks is not replaced with U+FFFD (cnighswonger#365) streamResponse() called chunk.toString() per chunk, which decodes a trailing partial code point as replacement characters. Use StringDecoder so the carry survives between chunks, and flush it at end-of-stream. Adds a test that cuts the wire inside a CJK character and inside an emoji (red on the merge base, green here). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #365.
What
streamResponse()inproxy/stream.mjsdecoded each upstream chunk independently withchunk.toString(). A multi-byte UTF-8 character that straddles a chunk boundary (any CJK character, most emoji, full-width punctuation) was emitted as two or threeU+FFFDreplacement characters inside the SSE JSON. That reaches Claude Code as real content — reply text, tool-call arguments, and the session transcript — and nothing downstream can repair it.This switches the decode to Node's
StringDecoder, which carries the trailing partial code point betweenwrite()calls, and flushes it withdecoder.end()after the upstream stream closes.Size
Three lines of production code plus an import.
TextDecoderwith{ stream: true }would be equivalent;StringDecoderwas chosen because the file already works with Buffers.Evidence (red first)
test/proxy-stream-utf8-boundary.test.mjsbuilds onecontent_block_deltaevent on the wire and cuts the byte stream inside a CJK character, inside a 4-byte emoji, and one byte per chunk. It runs through the realstreamResponse()withextSnapshot = [], so the only transform on the changed path is the decode.031d5e0): 3 of 5 fail —not ok 2/3/4, the split-character cases.node --test, Node 20.20): 1952 tests, 1945 pass, 0 fail, 7 skipped.The fifth test pins the end-of-stream behaviour you asked for in #365: a stream that ends mid-character flushes the dangling bytes as
U+FFFDrather than dropping them silently.Follow-up grep (#365, point 2)
Checked the other byte paths in the package for the same class of bug:
proxy/server.mjscollects request and response bodies withchunks.push(chunk)+Buffer.concat()and decodes once — safe.proxy/upstream.mjs— no per-chunk decode.bin/claude-via-proxy.mjs:165doesoutput += chunk.toString(), but only on the proxy's own stdout to match the ASCIIlistening online. Harmless in practice; left alone to keep this PR minimal.Non-Functional Requirements
StringDecoderis a thin stateful wrapper over the same decode; no measurable cost.🤖 Generated with Claude Code