fix: Drop invalid UTF-8 scanner candidates - #20389
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe scanner replaced unsafe UTF-8 conversions with checked conversions. Invalid UTF-8 candidates are discarded during full, incremental, and generic extraction. Tests cover invalid candidate filtering, valid and Unicode candidate preservation, position tracking, and CSS variable extraction with invalid UTF-8 input. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Confidence Score: 5/5The PR appears safe to merge; malformed UTF-8 candidates are omitted without changing extraction, restoration, or position semantics for surviving candidates. The checked conversions operate independently on extracted tokens, cover persistent scanner-state and positioned-candidate paths, and preserve the existing byte-offset and legacy restoration logic. Reviews (1): Last reviewed commit: "fix: address self-review findings" | Re-trigger Greptile |
|
Saw this was created after my fix comment #20368 on the issue so I ran the repro harness against this branch to check it closes what I found there. On main 4be6110 the four invalid-byte cases fail as before, lone Two things worth flagging while you're here though:
|
Summary
Replace the three unchecked scanner conversions in
crates/oxide/src/scanner/mod.rswith checked conversions that omit only extracted slices that are not valid UTF-8. Apply the check in the sharedextractpipeline so initial scans, incrementalscan_contentcalls, and CSS-variable extraction cannot insert invalid strings into scanner state, and apply the same policy to both branches ofget_candidates_with_positionswhile preserving byte offsets and the legacy-[]restoration. Keep the extractor's byte-oriented CSS identifier classification unchanged: accepting non-ASCII bytes during extraction is useful for valid multibyte code points, while the conversion boundary is the authoritative place to enforce theStringcontract.The scanner currently converts extracted byte slices with unchecked UTF-8 constructors at the shared extraction boundary and both candidate-with-position branches. A source file containing a stray continuation byte can therefore produce an invalid
String, violating Rust's string invariant and allowing corrupted candidates to persist in a long-lived scanner. The thread provides a deterministic reproduction using invalid bytes inside an arbitrary value, so the problem no longer depends on reproducing the originally reported Turbopack race. Valid candidates found alongside malformed byte sequences must continue to be returned normally.Fixes #20368
Test plan
Not applicable to this change.
AI was used for assistance.