Fix: Escape ? and | characters in TokenEscaper by thakoreh · Pull Request #502 · redis/redis-vl-python

thakoreh · 2026-02-25T20:55:20Z

Summary

Added ? and | to the escape patterns in TokenEscaper to comply with RediSearch query syntax requirements
Enabled previously commented-out test cases for pipe and question mark
Updated test expectations for the symbols test case

Problem

The TokenEscaper class was not escaping ? and | characters, which are special characters in RediSearch queries that need to be escaped according to Redis documentation.

Solution

Updated the regex patterns:

DEFAULT_ESCAPED_CHARS: Added ? and | to the character class
ESCAPED_CHARS_NO_WILDCARD: Added ? and | to the character class

Testing

Verified locally that:

question?mark → question\?mark
pipe|tag → pipe\|tag
& symbols, like * and ? → \&\ symbols\,\ like\ \*\ and\ \?

Fixes #490

Note

Low Risk
Low risk: small regex and test-only changes affecting query escaping for two additional characters; potential behavior change is limited to inputs containing ?/| and wildcard-preservation mode.

Overview
Fixes RediSearch token escaping by adding ? and | to TokenEscaper’s escape patterns, and updating the docs link reference.

Updates unit tests to assert ?/| are escaped in normal mode, and adds coverage around preserve_wildcards=True to ensure */? can be preserved while still escaping other special characters (e.g., |).

^{Written by Cursor Bugbot for commit 699e332. This will update automatically on new commits. Configure here.}

Added ? and | to the escape patterns in TokenEscaper to comply with RediSearch query syntax requirements. Changes: - Updated DEFAULT_ESCAPED_CHARS regex to include ? and | - Updated ESCAPED_CHARS_NO_WILDCARD regex to include ? and | - Enabled previously commented-out test cases for pipe and question mark - Updated test expectations for symbols test case Fixes redis#490

jit-ci · 2026-02-25T20:55:32Z

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

Copilot

Pull request overview

This PR updates TokenEscaper to escape ? and | characters so generated RediSearch queries comply with RediSearch escaping/tokenization rules, and it re-enables/updates unit tests that validate the new escaping behavior.

Changes:

Added ? and | to the TokenEscaper escape character patterns (both default and “no wildcard” variants).
Enabled/added test cases asserting correct escaping for pipe|tag and question?mark.
Updated an existing “symbols” test expectation to require escaping ?.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`redisvl/utils/token_escaper.py`	Extends the escape regex character classes to include `?` and `
`tests/unit/test_token_escaper.py`	Updates expectations and enables new cases verifying `?` and `

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

redisvl/utils/token_escaper.py

- Remove ? from ESCAPED_CHARS_NO_WILDCARD to match * behavior - ? is a single-character wildcard in RediSearch queries - Add tests for preserve_wildcards behavior with * and ?

thakoreh · 2026-02-28T13:20:28Z

Addressed the Bugbot feedback in 699e332:

Fixed: ? now preserved in wildcard mode

? is a single-character wildcard in RediSearch (like * for prefix/suffix matching). Updated ESCAPED_CHARS_NO_WILDCARD to exclude ? alongside * so both wildcards are preserved when preserve_wildcards=True.

Changes:

Removed ? from ESCAPED_CHARS_NO_WILDCARD pattern
Added tests for preserve_wildcards behavior with * and ?

@vishal-bala I'll address the URL update in a separate commit if you'd like me to handle that as well.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-28T13:22:46Z

redisvl/utils/token_escaper.py

+    # Same as above but excludes * and ? to allow wildcard patterns
+    ESCAPED_CHARS_NO_WILDCARD = r"[,.<>{}\[\]\\\"\':;!@#$%^&()\-+=~\/ |]"


PR description says ESCAPED_CHARS_NO_WILDCARD added both ? and |, but the code intentionally excludes ? so it can be preserved when preserve_wildcards=True (only | is added). Please update the PR description (or the code) so they match, since this affects the documented behavior of wildcard queries.

Copilot · 2026-02-28T13:22:47Z

tests/unit/test_token_escaper.py

+        ("mixed*and|pipe", "mixed*and\|pipe"),
+        ("question?and|pipe", "question\?and\|pipe"),  # ? escaped when not preserving
+    ],
+    ids=["star", "question", "both", "star-only", "question-escaped"],
+)
+def test_escape_preserve_wildcards(escaper, test_input, expected):
+    """Test that * and ? are preserved when preserve_wildcards=True."""
+    # These tests verify wildcard preservation behavior
+    if "*" in test_input and "?" in test_input:
+        result = escaper.escape(test_input, preserve_wildcards=True)
+        assert "*" in result and "?" in result
+    elif "*" in test_input:
+        assert "*" in escaper.escape(test_input, preserve_wildcards=True)
+    elif "?" in test_input:
+        assert "?" in escaper.escape(test_input, preserve_wildcards=True)


test_escape_preserve_wildcards doesn’t actually validate the escaped output: the expected parameter is unused, and the current "*" in result / "?" in result checks will still pass even if */? are escaped (e.g., \* still contains *). This leaves the new preserve_wildcards behavior effectively untested (including whether | is escaped). Use the parametrized expected value and assert the full escaped string for each case; also fix the question?and|pipe expectation to keep ? unescaped when preserve_wildcards=True while still escaping |.

Suggested change

("mixed*and|pipe", "mixed*and\|pipe"),

("question?and|pipe", "question\?and\|pipe"), # ? escaped when not preserving

],

ids=["star", "question", "both", "star-only", "question-escaped"],

)

def test_escape_preserve_wildcards(escaper, test_input, expected):

"""Test that * and ? are preserved when preserve_wildcards=True."""

# These tests verify wildcard preservation behavior

if "*" in test_input and "?" in test_input:

result = escaper.escape(test_input, preserve_wildcards=True)

assert "*" in result and "?" in result

elif "*" in test_input:

assert "*" in escaper.escape(test_input, preserve_wildcards=True)

elif "?" in test_input:

assert "?" in escaper.escape(test_input, preserve_wildcards=True)

(r"mixed*and|pipe", r"mixed*and\|pipe"),

(r"question?and|pipe", r"question?and\|pipe"), # ? preserved, | escaped

],

ids=["star", "question", "both", "star-only", "question-preserved"],

)

def test_escape_preserve_wildcards(escaper, test_input, expected):

"""Test that * and ? are preserved when preserve_wildcards=True."""

result = escaper.escape(test_input, preserve_wildcards=True)

assert result == expected

Copilot · 2026-02-28T13:22:47Z

redisvl/utils/token_escaper.py

+    # Same as above but excludes * and ? to allow wildcard patterns
+    ESCAPED_CHARS_NO_WILDCARD = r"[,.<>{}\[\]\\\"\':;!@#$%^&()\-+=~\/ |]"


The comment on ESCAPED_CHARS_NO_WILDCARD says it excludes * and ? (so preserve_wildcards=True preserves both), but the escape() docstring still says it only preserves *. Please update the preserve_wildcards parameter documentation to reflect the actual behavior (preserving both * and ?), so callers don’t assume ? will be escaped/preserved incorrectly.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

cursor · 2026-02-28T13:23:09Z

tests/unit/test_token_escaper.py

+    elif "*" in test_input:
+        assert "*" in escaper.escape(test_input, preserve_wildcards=True)
+    elif "?" in test_input:
+        assert "?" in escaper.escape(test_input, preserve_wildcards=True)


Test ignores expected parameter, never verifying actual output

Medium Severity

The test_escape_preserve_wildcards function receives an expected parameter via @pytest.mark.parametrize but never references it. Instead, the test body only performs loose containment checks ("*" in result), which would pass even if the escaping logic were completely wrong. Additionally, the test case "question?and|pipe" has an expected value of "question\?and\|pipe" (with ? escaped), but the test calls escape with preserve_wildcards=True, which preserves ? unescaped — so the expected value is itself incorrect for what the test claims to verify.

Additional Locations (1)

tests/unit/test_token_escaper.py#L133-L134

Copilot AI review requested due to automatic review settings February 25, 2026 20:55

Copilot started reviewing on behalf of thakoreh February 25, 2026 20:55 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

vishal-bala reviewed Feb 26, 2026

View reviewed changes

redisvl/utils/token_escaper.py Outdated Show resolved Hide resolved

vishal-bala added the auto:patch Increment the patch version when merged label Feb 26, 2026

docs: update Redis docs URL to current documentation

70ceefb

cursor bot reviewed Feb 26, 2026

View reviewed changes

redisvl/utils/token_escaper.py Outdated Show resolved Hide resolved

fix: preserve ? wildcard alongside * in preserve_wildcards mode

699e332

- Remove ? from ESCAPED_CHARS_NO_WILDCARD to match * behavior - ? is a single-character wildcard in RediSearch queries - Add tests for preserve_wildcards behavior with * and ?

Copilot AI review requested due to automatic review settings February 28, 2026 13:20

Copilot started reviewing on behalf of thakoreh February 28, 2026 13:20 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

cursor bot reviewed Feb 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Escape ? and | characters in TokenEscaper#502

Fix: Escape ? and | characters in TokenEscaper#502
thakoreh wants to merge 3 commits intoredis:mainfrom
thakoreh:fix-token-escaper-pipe-question

thakoreh commented Feb 25, 2026 •

edited by cursor bot

Loading

Uh oh!

jit-ci bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

thakoreh commented Feb 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Feb 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# Same as above but excludes * and ? to allow wildcard patterns
		ESCAPED_CHARS_NO_WILDCARD = r"[,.<>{}\[\]\\\"\':;!@#$%^&()\-+=~\/ \|]"

Conversation

thakoreh commented Feb 25, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Testing

Uh oh!

jit-ci bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

thakoreh commented Feb 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 28, 2026

Choose a reason for hiding this comment

Test ignores expected parameter, never verifying actual output

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thakoreh commented Feb 25, 2026 •

edited by cursor bot

Loading

Test ignores `expected` parameter, never verifying actual output