Repository navigation
Strip a leading byte order mark once, not twice, for an encoding with a preamble [patch] - #147
Merged
Merged
Conversation
… a preamble [patch] StreamReader skips the encoding's own preamble whatever detectEncodingFromByteOrderMarks says, and AsyncProcessStreamReader then dropped a leading U+FEFF from the first chunk as well. For Encoding.UTF8 (OutputHandler's default), Encoding.Unicode and the other encodings with a preamble, output starting with a BOM followed by a real U+FEFF lost both, so the same bytes decoded differently through Encoding.UTF8 and new UTF8Encoding(false). The manual strip now runs only for an encoding with no preamble, where the reader leaves the BOM in place. Fixes #145 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW45a4XVwC8wG7P95kY
|
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 #145
What was wrong
AsyncProcessStreamReaderbuilds its readers withdetectEncodingFromByteOrderMarks: falseand then removes one leading U+FEFF itself inStripLeadingByteOrderMark. ButStreamReaderskips the encoding's own preamble whatever that flag says. So for an encoding with a preamble, which includesEncoding.UTF8(theOutputHandlerdefault) andEncoding.Unicode, the reader dropped the BOM and the manual strip then dropped a real U+FEFF after it.EF BB BF EF BB BF 41came out asAthroughEncoding.UTF8but asAthroughnew UTF8Encoding(false).What changed
The fix uses the first option the issue suggests. The manual strip now runs only when
outputHandler.Encoding.GetPreamble()is empty, which is the only case where the reader leaves the BOM in place. Either way, exactly one BOM is removed. The doc comment onStripLeadingByteOrderMarkand the field's comment explain why.Tests
Two new data-driven tests cover
Encoding.UTF8,new UTF8Encoding(false),Encoding.Unicodeandnew UnicodeEncoding(false, false):OnlyTheByteOrderMarkIsStrippedWhenARealU_FEFFFollowsIt: BOM + U+FEFF +AdeliversFEFF 0041for every encoding. It asserts on code points, so the kept U+FEFF is visible in a failure. That is the issue's "both deliverA", read literally: the BOM goes and the real U+FEFF stays.ALoneByteOrderMarkIsStrippedWhicheverEncodingReadsIt: a single BOM +hellodelivershellofor every encoding. This guards against overcorrecting into stripping nothing.Before the fix, the doubled-BOM test failed for
Encoding.UTF8andEncoding.Unicode, the two encodings with a preamble, and passed for the other two. After the fix, the full suite passes on net10.0 / Linux: 98 passed, 0 failed, 2 elevation tests skipped as usual. The existing single-BOM test from #76 still passes.Generated by Claude Code