Skip to content

Fix: Escape ? and | characters in TokenEscaper#502

Open
thakoreh wants to merge 2 commits intoredis:mainfrom
thakoreh:fix-token-escaper-pipe-question
Open

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

Conversation

@thakoreh
Copy link

@thakoreh thakoreh commented Feb 25, 2026

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?markquestion\?mark
  • pipe|tagpipe\|tag
  • & symbols, like * and ?\&\ symbols\,\ like\ \*\ and\ \?

Fixes #490


Note

Low Risk
Low risk: small regex change to query escaping behavior, with unit tests updated/expanded to cover the new cases.

Overview
Fixes Redis query escaping by updating TokenEscaper’s regex character classes to also escape ? and | (including the no-wildcard variant) and refreshes the Redis docs link.

Updates unit tests to assert escaping for ? and | (enabling previously skipped cases) and adjusts the expected output for a symbols example to include escaped ?.

Written by Cursor Bugbot for commit 70ceefb. 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
Copilot AI review requested due to automatic review settings February 25, 2026 20:55
@jit-ci
Copy link

jit-ci bot commented Feb 25, 2026

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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question mark wildcard not preserved in no-wildcard pattern

High Severity

In RediSearch, ? is a wildcard character for single-character matching, just like * is for prefix/suffix matching. ESCAPED_CHARS_NO_WILDCARD intentionally excludes * to preserve wildcards when preserve_wildcards=True, but this change adds ? to that pattern. This means ? gets escaped even when wildcards are meant to be preserved, breaking single-character wildcard queries (e.g., via the Tag % "patter?" LIKE operator).

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand TokenEscaper to escape ? and | characters

3 participants