From 1c6ca81b8a18c249c04446c6d494db0a95673e6a Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 5 Sep 2026 23:05:42 +0200 Subject: [PATCH] Fix dead ill-formed-fourth-byte UTF-8 test sections (byte3/byte4 typo) The "ill-formed: wrong fourth byte" SECTIONs in unit-unicode3.cpp, unit-unicode4.cpp, and unit-unicode5.cpp guarded their loop with a check on byte3 instead of byte4. Since the enclosing loop already restricts byte3 to its valid range, the guard was always true and the section's "continue" fired unconditionally, so check_utf8string()/check_utf8dump() were never actually invoked for a malformed fourth byte. Fixing the guard naively (byte3 -> byte4) would also have swept the full byte2 x byte3 combinatorics for every byte4 value, adding millions of redundant iterations: the lexer validates continuation bytes strictly in sequence with early exit (see next_byte_in_range() in lexer.hpp), so once byte2/byte3 are within their valid range, the byte4 outcome does not depend on which valid byte2/byte3 values were chosen. Instead, byte2 and byte3 are now held to a small hedge of representative valid prefixes (range corners plus a midpoint) while byte4 is still swept exhaustively over its full 0x00-0xFF range, since that is the actual property under test. Also fixed the garbled "skip fourth second byte" comment in unit-unicode3.cpp. Verified offline: before the fix, the "wrong fourth byte" subcase executes 0 assertions in all three files (proving it was dead code); after the fix, it executes 11520 (unicode3), 34560 (unicode4), and 11520 (unicode5) assertions, and a deliberately reintroduced bug in the lexer's byte4 range check causes it to fail (proving it is now meaningful). Total per-file assertion counts grow by the same small amounts, not by millions, and all other sections in these files still pass unchanged. Fixes #5416 Signed-off-by: Niels Lohmann --- tests/src/unit-unicode3.cpp | 43 ++++++++++++++++++++++++++----------- tests/src/unit-unicode4.cpp | 43 ++++++++++++++++++++++++++----------- tests/src/unit-unicode5.cpp | 43 ++++++++++++++++++++++++++----------- 3 files changed, 93 insertions(+), 36 deletions(-) diff --git a/tests/src/unit-unicode3.cpp b/tests/src/unit-unicode3.cpp index 739a3dad3..01ea484bd 100644 --- a/tests/src/unit-unicode3.cpp +++ b/tests/src/unit-unicode3.cpp @@ -296,23 +296,42 @@ TEST_CASE("Unicode (3/5)" * doctest::skip()) SECTION("ill-formed: wrong fourth byte") { + // The lexer (see next_byte_in_range() in lexer.hpp) validates the + // continuation bytes strictly in sequence and bails out on the first + // byte that is out of range. So once byte2 and byte3 are anywhere + // inside their own valid range, whether byte4 is accepted or rejected + // depends only on byte4's value -- not on which particular valid + // byte2/byte3 combination was used to reach it. Sweeping the full + // byte2 x byte3 combinatorics here (as the other "wrong Nth byte" + // sections do for the byte they target) would therefore add a huge + // number of iterations for zero additional coverage. Instead, byte2 + // and byte3 are held to a small hedge of representative valid + // prefixes -- the corners and midpoint of their valid ranges -- while + // byte4 is still swept exhaustively over 0x00-0xFF, since "byte4 out + // of range is rejected for every value it could take" is the actual + // property under test. If the UTF-8 decoder is ever reworked (e.g. + // into a table-driven/bulk scanner), this equivalence-class + // assumption should be re-audited. + static const int byte2_values[] = {0x90, 0x90, 0xBF, 0xBF, 0xA8}; + static const int byte3_values[] = {0x80, 0xBF, 0x80, 0xBF, 0xA0}; + for (int byte1 = 0xF0; byte1 <= 0xF0; ++byte1) { - for (int byte2 = 0x90; byte2 <= 0xBF; ++byte2) + for (size_t idx = 0; idx < sizeof(byte2_values) / sizeof(byte2_values[0]); ++idx) { - for (int byte3 = 0x80; byte3 <= 0xBF; ++byte3) - { - for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) - { - // skip fourth second byte - if (0x80 <= byte3 && byte3 <= 0xBF) - { - continue; - } + const int byte2 = byte2_values[idx]; + const int byte3 = byte3_values[idx]; - check_utf8string(false, byte1, byte2, byte3, byte4); - check_utf8dump(false, byte1, byte2, byte3, byte4); + for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) + { + // skip correct fourth byte + if (0x80 <= byte4 && byte4 <= 0xBF) + { + continue; } + + check_utf8string(false, byte1, byte2, byte3, byte4); + check_utf8dump(false, byte1, byte2, byte3, byte4); } } } diff --git a/tests/src/unit-unicode4.cpp b/tests/src/unit-unicode4.cpp index f7047201c..943e2ecb0 100644 --- a/tests/src/unit-unicode4.cpp +++ b/tests/src/unit-unicode4.cpp @@ -296,23 +296,42 @@ TEST_CASE("Unicode (4/5)" * doctest::skip()) SECTION("ill-formed: wrong fourth byte") { + // The lexer (see next_byte_in_range() in lexer.hpp) validates the + // continuation bytes strictly in sequence and bails out on the first + // byte that is out of range. So once byte2 and byte3 are anywhere + // inside their own valid range, whether byte4 is accepted or rejected + // depends only on byte4's value -- not on which particular valid + // byte2/byte3 combination was used to reach it. Sweeping the full + // byte2 x byte3 combinatorics here (as the other "wrong Nth byte" + // sections do for the byte they target) would therefore add a huge + // number of iterations for zero additional coverage. Instead, byte2 + // and byte3 are held to a small hedge of representative valid + // prefixes -- the corners and midpoint of their valid ranges -- while + // byte4 is still swept exhaustively over 0x00-0xFF, since "byte4 out + // of range is rejected for every value it could take" is the actual + // property under test. If the UTF-8 decoder is ever reworked (e.g. + // into a table-driven/bulk scanner), this equivalence-class + // assumption should be re-audited. + static const int byte2_values[] = {0x80, 0x80, 0xBF, 0xBF, 0xA0}; + static const int byte3_values[] = {0x80, 0xBF, 0x80, 0xBF, 0xA0}; + for (int byte1 = 0xF1; byte1 <= 0xF3; ++byte1) { - for (int byte2 = 0x80; byte2 <= 0xBF; ++byte2) + for (size_t idx = 0; idx < sizeof(byte2_values) / sizeof(byte2_values[0]); ++idx) { - for (int byte3 = 0x80; byte3 <= 0xBF; ++byte3) - { - for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) - { - // skip correct fourth byte - if (0x80 <= byte3 && byte3 <= 0xBF) - { - continue; - } + const int byte2 = byte2_values[idx]; + const int byte3 = byte3_values[idx]; - check_utf8string(false, byte1, byte2, byte3, byte4); - check_utf8dump(false, byte1, byte2, byte3, byte4); + for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) + { + // skip correct fourth byte + if (0x80 <= byte4 && byte4 <= 0xBF) + { + continue; } + + check_utf8string(false, byte1, byte2, byte3, byte4); + check_utf8dump(false, byte1, byte2, byte3, byte4); } } } diff --git a/tests/src/unit-unicode5.cpp b/tests/src/unit-unicode5.cpp index e4dcc2131..e3d647554 100644 --- a/tests/src/unit-unicode5.cpp +++ b/tests/src/unit-unicode5.cpp @@ -296,23 +296,42 @@ TEST_CASE("Unicode (5/5)" * doctest::skip()) SECTION("ill-formed: wrong fourth byte") { + // The lexer (see next_byte_in_range() in lexer.hpp) validates the + // continuation bytes strictly in sequence and bails out on the first + // byte that is out of range. So once byte2 and byte3 are anywhere + // inside their own valid range, whether byte4 is accepted or rejected + // depends only on byte4's value -- not on which particular valid + // byte2/byte3 combination was used to reach it. Sweeping the full + // byte2 x byte3 combinatorics here (as the other "wrong Nth byte" + // sections do for the byte they target) would therefore add a huge + // number of iterations for zero additional coverage. Instead, byte2 + // and byte3 are held to a small hedge of representative valid + // prefixes -- the corners and midpoint of their valid ranges -- while + // byte4 is still swept exhaustively over 0x00-0xFF, since "byte4 out + // of range is rejected for every value it could take" is the actual + // property under test. If the UTF-8 decoder is ever reworked (e.g. + // into a table-driven/bulk scanner), this equivalence-class + // assumption should be re-audited. + static const int byte2_values[] = {0x80, 0x80, 0x8F, 0x8F, 0x88}; + static const int byte3_values[] = {0x80, 0xBF, 0x80, 0xBF, 0xA0}; + for (int byte1 = 0xF4; byte1 <= 0xF4; ++byte1) { - for (int byte2 = 0x80; byte2 <= 0x8F; ++byte2) + for (size_t idx = 0; idx < sizeof(byte2_values) / sizeof(byte2_values[0]); ++idx) { - for (int byte3 = 0x80; byte3 <= 0xBF; ++byte3) - { - for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) - { - // skip correct fourth byte - if (0x80 <= byte3 && byte3 <= 0xBF) - { - continue; - } + const int byte2 = byte2_values[idx]; + const int byte3 = byte3_values[idx]; - check_utf8string(false, byte1, byte2, byte3, byte4); - check_utf8dump(false, byte1, byte2, byte3, byte4); + for (int byte4 = 0x00; byte4 <= 0xFF; ++byte4) + { + // skip correct fourth byte + if (0x80 <= byte4 && byte4 <= 0xBF) + { + continue; } + + check_utf8string(false, byte1, byte2, byte3, byte4); + check_utf8dump(false, byte1, byte2, byte3, byte4); } } }