fix(postgrest-js): correctly format array values in not() with 'in' operator - #2631
fix(postgrest-js): correctly format array values in not() with 'in' operator#2631diptomistry wants to merge 2 commits into
Conversation
… values - Introduced a new function `cleanFilterValues` to deduplicate and format array values for the 'in' operator, ensuring proper quoting of reserved characters. - Updated the `not` method to utilize `cleanFilterValues` for array inputs, maintaining the original format for pre-formatted strings. - Added tests to verify the correct formatting of array values and handling of reserved characters in the 'not' operator.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds a shared Merge Risk: ⚪ Minimal · up to This change fixes array formatting for the generic negated Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| this { | ||
| this.url.searchParams.append(column, `not.${operator}.${value}`) | ||
| if (operator === 'in' && Array.isArray(value)) { | ||
| this.url.searchParams.append(column, `not.in.(${cleanFilterValues(value)})`) |
There was a problem hiding this comment.
⚪ Severity: LOW
An attacker-controlled array element reaches not.in through cleanFilterValues, but embedded " and \ are not escaped, while current PostgREST reserved ., :, and * are not quoted. The resulting URL can split or reinterpret values, changing the predicate and incorrectly scoping rows when this filter protects a privileged read.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: The root cause is in the cleanFilterValues function (lines 44–53) and the PostgrestReservedCharsRegexp pattern (line 36), not at the not.in call site itself. Two changes are needed:
- Expand
PostgrestReservedCharsRegexpat line 36 to include the additional PostgREST reserved characters (.,:,*,!,|,&) that can also alter filter semantics when unquoted:
const PostgrestReservedCharsRegexp = new RegExp('[,().:*!|&]')- Escape embedded
\and"before double-quote-wrapping incleanFilterValues(line 49). Currently"${s}"is emitted without escaping, so a value likea",bbecomes"a",b"— PostgREST splits it into two elements. The corrected map callback:
if (typeof s === 'string' && PostgrestReservedCharsRegexp.test(s))
return `"${s.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`These two changes together ensure that any string value is both correctly quoted when it contains reserved characters and that internal quotes/backslashes cannot escape the quoting, preventing filter-predicate splitting or reinterpretation.
There was a problem hiding this comment.
Fixed — cleanFilterValues now escapes embedded \ and " before
quote-wrapping, so values like a",b can no longer break out of their
quoting and split the filter list.
Deliberately left PostgrestReservedCharsRegexp unchanged: widening it to
add . : * ! | & would change output for a large share of existing
.in()/.notIn() calls (emails, UUIDs, decimals, etc. all contain .),
which is a broader behavioral change than this PR's scope. Filed as a
separate follow-up for a maintainer to weigh in on: ##2633
- Updated the `cleanFilterValues` function to escape embedded double quotes and backslashes when quoting reserved characters. - Added tests to ensure proper escaping behavior for the 'in' operator and the 'not' operator with embedded quotes and backslashes.
1 participant
Problem
.not(column, 'in', arrayValue)produces a malformed PostgREST URL becauseunlike
.in(),.not()has no array-aware formatting — it relies on JS'sdefault array-to-string coercion, which omits the required wrapping
parentheses. PostgREST rejects the request with a PGRST100 parse error.
Originally reported at supabase/postgrest-js#105 (repo archived; filing here
per its migration notice).
Fix
Extracted the array-cleaning logic already used by
.in()(and the existing.notIn()helper) into a sharedcleanFilterValuesfunction, and taught.not()to use it whenoperator === 'in'andvalueis an array.Why not just use
.notIn()?.notIn()already worked correctly — this doesn't touch its behavior beyondrouting it through the same shared helper. The bug is specifically in the
generic
.not(column, operator, value)escape hatch, which docs and examplesstill teach as
.not('id', 'in', '(5,6,7)'). Callers reasonably try passinga plain array there too, the same way they would to
.in(). This fix closesthat footgun without deprecating or changing
.notIn().This mirrors the same fix already shipped in postgrest-dart
(supabase/postgrest-dart#32), for consistency across SDKs.
Testing
Added 4 URL-level unit tests: array input, reserved-character quoting,
pre-formatted string input (no regression), and a non-
inoperator call(no regression). Full Docker-based integration suite not run locally
(unavailable in this environment) — CI should cover it.