fix(pattern): Incorrect matches with adjacent any and wildcard matche…

…rs (#4072) Joining `?` with `*` is an invalid optimization, the `?` implicitly forces a minimum length match, where as `*` matches 0..N characters.
getsentry · Sep 24, 2024 · e09b7e1 · e09b7e1
1 parent 385cc89
commit e09b7e1
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 - Report invalid spans with appropriate outcome reason. ([#4051](https://github.com/getsentry/relay/pull/4051))
 - Use the duration reported by the profiler instead of the transaction. ([#4058](https://github.com/getsentry/relay/pull/4058))
+- Incorrect pattern matches involving adjacent any and wildcard matchers. ([#4072](https://github.com/getsentry/relay/pull/4072))
 
 **Features:**
 

diff --git a/relay-pattern/src/lib.rs b/relay-pattern/src/lib.rs
@@ -607,10 +607,6 @@ impl Tokens {
             // We can collapse multiple wildcards into a single one.
             // TODO: separator special handling (?)
             (Token::Wildcard, Token::Wildcard) => {}
-            // `*` followed by `?` is just `*`.
-            (Token::Wildcard, Token::Any(_)) => {}
-            // `?` followed by `*` is just `*`.
-            (last @ Token::Any(_), Token::Wildcard) => *last = Token::Wildcard,
             // Collapse multiple literals into one.
             (Token::Literal(ref mut last), Token::Literal(s)) => last.push_str(&s),
             // Everything else is just another token.
@@ -858,8 +854,12 @@ mod tests {
         assert_pattern!("foo*", "foo___");
         assert_pattern!("foo*", "foo");
         assert_pattern!("foo**", "foo");
-        assert_pattern!("foo*?*", "foo");
-        assert_pattern!("foo*?*?", "foo");
+        assert_pattern!("foo*?*", NOT "foo");
+        assert_pattern!("foo*?*", "foo_");
+        assert_pattern!("foo*?*?", "foo_____");
+        assert_pattern!("foo*?*?", NOT "foo");
+        assert_pattern!("foo*?*?", "foo__");
+        assert_pattern!("foo*?*?", "foo_____");
         assert_pattern!("foo**", "foo___");
         assert_pattern!("foo*?*", "foo___");
         assert_pattern!("foo*?*?", "foo___");
@@ -896,20 +896,22 @@ mod tests {
     #[test]
     fn test_prefix_strategy() {
         assert_strategy!("foo*", Prefix);
-        assert_strategy!("foo*?", Prefix);
-        assert_strategy!("foo?*", Prefix);
-        assert_strategy!("foo*?*", Prefix);
-        assert_strategy!("foo??***???****", Prefix);
-        assert_strategy!("foo***???****??", Prefix);
+        assert_strategy!("foo**", Prefix);
+        assert_strategy!("foo?*", Regex);
     }
 
     #[test]
     fn test_suffix() {
         assert_pattern!("*foo", "___foo");
         assert_pattern!("*foo", "foo");
         assert_pattern!("**foo", "foo");
-        assert_pattern!("*?*foo", "foo");
-        assert_pattern!("?*?*foo", "foo");
+        assert_pattern!("*?*foo", NOT "foo");
+        assert_pattern!("*?*foo", "_foo");
+        assert_pattern!("*?*foo", "_____foo");
+        assert_pattern!("?*?*foo", NOT "foo");
+        assert_pattern!("?*?*foo", NOT "_foo");
+        assert_pattern!("?*?*foo", "__foo");
+        assert_pattern!("?*?*foo", "_____foo");
         assert_pattern!("**foo", "___foo");
         assert_pattern!("*?*foo", "___foo");
         assert_pattern!("*?*?foo", "__foo");
@@ -946,11 +948,8 @@ mod tests {
     #[test]
     fn test_suffix_strategy() {
         assert_strategy!("*foo", Suffix);
-        assert_strategy!("*?foo", Suffix);
-        assert_strategy!("?*foo", Suffix);
-        assert_strategy!("*?*foo", Suffix);
-        assert_strategy!("??***???****foo", Suffix);
-        assert_strategy!("***???****??foo", Suffix);
+        assert_strategy!("**foo", Suffix);
+        assert_strategy!("*?foo", Regex);
     }
 
     #[test]
@@ -1006,11 +1005,11 @@ mod tests {
     #[test]
     fn test_contains_strategy() {
         assert_strategy!("*foo*", Contains);
-        assert_strategy!("*?foo?*", Contains);
-        assert_strategy!("?*foo*?", Contains);
-        assert_strategy!("*?*foo*?*", Contains);
-        assert_strategy!("??***???****foo??***???****", Contains);
-        assert_strategy!("***???****??foo***???****??", Contains);
+        assert_strategy!("**foo**", Contains);
+        assert_strategy!("*?foo*", Regex);
+        assert_strategy!("*foo?*", Regex);
+        assert_strategy!("*foo*?", Regex);
+        assert_strategy!("?*foo*", Regex);
     }
 
     #[test]
@@ -1037,8 +1036,8 @@ mod tests {
         assert_strategy!("{*}", Static);
         assert_strategy!("{*,}", Static);
         assert_strategy!("{foo,*}", Static);
-        assert_strategy!("{foo,*}?{*,bar}", Static);
-        assert_strategy!("{*,}?{*,}", Static);
+        assert_strategy!("{foo,*}?{*,bar}", Regex);
+        assert_strategy!("{*,}?{*,}", Regex);
     }
 
     #[test]
@@ -1069,6 +1068,33 @@ mod tests {
         assert_pattern!("?", "/");
     }
 
+    #[test]
+    fn test_any_wildcard() {
+        assert_pattern!("??*", NOT "");
+        assert_pattern!("??*", NOT "a");
+        assert_pattern!("??*", "ab");
+        assert_pattern!("??*", "abc");
+        assert_pattern!("??*", "abcde");
+
+        assert_pattern!("*??", NOT "");
+        assert_pattern!("*??", NOT "a");
+        assert_pattern!("*??", "ab");
+        assert_pattern!("*??", "abc");
+        assert_pattern!("*??", "abcde");
+
+        assert_pattern!("*??*", NOT "");
+        assert_pattern!("*??*", NOT "a");
+        assert_pattern!("*??*", "ab");
+        assert_pattern!("*??*", "abc");
+        assert_pattern!("*??*", "abcde");
+
+        assert_pattern!("*?*?*", NOT "");
+        assert_pattern!("*?*?*", NOT "a");
+        assert_pattern!("*?*?*", "ab");
+        assert_pattern!("*?*?*", "abc");
+        assert_pattern!("*?*?*", "abcde");
+    }
+
     #[test]
     fn test_escapes() {
         assert_pattern!(r"f\\o", r"f\o");