Repository navigation
fix(scanner): reject invalid regexes at compile time - #27
Merged
Merged
Conversation
Lazy regex compilation (9dd9581) dropped the compile-time error for invalid regexes. A pattern RE2 rejects, such as a repetition above 1000, now compiles without error and silently never matches, and SkipInvalidRegex no longer does what its doc says. Check the final RE2 pattern with regexp/syntax in compileRegex. It covers the {,N} rewrite and hex strings, costs ~4ms for 1,140 regexes (vs ~250ms for a go-re2 compile), and keeps the go-re2 compile lazy.
regexp/syntax is stricter than RE2 in Latin-1 mode: it rejects raw bytes 0x80-0xFF and \C, and ignores a custom RegexCompiler. Such patterns compiled and matched before and would now fail Compile, or be silently dropped under SkipInvalidRegex. Keep the parse as the fast path and confirm any rejection with the configured compiler.
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.
Lazy regex compilation (9dd9581) dropped the compile-time error for invalid regexes. A pattern RE2 rejects, such as a repetition above 1000, now compiles without error and silently never matches, and
SkipInvalidRegexno longer does what its doc says.compileRegexnow callscheckRegexon the final pattern, which covers the{,N}rewrite and hex strings:regexp/syntax(fast path, RE2 syntax including the repetition limit).RegexCompilerand report its error. Go syntax is stricter than RE2 in Latin-1 mode (raw bytes 0x80-0xFF,\C), and a custom compiler may accept more, so the real compiler has the final say.The go-re2 compile for valid patterns stays lazy.
Performance
Compileon the ecomscan-signatures ruleset (26,739 rules, 1,150 regex patterns), 10 interleaved runs each, benchstat:Compile.SkipInvalidRegex, an invalid regex is no longer registered, so its atoms stay out of the Aho-Corasick automaton.Out of scope
extractAtomsreads\Cas a literalC(atomabcdCfghforabcd\Cfgh), so such a regex compiles but can never match. This is unchanged from main; the test only asserts that it compiles.