Anchor the utf-8-variants regexes at the true end of the buffer (fixes #236) - #237
Open
sebollin wants to merge 1 commit into
Open
Anchor the utf-8-variants regexes at the true end of the buffer (fixes #236)#237sebollin wants to merge 1 commit into
sebollin wants to merge 1 commit into
Conversation
Python's $ also matches before a trailing newline, so a \n at the end of the input was consumed as part of a CESU-8 sequence or a Java null. Use \Z, which only matches at the very end, keeping the intent for truncated input in the streaming decoder. Fixesrspeer#236
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 freeto 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#236.
CESU8_EXPR,SURROGATE_EXPRandNULL_EXPRuse$where they mean "the buffer ends here", but in Python$also matches just before a trailing newline. A\nat the end of the input therefore gets consumed as part of the match:The second one is the bad case:
_buffer_decode_surrogatesbuilds the codepoint frominput[5] & 0x3F, so the newline byte becomes the low six bits and an invalid five-byte sequence decodes to U+1F60A instead of raising.This replaces the seven
$anchors with\Z, which only matches at the very end of the string. The truncated-input behaviour the$alternatives exist for is unchanged, since a truncated buffer ends where\Zmatches.After the change both examples raise
UnicodeDecodeError, andftfy.fix_encoding("Hi guys í ½í¸\n")keeps the newline instead of returning'Hi guys 😊'.Testing. The existing suite passes exactly as it does on
main(336 passed, 10 xfailedundertests/; the same 7 environment-related failures appear with and without the patch on my machine — the CLI entry point and onewcwidthdoctest).I also ran a differential battery of 65,268 byte sequences — the full sweep of
ED xx yy,C0followed by every byte, six-byte CESU-8 candidates, truncations, overlong forms, valid UTF-8 mixed with CESU-8, and ~30k noise strings weighted towards0xED/0xC0— comparing the codec before and after. Exactly four sequences change, and all four are the bug:Sequences that should still decode are untouched, including a newline after a complete sequence (
b"\xed\xa0\xbd\xed\xb8\x8d\n"andb"\xc0\x80\n"), and feeding the incremental decoder one byte at a time gives the same result as decoding the whole buffer.Found while porting the codec to R for lupa, which needs CESU-8 decoding without a Python dependency.