Skip to content

Commit fcc4980

Browse files
committed
automata: fix bug in reverse suffix/inner optimization
A minimal reproducer of this bug is on a haystack of `zabb` with the regex `.bb|b`. The `regex` crate will report a match at `2..3`, but the correct match is `1..4`. While this seems like a simple regex, there are a pretty specific set of circumstances required to trigger the bug: 1. There are no prefix literals that activate a standard prefix literal scan. 2. There needs to be an extractable *suffix* or *inner* literal. 3. An actual match needs to be present in the haystack. 4. The regex and haystack Crucially, note that because of (3), this bug will never lead to `Regex::is_match` providing a false positive *or* a false negative. This bug is strictly about leftmost-first match semantics being incorrect in some cases and will report an incorrect match span. (4) could do with a bit more explanation, since it's rather subtle. Let's trace the minimal example through the regex crate's "reverse suffix" optimization. During compilation, there is no prefix literal that can be extracted. The `.` defeats that class of optimization. Moreover, there is a suffix literal in the regex. That is, all matches for `.bb|b` must end with `b`. The regex crate sees this and will scan for matches of `b`. It will then attempt to match the regex in reverse at each candidate match of `b`. Let's see what happens: * Find first occurrence of `b` at offset `2` in `zabb`. * Start reverse confirmation step at offset `2`. * The second alternation branch, `b` in `.bb|b`, matches at `2..3`. * The second alternation branch is reported as the overall match. This happens because the first alternation branch, `.bb`, does _not_ have a match ending at offset `3`. The fundamental problem here is that there is an overlap between the reverse automaton for confirming the match and the literal scan. Small changes, even to the haystack, can result in the bug disappearing. For example, with a haystack of `zbb`, the correct match of `0..3` is reported. This occurs because there is a quadratic "trip wire" that triggers in this case that causes the search to bail out and fall back to a DFA without using any literal optimizations. This bug also applies to the "reverse inner" optimization. This can happen when the literal is extracted from inside the regex as opposed to it being a suffix literal. For example, the regex `(?:..acbb|b)a(?:c|d)` on the haystack `xzbacbbac` reported a match at `2..5`, but the correct match is `1..9`. Note that #1355 technically fixes this problem and is much simpler, but in so doing, makes the reverse suffix and inner optimizations completely ineffective. Fixes #1354, Closes #1355
1 parent 2c7b172 commit fcc4980

3 files changed

Lines changed: 1017 additions & 57 deletions

File tree

regex-automata/src/meta/reverse_inner.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ use crate::{util::prefilter::Prefilter, MatchKind};
5050
///
5151
/// Note that this assumes leftmost-first match semantics, so callers must
5252
/// not call this otherwise.
53-
pub(crate) fn extract(hirs: &[&Hir]) -> Option<(Hir, Prefilter)> {
53+
pub(crate) fn extract(
54+
hirs: &[&Hir],
55+
) -> Option<(Hir, Prefilter, Vec<Vec<u8>>)> {
5456
if hirs.len() != 1 {
5557
debug!(
5658
"skipping reverse inner optimization since it only \
@@ -73,7 +75,7 @@ pub(crate) fn extract(hirs: &[&Hir]) -> Option<(Hir, Prefilter)> {
7375
// we probably wouldn't be here looking for an inner prefilter.
7476
for i in 1..concat.len() {
7577
let hir = &concat[i];
76-
let pre = match prefilter(hir) {
78+
let (pre, lits) = match prefilter(hir) {
7779
None => continue,
7880
Some(pre) => pre,
7981
};
@@ -96,17 +98,17 @@ pub(crate) fn extract(hirs: &[&Hir]) -> Option<(Hir, Prefilter)> {
9698
// something better and more discriminatory by looking at the entire
9799
// suffix. We don't do this above to avoid making this loop worst case
98100
// quadratic in the length of 'concat'.
99-
let pre2 = match prefilter(&concat_suffix) {
100-
None => pre,
101-
Some(pre2) => {
101+
let (pre2, lits2) = match prefilter(&concat_suffix) {
102+
None => (pre, lits),
103+
Some((pre2, lits2)) => {
102104
if pre2.is_fast() {
103-
pre2
105+
(pre2, lits2)
104106
} else {
105-
pre
107+
(pre, lits)
106108
}
107109
}
108110
};
109-
return Some((concat_prefix, pre2));
111+
return Some((concat_prefix, pre2, lits2));
110112
}
111113
debug!(
112114
"skipping reverse inner optimization because a top-level \
@@ -124,7 +126,7 @@ pub(crate) fn extract(hirs: &[&Hir]) -> Option<(Hir, Prefilter)> {
124126
///
125127
/// Note that this assumes leftmost-first match semantics, so callers must
126128
/// not call this otherwise.
127-
fn prefilter(hir: &Hir) -> Option<Prefilter> {
129+
fn prefilter(hir: &Hir) -> Option<(Prefilter, Vec<Vec<u8>>)> {
128130
let mut extractor = literal::Extractor::new();
129131
extractor.kind(literal::ExtractKind::Prefix);
130132
let mut prefixes = extractor.extract(hir);
@@ -148,9 +150,10 @@ fn prefilter(hir: &Hir) -> Option<Prefilter> {
148150
prefixes.len(),
149151
prefixes
150152
);
151-
prefixes
152-
.literals()
153-
.and_then(|lits| Prefilter::new(MatchKind::LeftmostFirst, lits))
153+
let lits = prefixes.literals()?;
154+
let pre = Prefilter::new(MatchKind::LeftmostFirst, lits)?;
155+
let lits = lits.iter().map(|lit| lit.as_bytes().to_vec()).collect();
156+
Some((pre, lits))
154157
}
155158

156159
/// Looks for a "top level" HirKind::Concat item in the given HIR. This will

0 commit comments

Comments
 (0)