Browse Source

fix(on-demand): Add escaping of glob patterns for meta chars supported by Relay (#64552)

Riccardo Busetti 1 year ago
parent
commit
37db951d78

+ 24 - 1
src/sentry/snuba/metrics/extraction.py

@@ -1531,6 +1531,29 @@ def _get_satisfactory_threshold_and_metric(project: Project) -> tuple[int, str]:
     return threshold, metric_field
 
 
+def _escape_wildcard(value: str) -> str:
+    """
+    Escapes all characters in the wildcard which are considered as meta characters in the glob
+    implementation in Relay, which can be found at: https://docs.rs/globset/latest/globset/#syntax.
+
+    The goal of this function is to only preserve the `*` character as it is the only character that Sentry's
+    product offers to users to perform wildcard matching.
+    """
+    i, n = 0, len(value)
+    escaped = ""
+
+    while i < n:
+        c = value[i]
+        i = i + 1
+
+        if c in "[]{}?":
+            escaped += "\\" + c
+        else:
+            escaped += c
+
+    return escaped
+
+
 T = TypeVar("T")
 
 
@@ -1633,7 +1656,7 @@ class SearchQueryConverter:
             condition: RuleCondition = {
                 "op": "glob",
                 "name": _map_field_name(key),
-                "value": [value],
+                "value": [_escape_wildcard(value)],
             }
         else:
             # Special case for the `has` and `!has` operators which are parsed as follows:

+ 28 - 0
tests/sentry/snuba/metrics/test_extraction.py

@@ -16,6 +16,7 @@ from sentry.snuba.metrics.extraction import (
     to_standard_metrics_query,
 )
 from sentry.testutils.pytest.fixtures import django_db_all
+from sentry.utils.glob import glob_match
 
 
 @django_db_all
@@ -460,6 +461,33 @@ def test_spec_wildcard() -> None:
     }
 
 
+@pytest.mark.parametrize(
+    "query,title,expected_pattern",
+    [
+        ("title:*[dispatch:*", "backend test [dispatch:something]", "*\\[dispatch:*"),
+        ("title:*{dispatch:*", "test {dispatch:something]", "*\\{dispatch:*"),
+        ("title:*dispatch]:*", "backend dispatch]:", "*dispatch\\]:*"),
+        ("title:*dispatch}:*", "test [dispatch}:", "*dispatch\\}:*"),
+        ("title:*?dispatch*", "backend ?dispatch", "*\\?dispatch*"),
+    ],
+)
+def test_spec_wildcard_escaping(query, title, expected_pattern) -> None:
+    spec = OnDemandMetricSpec("count()", query)
+
+    assert spec._metric_type == "c"
+    assert spec.field_to_extract is None
+    assert spec.op == "sum"
+    assert spec.condition == {
+        "name": "event.transaction",
+        "op": "glob",
+        "value": [expected_pattern],
+    }
+
+    # We also validate using Relay's glob implementation to make sure the escaping
+    # is interpreted correctly.
+    assert glob_match(title, expected_pattern, ignorecase=True)
+
+
 def test_spec_count_if() -> None:
     spec = OnDemandMetricSpec("count_if(transaction.duration,equals,300)", "")