Skip to content

Commit 8fdd26a

Browse files
committed
fix: WITHSCORES+RETURN0 parsing, OR operand escaping, slop validation
- Fix stride mismatch in executor when WITHSCORES + RETURN 0 (NOCONTENT) produces [count, id, score, ...] without field arrays - Add _ScoreParseMixin with _has_return_0() helper for both sync/async executors - Escape individual OR operands in fulltext() to prevent accidental negation (anti-virus) and field injection (@field) - Validate slop >= 0 in parser, raise ValueError for negative values - Add tests for all three fixes
1 parent 5010b75 commit 8fdd26a

5 files changed

Lines changed: 71 additions & 8 deletions

File tree

sql_redis/executor.py

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,20 @@ class QueryResult:
103103
count: int
104104

105105

106-
class Executor:
106+
class _ScoreParseMixin:
107+
"""Shared helper for detecting RETURN 0 (NOCONTENT) in translated args."""
108+
109+
@staticmethod
110+
def _has_return_0(args: list[str]) -> bool:
111+
"""Return True when the args contain 'RETURN 0' (no document fields)."""
112+
try:
113+
idx = args.index("RETURN")
114+
return args[idx + 1] == "0"
115+
except (ValueError, IndexError):
116+
return False
117+
118+
119+
class Executor(_ScoreParseMixin):
107120
"""Executes SQL queries against Redis."""
108121

109122
def __init__(self, client: redis.Redis, schema_registry: SchemaRegistry) -> None:
@@ -169,8 +182,19 @@ def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
169182
# Check if WITHSCORES was requested — changes response format
170183
with_scores = "WITHSCORES" in translated.args
171184
score_alias = translated.score_alias
172-
173-
if with_scores:
185+
# RETURN 0 suppresses document fields (like NOCONTENT);
186+
# with WITHSCORES the reply is [count, id, score, id, score, ...]
187+
no_content = self._has_return_0(translated.args)
188+
189+
if with_scores and no_content:
190+
# WITHSCORES + RETURN 0: [count, id1, score1, id2, score2, ...]
191+
# Stride of 2: key, score (no field array)
192+
for i in range(1, len(raw_result) - 1, 2):
193+
score = raw_result[i + 1]
194+
alias = score_alias or "__score"
195+
row = {alias: score}
196+
rows.append(row)
197+
elif with_scores:
174198
# WITHSCORES format: [count, key1, score1, [fields1], key2, score2, [fields2], ...]
175199
# Stride of 3: key, score, field_list
176200
for i in range(1, len(raw_result) - 2, 3):
@@ -198,7 +222,7 @@ def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
198222
return QueryResult(rows=rows, count=count)
199223

200224

201-
class AsyncExecutor:
225+
class AsyncExecutor(_ScoreParseMixin):
202226
"""Async version of Executor for use with redis.asyncio clients."""
203227

204228
def __init__(
@@ -271,8 +295,16 @@ async def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
271295
if translated.command == "FT.SEARCH":
272296
with_scores = "WITHSCORES" in translated.args
273297
score_alias = translated.score_alias
298+
no_content = self._has_return_0(translated.args)
274299

275-
if with_scores:
300+
if with_scores and no_content:
301+
# WITHSCORES + RETURN 0: [count, id1, score1, id2, score2, ...]
302+
for i in range(1, len(raw_result) - 1, 2):
303+
score = raw_result[i + 1]
304+
alias = score_alias or "__score"
305+
row = {alias: score}
306+
rows.append(row)
307+
elif with_scores:
276308
# WITHSCORES format: [count, key1, score1, [fields1], ...]
277309
for i in range(1, len(raw_result) - 2, 3):
278310
score = raw_result[i + 1]

sql_redis/parser.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,12 +975,16 @@ def _add_function_condition(
975975
field_name = args[0].name if isinstance(args[0], exp.Column) else None
976976
value = self._extract_literal_value(args[1])
977977

978-
# Optional 3rd arg: slop (int)
978+
# Optional 3rd arg: slop (non-negative int)
979979
slop = None
980980
if len(args) >= 3:
981981
slop_val = self._extract_literal_value(args[2])
982982
if slop_val is not None:
983983
slop = int(slop_val)
984+
if slop < 0:
985+
raise ValueError(
986+
f"FULLTEXT slop argument must be a non-negative integer (got {slop})"
987+
)
984988

985989
# Optional 4th arg: inorder (boolean-like: 1/0 or true/false)
986990
inorder = False

sql_redis/query_builder.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,10 @@ def build_text_condition(
173173
terms = " ".join(filtered_words) if filtered_words else value
174174
search_value = f"({terms})"
175175
elif " OR " in value:
176-
# OR union within text field: split on ' OR ' and join with |
177-
or_terms = [t.strip() for t in value.split(" OR ")]
176+
# OR union within text field: split on ' OR ', escape each term, join with |
177+
or_terms = [
178+
self._escape_fulltext_term(t.strip()) for t in value.split(" OR ")
179+
]
178180
search_value = f"({'|'.join(or_terms)})"
179181
else:
180182
# Single-word FULLTEXT — escape to prevent accidental operator injection

tests/test_query_builder.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,3 +567,18 @@ def test_multi_field_non_exact_escapes(self):
567567
["title", "description"], "FULLTEXT", "hello|world"
568568
)
569569
assert result == "(@title|description:hello\\|world)"
570+
571+
def test_or_terms_escape_special_chars(self):
572+
"""OR operands are escaped before joining with |."""
573+
builder = QueryBuilder()
574+
result = builder.build_text_condition(
575+
"title", "FULLTEXT", "laptop OR anti-virus"
576+
)
577+
# '-' is a RediSearch operator and should be escaped in OR terms
578+
assert result == "@title:(laptop|anti\\-virus)"
579+
580+
def test_or_terms_escape_at_sign(self):
581+
"""OR operands escape @ to prevent field injection."""
582+
builder = QueryBuilder()
583+
result = builder.build_text_condition("title", "FULLTEXT", "hello OR @world")
584+
assert result == "@title:(hello|\\@world)"

tests/test_sql_parser.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,3 +855,13 @@ def test_exists_in_where_raises_error(self):
855855
parser = SQLParser()
856856
with pytest.raises(ValueError, match="exists.*aggregate"):
857857
parser.parse("SELECT * FROM idx WHERE exists(email)")
858+
859+
860+
class TestSQLParserFulltextValidation:
861+
"""Tests for fulltext() argument validation."""
862+
863+
def test_fulltext_negative_slop_raises(self):
864+
"""Negative slop in fulltext() raises ValueError."""
865+
parser = SQLParser()
866+
with pytest.raises(ValueError, match="non-negative integer"):
867+
parser.parse("SELECT * FROM idx WHERE fulltext(title, 'hello world', -1)")

0 commit comments

Comments
 (0)