Repository navigation
feat: invalid UTF-8 bytes match as U+FFFD width 1 - #182
Merged
Merged
Conversation
Go 1.27.2 uses export data version 5, golangci-lint v2.13 supports only version 4. CI failed on v0.12.27 merge commit.
Phase 1, steps 1+3: foundation for invalid UTF-8 handling. - dfa/lazy/state.go: rename tagReserved (bit 29) to tagQuit, add QuitState constant and IsQuitTag() method. Signals DFA cannot handle the current byte — search must fall back to NFA. - nfa/nfa.go: add LookInvalidUTF8 look-around assertion. Two-part semantics: position is at rune boundary AND utf8.DecodeRune returns (RuneError, 1). Fix lookNames array to include all 7 look kinds. No behavioral changes — constants and types only.
Phase 1, step 2: add QuitState fallback to all 9 DFA search switch statements. On QuitState, the DFA falls back to PikeVM for the current find call (find-level fallback, not byte-level). Forward searches: nfaFallback(haystack, startPos) or pikevm.SearchAt Reverse searches: nfaFallbackReverse(haystack, start, end) or pikevm.Search No behavioral changes yet — nothing produces QuitState transitions. That comes in step 4 (temporary 3c quit on bytes >= 0x80).
Phase 1, step 4: temporary 3c fallback for invalid UTF-8 handling. - DFA struct: add hasInvalidUTF8Look flag, scanned at build time - Builder: add checkHasInvalidUTF8Look() (same pattern as word boundary) - determinize: if hasInvalidUTF8Look && b >= 0x80, set QuitState in flatTrans and return NFAFallback error. Search loop falls back to PikeVM. No behavioral changes yet — no NFA currently emits LookInvalidUTF8. Activates when NFA compiler inserts Look-gated fallback (phase 2, step 6). This is the temporary 3c approach: correct immediately on all inputs, slow on non-ASCII text. Phase 3 replaces with UTF-8 validator product.
…179) Phase 1 hardening after self-review: - lazy.go: 4x-unrolled fast paths (searchEarliestMatch, searchAt) used `nextID != InvalidState && nextID != DeadState` which would treat QuitState as valid transition. Replaced with `!nextID.IsTagged()` — catches all tagged states (Invalid/Dead/Quit/future) in one check. - cache.go: safeOffset() and getState() now guard against QuitState alongside DeadState/InvalidState. Without this, QuitState would extract garbage offset from tag bits. - builder.go: DetectAccelerationFromFlat skips QuitState transitions (same as DeadState — not a real transition to another state). - quit_test.go: unit tests that inject QuitState into flatTrans and verify DFA falls back to PikeVM with correct results. Tests cover Find (forward), IsMatch, and tag bit semantics.
Phase 1 review fixes (3 must-fix): 1. SearchAtAnchored QuitState used unanchored nfaFallback — would return matches at wrong positions. Added nfaFallbackAnchored that verifies start == startPos. Test: `b+` at position 0 of "ab" with quit on 'a' must return -1, not 2. 2. nfaFallbackReverse sliced haystack (pikevm.Search(haystack[start:end])) which loses left context needed for \b, (?m)^, and LookInvalidUTF8 rune-boundary check. Replaced with pikevm.SearchBetween(haystack, start, end) which preserves full context. IsMatchReverse QuitState handler also fixed. 3. gofmt alignment on state.go constants (tagQuit shorter than tagReserved).
Phase 2, step 6: three insertion points for LookInvalidUTF8.
- compileUnicodeClassLarge: coversAllNonASCII branch always gets fallback
(U+FFFD is in any negated ASCII class). Non-covering branch checks
containsRuneFFFD(ranges) before adding.
- compileUTF8Any: replaced direct 0x80-0xFF/0xC0-0xC1/0xF5-0xFF
transitions with buildInvalidUTF8Fallback (Look-gated).
- compileUTF8AnySparse: removed inline invalid byte transitions from
sparse state. Added separate Look-gated branch via split chain.
New helpers:
- buildInvalidUTF8Fallback: LookInvalidUTF8 -> ByteRange(0x80,0xFF) -> end
- containsRuneFFFD: checks if U+FFFD is in rune ranges
NFA now emits LookInvalidUTF8 for '.', '(?s).', '\S', '\D', '\W',
'[^x]', '\P{Han}' — any class containing U+FFFD. The DFA's temporary
3c quit (phase 1) will activate for these patterns.
PikeVM/BT evaluation of the Look assertion is next (tasks #7, #8).
Phase 2, steps 7+8: add LookInvalidUTF8 case to checkLookAssertion. Both PikeVM and BoundedBacktracker use checkLookAssertion, so one change covers both engines. Two-part semantics: 1. Rune boundary: isInsideValidSequence() looks back up to 3 bytes for a valid UTF-8 lead byte covering this position. If found, the assertion fails (byte is inside a valid multi-byte sequence). Cost: only on continuation bytes (0x80-0xBF). 2. Invalid sequence: utf8.DecodeRune(haystack[pos:]) returns size==1 (RuneError). This catches orphan continuations, overlong leads (C0-C1), out-of-range bytes (F5-FF), and truncated sequences. New helpers: isInvalidUTF8Position, isInsideValidSequence, utf8LeadByteLen. Smoke test: all 7 D-cases now match stdlib. #174 and rune-boundary regression gates hold.
Phase 2, steps 7-9: complete invalid UTF-8 handling.
NFA engines:
- checkLookAssertion: add LookInvalidUTF8 case with two-part semantics:
rune boundary check (isInsideValidSequence) + utf8.DecodeRune == RuneError
- New helpers: isInvalidUTF8Position, isInsideValidSequence, utf8LeadByteLen
- Both PikeVM and BoundedBacktracker covered (shared checkLookAssertion)
NFA compiler:
- compileLiteral: U+FFFD compiles as (valid EF BF BD) OR (Look-gated 0x80-0xFF)
- compileRuneFFFD helper for the alternation
Acceptance tests (all 10 criteria from design doc):
- Criteria 1-4: negated classes match invalid bytes
- Criterion 5: literal U+FFFD matches invalid byte
- Criterion 6: literal U+FFFD matches valid encoding
- Criterion 7: \pL does NOT match invalid byte
- Criterion 9: dot matches each invalid byte as width 1
- Criterion 10: rune-boundary invariant ([^К]b on Кb = no match)
Removed t.Skip("#179") from fuzz_stdlib_test.go and regex_unicode_test.go.
Phase 2, step 10: fuzz hardening. - hasUTF8CodepointDifference: removed D-class patterns (dot, \D, \W, \S, [^x], .+, (.)+) from whitelist — all fixed by #174/#179. Only empty-match stepping patterns remain (class B/C: #175, #176). - seedInputs: added 7 invalid UTF-8 inputs (\xff, \x80, \xc0, embedded invalid, valid U+FFFD encoding, mixed valid+invalid). - TestInvalidUTF8NegatedCharClass: removed t.Skip, re-enabled. All 9 fuzz targets pass on seed corpus with invalid UTF-8 inputs.
Phase 2 review fix: OnePass regression. Problem: LookInvalidUTF8 in NFA caused OnePass builder to see overlapping byte ranges (0x80-0xFF from FFFD path vs multi-byte UTF-8 paths) → rejected as not-one-pass → 9-17x regression on FindSubmatch for log-parsing patterns. Fix: OnePass builder skips LookInvalidUTF8 in epsilon closure (avoids ambiguity), records FFFD targets separately. buildTransitions fills dead byte classes >= 0x80 with conditional FFFD transitions flagged via lookInvalidUTF8Bit in the look-around field. Search loop checks IsInvalidUTF8Position at runtime before taking conditional transitions. - builder.go: skip LookInvalidUTF8 in closure, record fffdTargets, fill dead byte classes with conditional FFFD transitions - transition.go: add lookInvalidUTF8Bit, NeedsInvalidUTF8Check() - search.go: runtime check before IsDead - onepass.go: runtime check in IsMatch - pikevm.go: export IsInvalidUTF8Position for cross-package use - gofmt: fix formatting on fuzz_stdlib_test.go, nfa/compile.go
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Benchmark ComparisonComparing Summary:
|
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
1 participant
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
Invalid UTF-8 bytes now match as U+FFFD (replacement character) width 1, matching Go stdlib semantics.
LookInvalidUTF8assertion gates single-byte0x80-0xFFtransitions behind a rune-boundary + DecodeRune check\x{FFFD}matches both valid encoding (EF BF BD) and invalid bytesTest plan
\S{2}on "К" = no match[^К]bon "Кb" = no match-raceclean on root + both DFA packages./[^x], no regressionFixes #179