diff --git a/redisvl/mcp/filters.py b/redisvl/mcp/filters.py index 862e6d4b..b5a587bc 100644 --- a/redisvl/mcp/filters.py +++ b/redisvl/mcp/filters.py @@ -23,8 +23,8 @@ # Dropping them costs nothing in containment, because containment comes from the # delimiters rather than from these. With `(` and `)` escaped, the value cannot # close its own `@field:(...)`, so anything it carries -- including a `|`, which -# no escaper in RedisVL touches -- stays scoped to this one field instead of -# reaching the surrounding expression. +# the like path deliberately leaves live so a pattern can express a union -- +# stays scoped to this one field instead of reaching the surrounding expression. _LIKE_ESCAPED_CHARS = re.compile(r"[,.<>{}\[\]\\\"\':;!@#$^&()\-+=~\/]") _LIKE_ESCAPER = TokenEscaper(escape_chars_re=_LIKE_ESCAPED_CHARS) diff --git a/redisvl/utils/token_escaper.py b/redisvl/utils/token_escaper.py index 494493f3..26e93df8 100644 --- a/redisvl/utils/token_escaper.py +++ b/redisvl/utils/token_escaper.py @@ -10,9 +10,12 @@ class TokenEscaper: # Characters that Redis Search requires us to escape during queries. # Source: https://redis.io/docs/latest/develop/ai/search-and-query/advanced-concepts/escaping/ - DEFAULT_ESCAPED_CHARS = r"[,.<>{}\[\]\\\"\':;!@#$%^&*()\-+=~\/ \?]" + DEFAULT_ESCAPED_CHARS = r"[,.<>{}\[\]\\\"\':;!@#$%^&*()\-+=~|\/ \?]" - # Same as above but excludes * and ? to allow wildcard patterns + # Same as above but excludes * and ? to allow wildcard patterns, and `|` + # because the `%` (LIKE) operator documents it as a union between wildcard + # patterns -- see `Tag.__mod__`. On the default path `|` is escaped, since + # a value carrying one would otherwise widen its clause into a union. ESCAPED_CHARS_NO_WILDCARD = r"[,.<>{}\[\]\\\"\':;!@#$%^&()\-+=~\/ ]" def __init__(self, escape_chars_re: Pattern | None = None): diff --git a/tests/unit/test_filter.py b/tests/unit/test_filter.py index db1dd08d..7ebbefb4 100644 --- a/tests/unit/test_filter.py +++ b/tests/unit/test_filter.py @@ -107,6 +107,19 @@ def test_tag_wildcard_preserves_asterisk(): assert str(tf_like) == "@tag_field:{tech*}" +def test_tag_equality_escapes_pipe_but_list_still_unions(): + """A `|` inside one tag value is literal; a list of values is still a union.""" + # Unescaped, this value would widen its clause into a union across tenants. + assert str(Tag("tenant_id") == "acme|victim") == "@tenant_id:{acme\\|victim}" + assert str(Tag("tenant_id") != "acme|victim") == "(-@tenant_id:{acme\\|victim})" + + # Values are escaped before being joined, so the list form is unaffected. + assert str(Tag("tenant_id") == ["acme", "victim"]) == "@tenant_id:{acme|victim}" + + # The % operator documents `|` as a union between wildcard patterns. + assert str(Tag("category") % "elec*|*soft") == "@category:{elec*|*soft}" + + def test_tag_wildcard_combined_with_exact_match(): """Test combining wildcard and exact match Tag filters in the same query.""" # Create filters with different operators diff --git a/tests/unit/test_token_escaper.py b/tests/unit/test_token_escaper.py index c1d6fd89..3deb09c7 100644 --- a/tests/unit/test_token_escaper.py +++ b/tests/unit/test_token_escaper.py @@ -121,3 +121,24 @@ def test_escape_long_string(escaper): # Use pytest's benchmark fixture to check performance escaped = escaper.escape(long_str) assert escaped == expected + + +@pytest.mark.parametrize( + ("test_input,expected"), + [ + ("acme|victim", r"acme\|victim"), + ("a|b|c", r"a\|b\|c"), + ("|leading", r"\|leading"), + ], + ids=["pair", "chain", "leading"], +) +def test_escape_pipe_on_the_default_path(escaper, test_input, expected): + # A `|` is RediSearch's union operator and binds looser than the implicit + # intersection, so one reaching the query string raw widens its clause. + assert escaper.escape(test_input) == expected + + +def test_pipe_stays_live_when_wildcards_are_preserved(escaper): + # The `%` operator documents `|` as a union between wildcard patterns, so + # the no-wildcard class must not escape it. + assert escaper.escape("elec*|*soft", preserve_wildcards=True) == "elec*|*soft"