automata: fix onepass::DFA::try_search_slots panic when too many slots are given

The API specifically documents that the caller may provide *any* number
of slots. But the onepass implementation was panicking when too many
were provided. (I had only considered the case when too few were
provided when I wrote one-pass implementation I think.)

This already works for the PikeVM and the bounded backtracker. This
commit adds tests covering those as well.

Note that the bug report specifically cites a zero-repetition capture
group as the instigator here. While that may have provided the
circumstances for the bug to appear, the actual bug is more general than
that. You don't need any capture groups at all to provoke it. You just
need to provide more slots than there are in the compiled regex.

Fixes #1327, Closes #1329

Author: keith-hall

regex-automata/src/dfa/onepass.rs

@@ -2093,9 +2093,20 @@ impl DFA {
         // be bad. In theory, we could avoid all this slot clearing if we knew
         // that every slot was always activated for every match. Then we would
         // know they would always be overwritten when a match is found.
+        //
+        // NOTE: We have to be careful here to avoid setting a length that
+        // exceeds the number of slots in our cache. Otherwise copying into
+        // the cache later will fail. This can happen when the number of
+        // caller provided slots is bigger than the number of slots in the
+        // compiled regex. (It's a bit of a weird case, but for simplicity and
+        // flexibility reasons, it is an API guarantee that the caller can
+        // provided any number of slots that they want.)
         let explicit_slots_len = core::cmp::min(
             Slots::LIMIT,
-            slots.len().saturating_sub(self.explicit_slot_start),
+            core::cmp::min(
+                slots.len().saturating_sub(self.explicit_slot_start),
+                cache.explicit_slots.len(),
+            ),
         );
         cache.setup_search(explicit_slots_len);
         for slot in cache.explicit_slots() {
@@ -2216,10 +2227,28 @@ impl DFA {
         // the path to the match state.
         if self.explicit_slot_start < slots.len() {
             // NOTE: The 'cache.explicit_slots()' slice is setup at the
-            // beginning of every search such that it is guaranteed to return a
-            // slice of length equivalent to 'slots[explicit_slot_start..]'.
-            slots[self.explicit_slot_start..]
-                .copy_from_slice(cache.explicit_slots());
+            // beginning of every search such that it is guaranteed
+            // to return a slice that is at most equal in length to
+            // 'slots[explicit_slot_start..]'. It may be smaller in
+            // cases where the caller provided more slots than there
+            // are in the compiled regex. In which case, we limit the
+            // length of `slots` to what we actually have.
+            let cache_slots = cache.explicit_slots();
+            slots[self.explicit_slot_start..][..cache_slots.len()]
+                .copy_from_slice(cache_slots);
+
+            // There is an edge case when a regex has capture groups with zero
+            // repetition (e.g., (abc){0}), the cache may have fewer explicit
+            // slots than what the caller provided.
+            // So we copy only the slots that exist in the cache.
+            // let cache_slots = cache.explicit_slots();
+            // let available = core::cmp::min(
+            // cache_slots.len(),
+            // slots.len().saturating_sub(self.explicit_slot_start),
+            // );
+            // slots[self.explicit_slot_start
+            // ..self.explicit_slot_start + available]
+            // .copy_from_slice(&cache_slots[..available]);
             epsilons.slots().apply(at, &mut slots[self.explicit_slot_start..]);
         }
         *matched_pid = Some(pid);

regex-automata/tests/dfa/onepass/mod.rs

@@ -1,2 +1,3 @@
+mod regression;
 #[cfg(not(miri))]
 mod suite;

regex-automata/tests/dfa/onepass/regression.rs

@@ -0,0 +1,61 @@
+// Regression test for zero-repetition capture groups,
+// which caused a panic when the Vec passed into search_slots
+// contained space for the capture group which would never
+// have any results.
+//
+// See: https://github.com/rust-lang/regex/issues/1327
+#[test]
+fn zero_repetition_capture_group() {
+    use regex_automata::{
+        dfa::onepass::DFA, util::primitives::NonMaxUsize, Anchored, Input,
+    };
+
+    let expr = DFA::new(r"(abc)(ABC){0}").unwrap();
+    let s = "abcABC";
+    let input = Input::new(s).span(0..s.len()).anchored(Anchored::Yes);
+
+    // Test with 4 slots, so the whole match plus the first capture group.
+    let mut cache = expr.create_cache();
+    let mut slots: Vec<Option<NonMaxUsize>> = vec![None; 4];
+    let pid = expr.try_search_slots(&mut cache, &input, &mut slots).unwrap();
+    assert_eq!(pid, Some(regex_automata::PatternID::must(0)));
+    assert_eq!(slots[0], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[1], Some(NonMaxUsize::new(3).unwrap()));
+    assert_eq!(slots[2], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[3], Some(NonMaxUsize::new(3).unwrap()));
+
+    // Test with larger slot array, which would fit the
+    // zero-repetition capture group.
+    slots.resize(6, None);
+    let pid = expr.try_search_slots(&mut cache, &input, &mut slots).unwrap();
+    assert_eq!(pid, Some(regex_automata::PatternID::must(0)));
+    // First capture group should match
+    assert_eq!(slots[2], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[3], Some(NonMaxUsize::new(3).unwrap()));
+    // Second capture group with {0} should be None.
+    assert_eq!(slots[4], None);
+    assert_eq!(slots[5], None);
+}
+
+// Another regression test for the same case as
+// `zero_repetition_capture_group`, but uses a simpler pattern. That
+// is, a zero-repetition capture group is a red herring. The actual bug
+// is simpler: it happens whenever too many slots are provided by the
+// caller.
+#[test]
+fn too_many_slots_normal_pattern() {
+    use regex_automata::{
+        dfa::onepass::DFA, util::primitives::NonMaxUsize, Anchored, Input,
+    };
+
+    let expr = DFA::new(r"abc").unwrap();
+    let s = "abc";
+    let input = Input::new(s).span(0..s.len()).anchored(Anchored::Yes);
+
+    let mut cache = expr.create_cache();
+    let mut slots: Vec<Option<NonMaxUsize>> = vec![None; 4];
+    let pid = expr.try_search_slots(&mut cache, &input, &mut slots).unwrap();
+    assert_eq!(pid, Some(regex_automata::PatternID::must(0)));
+    assert_eq!(slots[0], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[1], Some(NonMaxUsize::new(3).unwrap()));
+}

regex-automata/tests/nfa/thompson/backtrack/mod.rs

@@ -1,2 +1,3 @@
+mod regression;
 #[cfg(not(miri))]
 mod suite;

regex-automata/tests/nfa/thompson/backtrack/regression.rs

@@ -0,0 +1,20 @@
+// Tests that we can call the backtracker with more slots
+// than is actually in the compiled regex.
+#[test]
+fn too_many_slots_normal_pattern() {
+    use regex_automata::{
+        nfa::thompson::backtrack::BoundedBacktracker,
+        util::primitives::NonMaxUsize, Anchored, Input,
+    };
+
+    let expr = BoundedBacktracker::new(r"abc").unwrap();
+    let s = "abc";
+    let input = Input::new(s).span(0..s.len()).anchored(Anchored::Yes);
+
+    let mut cache = expr.create_cache();
+    let mut slots: Vec<Option<NonMaxUsize>> = vec![None; 4];
+    let pid = expr.try_search_slots(&mut cache, &input, &mut slots).unwrap();
+    assert_eq!(pid, Some(regex_automata::PatternID::must(0)));
+    assert_eq!(slots[0], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[1], Some(NonMaxUsize::new(3).unwrap()));
+}

regex-automata/tests/nfa/thompson/pikevm/mod.rs

@@ -1,2 +1,3 @@
+mod regression;
 #[cfg(not(miri))]
 mod suite;

regex-automata/tests/nfa/thompson/pikevm/regression.rs

@@ -0,0 +1,20 @@
+// Tests that we can call the PikeVM with more slots
+// than is actually in the compiled regex.
+#[test]
+fn too_many_slots_normal_pattern() {
+    use regex_automata::{
+        nfa::thompson::pikevm::PikeVM, util::primitives::NonMaxUsize,
+        Anchored, Input,
+    };
+
+    let expr = PikeVM::new(r"abc").unwrap();
+    let s = "abc";
+    let input = Input::new(s).span(0..s.len()).anchored(Anchored::Yes);
+
+    let mut cache = expr.create_cache();
+    let mut slots: Vec<Option<NonMaxUsize>> = vec![None; 4];
+    let pid = expr.search_slots(&mut cache, &input, &mut slots);
+    assert_eq!(pid, Some(regex_automata::PatternID::must(0)));
+    assert_eq!(slots[0], Some(NonMaxUsize::new(0).unwrap()));
+    assert_eq!(slots[1], Some(NonMaxUsize::new(3).unwrap()));
+}