Skip to content

Commit

Permalink
bug: fix complete literal optimization issue
Browse files Browse the repository at this point in the history
This bug results in some regexes reporting matches at every position
even when it should't. The bug happens because the internal literal
optimizer winds up using an "empty" searcher that reports a match at
every position. This is technically correct whenever the literal
searcher is used as a prefilter (although slower than necessary), but an
optimization added later enabled the searcher to run on its own and not
as a prefilter. i.e., Without the confirm step by the regex engine. In
that context, the "empty" searcher is totally incorrect.

So this bug corresponds to a code path where the "empty" literal
searcher is used, but is also in a case where the literal searcher is
used directly to find matches and not as a prefilter. I believe at
least the following are required to trigger this path:

* The literals extracted need to be "complete." That is, the language
described by the regex is small and finite.
* There needs to be at least 26 distinct starting bytes among all of
the elements in the language described by the regex.
* There needs to be fewer than 26 distinct ending bytes among all of
the elements in the language described by the regex.
* Possibly other criteria...

The actual fix is to change the code that selects the literal searcher.
Indeed, there was even a comment in the erroneous code saying that the
path was impossible, but of course, it isn't. We change that path to
return None, as it should have long ago. This in turn results in the
case outlined above not using a literal searcher and just the regex
engine.

Fixes #999
  • Loading branch information
BurntSushi committed May 25, 2023
1 parent d555d61 commit 7559e2d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/exec.rs
Expand Up @@ -1439,7 +1439,18 @@ impl ExecReadOnly {
// This case shouldn't happen. When the regex isn't
// anchored, then complete prefixes should imply complete
// suffixes.
Some(MatchType::Literal(MatchLiteralType::Unanchored))
//
// The above is wrong! This case can happen. While
// complete prefixes should imply complete suffixes
// here, that doesn't necessarily mean we have a useful
// prefix matcher! It could be the case that the literal
// searcher decided the prefixes---even though they are
// "complete"---weren't good enough and thus created an
// empty matcher. If that happens and we return Unanchored
// here, then we'll end up using that matcher, which is
// very bad because it matches at every position. So...
// return None.
None
};
}
None
Expand Down
13 changes: 13 additions & 0 deletions tests/regression.rs
Expand Up @@ -248,3 +248,16 @@ fn regression_big_regex_overflow() {
let re = regex_new!(pat);
assert!(re.is_err());
}

#[test]
fn regression_complete_literals_suffix_incorrect() {
let needles = vec![
"aA", "bA", "cA", "dA", "eA", "fA", "gA", "hA", "iA", "jA", "kA",
"lA", "mA", "nA", "oA", "pA", "qA", "rA", "sA", "tA", "uA", "vA",
"wA", "xA", "yA", "zA",
];
let pattern = needles.join("|");
let re = regex!(&pattern);
let hay = "FUBAR";
assert_eq!(0, re.find_iter(hay).count());
}

0 comments on commit 7559e2d

Please sign in to comment.