Simplify decode() and validate UTF-8 continuation bytes#17
Merged
Conversation
Remove the internal for_each() helpers and let decode() call decode_codepoint() directly. This drops the duplicated scan that for_each() and decode_codepoint() each performed, simplifying the code and speeding up decoding (~20% for UTF-16, ~30% for UTF-8). The helpers were used nowhere outside decode(). Based on #15 by Sam Hocevar. Also validate continuation bytes in the UTF-8 decode_codepoint(). Without the for_each() grouping, an invalid lead byte would otherwise consume the following bytes unconditionally (they were only masked, never checked), swallowing trailing valid characters -- e.g. "\xE0AB" decoded to a single bogus code point instead of "AB". Rejecting bytes that are not 0x80-0xBF makes decode() resync byte-by-byte and recover the following text, matching the behaviour of ICU, Python, Rust and browsers (minus U+FFFD emission). The UTF-16 path already validated surrogate pairs and is unchanged in behaviour.
Cover that an invalid UTF-8 lead byte resyncs at the next byte instead of swallowing the following valid characters, that decode_codepoint() reports failure on bad continuation bytes, and the matching lone-surrogate resync for UTF-16. These fail against the un-validated decode and pass with it.
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.
Summary
Builds on #15 by @samhocevar, with one correctness fix added.
decode()— remove the internalfor_each()helpers (UTF-8 and UTF-16) and calldecode_codepoint()directly. This drops the duplicated byte scan and speeds up decoding by roughly 20% (UTF-16) and 30% (UTF-8). The helpers were used nowhere outsidedecode().decode_codepoint(). Without thefor_each()grouping, an invalid lead byte would consume the following bytes unconditionally — they were only masked with& 0x3F, never checked — and swallow trailing valid characters. Rejecting bytes that are not0x80–0xBFmakesdecode()resync byte-by-byte and recover the following text.Why the validation is needed
The bare simplification in #15 changes behaviour on invalid UTF-8 (valid input is unaffected):
E0 41 42B(swallowsA, bogus cp)AB<U+FFFD>ABF0 'Hello'<U+896C>lo(swallowsHel)Hello<U+FFFD>HelloE1 80 41<U+1001>(swallowsA)A<U+FFFD>AEvery mainstream decoder validates continuation bytes and never swallows the following valid characters. This change brings the resync behaviour in line with them. (The library still silently drops invalid bytes rather than emitting U+FFFD — that existing design choice is preserved; this PR is not a full Table 3-7 validator.)
Tests
Added regression tests (
test/test.cpp):decode invalid utf8 resync— invalid lead bytes resync and recover the following text instead of swallowing it.decode_codepoint rejects bad continuation bytes—decode_codepoint()returns 0 on bad continuation bytes, and still accepts well-formed sequences.utf16 decode invalid resync— a lone high surrogate resyncs without swallowing the next unit.These fail against the un-validated
decode()and pass with the fix.Verification
encode → decoderound-trip: 0 failures (UTF-8 and UTF-16), so no regression on valid input.errors='replace', U+FFFD stripped): 98.3% exact match; every remaining difference is the library's pre-existing permissiveness (overlong / surrogate-encoded / out-of-range), with no unexplained mismatches.-Wall -Wextra -Wshadow.