Skip to content

Commit 2e27246

Browse files
committed
fix: multi-word FULLTEXT escaping, stable score alias, slop float validation
- Escape individual terms in multi-word FULLTEXT branch while preserving ~ optional-term prefix (prevents @field injection and accidental negation) - Move score alias collision check out of per-row loop into _resolve_score_alias() so all rows use the same column name - Reject float slop values (e.g. 2.9) instead of silently truncating - Add tests for all three fixes (350 total)
1 parent 8fdd26a commit 2e27246

6 files changed

Lines changed: 143 additions & 51 deletions

File tree

sql_redis/executor.py

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class QueryResult:
104104

105105

106106
class _ScoreParseMixin:
107-
"""Shared helper for detecting RETURN 0 (NOCONTENT) in translated args."""
107+
"""Shared helpers for score-related response parsing."""
108108

109109
@staticmethod
110110
def _has_return_0(args: list[str]) -> bool:
@@ -115,6 +115,23 @@ def _has_return_0(args: list[str]) -> bool:
115115
except (ValueError, IndexError):
116116
return False
117117

118+
@staticmethod
119+
def _resolve_score_alias(score_alias: str | None, args: list[str]) -> str:
120+
"""Determine a stable score column name that won't collide with
121+
document fields. The alias is decided once before iterating rows
122+
so every row uses the same column name."""
123+
alias = score_alias or "__score"
124+
# Extract RETURN field names from args to detect collision
125+
try:
126+
idx = args.index("RETURN")
127+
count = int(args[idx + 1])
128+
return_fields = set(args[idx + 2 : idx + 2 + count])
129+
except (ValueError, IndexError):
130+
return_fields = set()
131+
if alias in return_fields:
132+
alias = f"__score_{alias}"
133+
return alias
134+
118135

119136
class Executor(_ScoreParseMixin):
120137
"""Executes SQL queries against Redis."""
@@ -181,17 +198,21 @@ def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
181198
if translated.command == "FT.SEARCH":
182199
# Check if WITHSCORES was requested — changes response format
183200
with_scores = "WITHSCORES" in translated.args
184-
score_alias = translated.score_alias
185201
# RETURN 0 suppresses document fields (like NOCONTENT);
186202
# with WITHSCORES the reply is [count, id, score, id, score, ...]
187203
no_content = self._has_return_0(translated.args)
204+
# Determine score column name once so every row is consistent
205+
alias = (
206+
self._resolve_score_alias(translated.score_alias, translated.args)
207+
if with_scores
208+
else ""
209+
)
188210

189211
if with_scores and no_content:
190212
# WITHSCORES + RETURN 0: [count, id1, score1, id2, score2, ...]
191213
# Stride of 2: key, score (no field array)
192214
for i in range(1, len(raw_result) - 1, 2):
193215
score = raw_result[i + 1]
194-
alias = score_alias or "__score"
195216
row = {alias: score}
196217
rows.append(row)
197218
elif with_scores:
@@ -201,10 +222,6 @@ def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
201222
score = raw_result[i + 1]
202223
row_data = raw_result[i + 2]
203224
row = dict(zip(row_data[::2], row_data[1::2]))
204-
alias = score_alias or "__score"
205-
# Guard: if alias collides with a document field, prefix it
206-
if alias in row:
207-
alias = f"__score_{alias}"
208225
row[alias] = score
209226
rows.append(row)
210227
else:
@@ -294,14 +311,17 @@ async def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
294311

295312
if translated.command == "FT.SEARCH":
296313
with_scores = "WITHSCORES" in translated.args
297-
score_alias = translated.score_alias
298314
no_content = self._has_return_0(translated.args)
315+
alias = (
316+
self._resolve_score_alias(translated.score_alias, translated.args)
317+
if with_scores
318+
else ""
319+
)
299320

300321
if with_scores and no_content:
301322
# WITHSCORES + RETURN 0: [count, id1, score1, id2, score2, ...]
302323
for i in range(1, len(raw_result) - 1, 2):
303324
score = raw_result[i + 1]
304-
alias = score_alias or "__score"
305325
row = {alias: score}
306326
rows.append(row)
307327
elif with_scores:
@@ -310,9 +330,6 @@ async def execute(self, sql: str, *, params: dict | None = None) -> QueryResult:
310330
score = raw_result[i + 1]
311331
row_data = raw_result[i + 2]
312332
row = dict(zip(row_data[::2], row_data[1::2]))
313-
alias = score_alias or "__score"
314-
if alias in row:
315-
alias = f"__score_{alias}"
316333
row[alias] = score
317334
rows.append(row)
318335
else:

sql_redis/parser.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,12 @@ def _add_function_condition(
980980
if len(args) >= 3:
981981
slop_val = self._extract_literal_value(args[2])
982982
if slop_val is not None:
983+
# Reject non-integer values (e.g. 2.9) instead of
984+
# silently truncating via int().
985+
if isinstance(slop_val, float) and slop_val != int(slop_val):
986+
raise ValueError(
987+
f"FULLTEXT slop argument must be an integer (got {slop_val})"
988+
)
983989
slop = int(slop_val)
984990
if slop < 0:
985991
raise ValueError(

sql_redis/query_builder.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,9 @@ def build_text_condition(
150150
search_value = f'"{escaped}"'
151151
elif " " in value and " OR " not in value:
152152
# FULLTEXT/MATCH with multi-word: tokenized search with stopword filtering.
153-
# FULLTEXT is intentionally pass-through — users craft RediSearch queries
154-
# via fulltext(), so operator chars like ~, |, - are preserved.
153+
# Each term is escaped to prevent accidental operator injection, but a
154+
# leading ~ (optional-term modifier) is preserved as an intentional
155+
# RediSearch operator.
155156
words = value.split()
156157
removed_stopwords = [
157158
w for w in words if w.lower() in REDIS_DEFAULT_STOPWORDS
@@ -170,7 +171,15 @@ def build_text_condition(
170171
stacklevel=2,
171172
)
172173

173-
terms = " ".join(filtered_words) if filtered_words else value
174+
escaped_words = []
175+
for w in (filtered_words if filtered_words else words):
176+
if w.startswith("~"):
177+
# Preserve ~ optional-term prefix, escape the rest
178+
escaped_words.append("~" + self._escape_fulltext_term(w[1:]))
179+
else:
180+
escaped_words.append(self._escape_fulltext_term(w))
181+
182+
terms = " ".join(escaped_words)
174183
search_value = f"({terms})"
175184
elif " OR " in value:
176185
# OR union within text field: split on ' OR ', escape each term, join with |

tests/test_query_builder.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,3 +582,21 @@ def test_or_terms_escape_at_sign(self):
582582
builder = QueryBuilder()
583583
result = builder.build_text_condition("title", "FULLTEXT", "hello OR @world")
584584
assert result == "@title:(hello|\\@world)"
585+
586+
def test_multiword_fulltext_escapes_special_chars(self):
587+
"""Multi-word FULLTEXT escapes dangerous chars like @ and |."""
588+
builder = QueryBuilder()
589+
result = builder.build_text_condition("title", "FULLTEXT", "hello @world")
590+
assert result == "@title:(hello \\@world)"
591+
592+
def test_multiword_fulltext_preserves_optional_prefix(self):
593+
"""Multi-word FULLTEXT preserves ~ optional-term prefix."""
594+
builder = QueryBuilder()
595+
result = builder.build_text_condition("title", "FULLTEXT", "laptop ~gaming")
596+
assert result == "@title:(laptop ~gaming)"
597+
598+
def test_multiword_fulltext_escapes_dash(self):
599+
"""Multi-word FULLTEXT escapes - to prevent accidental negation."""
600+
builder = QueryBuilder()
601+
result = builder.build_text_condition("title", "FULLTEXT", "hello anti-virus")
602+
assert result == "@title:(hello anti\\-virus)"

tests/test_sql_parser.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,3 +865,9 @@ def test_fulltext_negative_slop_raises(self):
865865
parser = SQLParser()
866866
with pytest.raises(ValueError, match="non-negative integer"):
867867
parser.parse("SELECT * FROM idx WHERE fulltext(title, 'hello world', -1)")
868+
869+
def test_fulltext_float_slop_raises(self):
870+
"""Float slop in fulltext() raises ValueError instead of silently truncating."""
871+
parser = SQLParser()
872+
with pytest.raises(ValueError, match="must be an integer"):
873+
parser.parse("SELECT * FROM idx WHERE fulltext(title, 'hello world', 2.9)")

0 commit comments

Comments
 (0)