Add input validation to encodings and property table lookups - #18
Merged
Conversation
Addresses the issues and inconsistencies reported in #16: - utf16::is_surrogate_pair() no longer reads s16[1] out of bounds when l is 1 (heap-buffer-overflow confirmed with ASan on the old code). - All eleven generated get_value() functions now return the default value for code points beyond U+10FFFF instead of indexing past the end of _blocks[] (global-buffer-overflow confirmed with ASan, e.g. is_letter(0x110000)). The guard is emitted by gen_tables.py, so the tables remain reproducible; regeneration is byte-identical apart from the guards. - utf8::decode_codepoint() now rejects ill-formed sequences per Table 3-7 of the Unicode Standard: overlong encodings, surrogates (CESU-8 style), and code points beyond U+10FFFF. decode() therefore never emits a non-scalar value, matching ICU, Python, Rust and Go. - utf16::decode_codepoint() now rejects unpaired low surrogates, as it already did for unpaired high surrogates. - utf8::codepoint_length(s8, l) is now consistent with decode_codepoint(): it returns 0 for ill-formed or truncated input instead of a length that may exceed l. - utf16::codepoint_length() is now consistent with encode_codepoint() and decode_codepoint(): 0 for surrogates and values beyond U+10FFFF (char32_t overload), and 0 for unpaired surrogates (char16_t* overload). - codepoint_count() skips ill-formed units the same way decode() does. This also fixes an infinite loop: the old code advanced by codepoint_length(), which could return 0 (e.g. on a lone continuation byte), so codepoint_count("a\x80b", 3) never returned. - Add is_scalar_value(), the validity check proposed in #16. Named is_scalar_value rather than is_scalar because the latter is ambiguous with std::is_scalar in code that uses both namespaces. Verified with the existing test suite plus new regression tests, an exhaustive encode/decode round-trip over all 1,112,064 scalar values, and a 200k-case fuzz comparison against Python's UTF-8 decoder, which now agrees on every input (modulo U+FFFD emission, which this library intentionally does not do).
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 #16.
Summary
Implements the sanity checks proposed by @samhocevar in #16, plus a couple of related fixes found along the way. After this change, the decoders never produce a non-scalar
char32_t, the property lookups are safe for anychar32_tinput, and thecodepoint_length/codepoint_countfamily is consistent withencode_codepoint/decode_codepoint.Memory safety (both confirmed with ASan on the old code)
utf16::is_surrogate_pair()reads16[1]out of bounds whenl == 1(heap-buffer-overflow). Now requiresl >= 2.general_category(),is_letter(),script(), …) indexed past the end of_blocks[]for code points beyond U+10FFFF (global-buffer-overflow, e.g.is_letter(0x110000)). All eleven generatedget_value()functions now return the default value (Cn/Unassigned/0) for out-of-range input — the same approach as ICU'su_charType(), Java'sCharacter.getType(int), and Go'sunicode.IsLetter. The guard is emitted bygen_tables.py, so the tables remain reproducible (regeneration is byte-identical apart from the guards).Decoder validation
utf8::decode_codepoint()now rejects ill-formed sequences per Table 3-7 of the Unicode Standard: overlong encodings (C0 AF, …), surrogates (ED A0 80, …), and code points beyond U+10FFFF (F4 90 80 80,F5–F7leads).decode()therefore never emits a non-scalar value, matching ICU / Python / Rust / Go.utf16::decode_codepoint()now rejects unpaired low surrogates, as it already did for unpaired high surrogates.API consistency (the inconsistencies listed in #16)
utf8::codepoint_length(s8, l)returns 0 for ill-formed or truncated input instead of a length that may exceedl; it now reports exactly whatdecode_codepoint()would consume.utf16::codepoint_length(char32_t)returns 0 for surrogates and values beyond U+10FFFF, consistent withencode_codepoint().utf16::codepoint_length(const char16_t*, l)returns 0 for unpaired surrogates, consistent withdecode_codepoint().codepoint_count()(both encodings) skips ill-formed units the same waydecode()does, socodepoint_count(s) == decode(s).size()always holds. This also fixes an infinite loop: the old code advanced bycodepoint_length(), which could return 0, so e.g.utf8::codepoint_count("a\x80b", 3)never returned.New helper
is_scalar_value(char32_t)— the validity check proposed in Input validation questions #16 (documented in the README). Namedis_scalar_valuerather thanis_scalarbecause the latter is ambiguous withstd::is_scalarin code that uses bothnamespace stdandnamespace unicode. (The snippet in Input validation questions #16 also had a small typo:0xD8000for0xD800.)Behavioural note
utf8::decode()previously passed through CESU-8-style surrogates, overlong forms, andF5–F7sequences as garbage code points; it now silently drops them, like every mainstream decoder. The library's existing design of not emitting U+FFFD is unchanged.Tests
13 new test cases: scalar-value checks, out-of-range property lookups, rejection of overlong/surrogate/out-of-range sequences with all boundary cases that must stay valid (U+0080, U+0800, U+D7FF, U+E000, U+10000, U+10FFFF), resync behaviour,
codepoint_length/codepoint_countconsistency, and theis_surrogate_pairbounds case.Verification
encode → decoderound-trip over all 1,112,064 scalar values: 0 failures (no regression on valid input);codepoint_length(char32_t)agrees withencode_codepoint()for every code point.errors='replace', U+FFFD stripped): 100% agreement (before this change: 98.3%, the gap being the now-rejected ill-formed forms).codepoint_count == decode().size()andcodepoint_length == decode_codepointhold at every position (run under ASan).-Wall -Wextra -Wshadow.