Skip to content

Preserve input encoding state across ResumableParser chunks - #1096

Closed
ydah wants to merge 1 commit into
ruby:masterfrom
ydah:fix-resumable-input-encoding
Closed

ydah wants to merge 1 commit into
ruby:masterfrom
ydah:fix-resumable-input-encoding

Conversation

@ydah

@ydah ydah commented Oct 5, 2026

Copy link
Copy Markdown
Member

ResumableParser transcodes each chunk independently, raising an encoding error when a character spans chunks.

require "json"

source = '["日本"]'.encode("UTF-16LE")
parser = JSON::ResumableParser.new
parser << source.byteslice(0, 5)
parser << source.byteslice(5..)
parser.parse
parser.value
# Before: Encoding::InvalidByteSequenceError on the first feed
# After: ["日本"]

Keep an Encoding::Converter across feeds and reset it on clear. Add coverage for split characters, encoding changes, and invalid sequences.

@byroot

byroot commented Oct 5, 2026

Copy link
Copy Markdown
Member

I don't follow. JSON is always UTF-8, as per the spec.

@ydah

ydah commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

I based this on the existing transcoding behavior. JSONEncodingTest#test_parse explicitly tests UTF-16 and UTF-32 input, and ResumableParser#<< also calls convert_encoding.

This PR addresses that conversion failing when a character spans chunks. Is transcoding intentional for ResumableParser, or should it only accept UTF-8 input? I apologize if I've misunderstood something.

@byroot

byroot commented Oct 5, 2026

Copy link
Copy Markdown
Member

JSONEncodingTest#test_parse explicitly tests UTF-16 and UTF-32 input, and ResumableParser#<< also calls convert_encoding.

Yeah, those are old tests, I wouldn't have added them myself. ResumableParser being a new API, I don't think we have to necessarily follow the same path. The use case for encoding other than UTF-8 is really very unclear, so I'd rather not add any overhead.

I'm open to eagerly validating / rejecting that case though.

@ydah

ydah commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

I see. I understand now, and I agree with your point. Thanks for explaining it. I'll close this PR.

@ydah ydah closed this Oct 5, 2026
@ydah
ydah deleted the fix-resumable-input-encoding branch October 5, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants