fix: Special characters in list-based where constraints break URL parsing#1134
fix: Special characters in list-based where constraints break URL parsing#1134AdrianCurtin wants to merge 4 commits intoparse-community:masterfrom
where constraints break URL parsing#1134Conversation
…arsing
`whereContainedIn`, `whereNotContainedIn`, and `whereArrayContainsAll`
embed String elements into the `where={...}` querystring without URL
encoding. `Uri(query: ...)` leaves `&`, `=`, `+`, and `#` as valid
query sub-delimiters, so an element like "Peanut Butter & Jelly"
causes the server to split the querystring at the literal `&` and
reject the request with `code: 102 Invalid parameter for query`.
Apply `Uri.encodeComponent` to String elements at constraint-build
time, following the pattern introduced in parse-community#866 for the scalar
`whereEqualTo` / `whereContains` family. Non-String elements (numbers,
pointers, etc.) are left untouched.
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
📝 WalkthroughWalkthroughQueryBuilder string handling now encodes string inputs via two new private helpers. Scalar string parameters use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes URL parsing issues when list-based where constraints contain special characters by percent-encoding string list elements before they are embedded into the where={...} query string.
Changes:
- Encode
Stringelements passed towhereContainedIn,whereNotContainedIn, andwhereArrayContainsAllusingUri.encodeComponent. - Add a small private helper to centralize list element encoding logic.
- Add tests covering encoding of
&,=,+,#and verifying non-String list elements are unaffected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/dart/lib/src/network/parse_query.dart | Applies string-element percent-encoding for list-based constraints via a shared helper. |
| packages/dart/test/src/network/parse_query_test.dart | Adds regression tests for special-character encoding and non-String pass-through behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| queryBuilder.whereContainedIn('topics', <dynamic>[ | ||
| "RFI's & Change Orders", | ||
| 'Schedule (Impacts, Delays, Inspections)', | ||
| ]); | ||
| queryBuilder.whereNotContainedIn('excluded', <dynamic>['a=b', 'c+d']); |
There was a problem hiding this comment.
These test list literals are typed as <dynamic>, which weakens static checking without being necessary here. Prefer <String>[...] for the string cases (and <int>[...] for the numeric case) so the tests better reflect typical API usage and catch type mistakes at compile time.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dart/lib/src/network/parse_query.dart`:
- Around line 321-329: The current _encodeStringElements function
percent-encodes the raw string which allows JSON metacharacters (", \, newlines)
to reappear unescaped after query decoding and break server-side JSON parsing;
fix it by JSON-escaping each string before percent-encoding: for each element in
_encodeStringElements, if e is a String call jsonEncode(e) to produce a
quoted/escaped JSON string, strip the surrounding quotes to get the JSON-escaped
inner content, then pass that inner content to Uri.encodeComponent; leave
non-String elements untouched. Update callers/uses (e.g., whereContainedIn,
whereNotContainedIn, whereArrayContainsAll) to rely on this corrected encoding
helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47f6b1a0-6bcb-4366-8f79-76a1a86b0ebc
📒 Files selected for processing (2)
packages/dart/lib/src/network/parse_query.dartpackages/dart/test/src/network/parse_query_test.dart
Encode string list elements by JSON-escaping then percent-encoding so quotes, backslashes and newlines remain valid in the server-decoded JSON, while characters like &, =, +, and # are protected from querystring parsing. Introduces _encodeStringElement (uses jsonEncode then Uri.encodeComponent) and updates _encodeStringElements to use it. Tests updated: stricter List<String>/List<int> typing, added a test to verify JSON-escaping of quotes and backslashes, and adjusted existing expectations for encoded delimiters.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// JSON-escapes [value] and then percent-encodes the escaped content. The | ||
| /// JSON escaping keeps `"`, `\` and newlines valid inside the surrounding | ||
| /// string literal once the server URL-decodes the query; the percent | ||
| /// encoding keeps `&`, `=`, `+` and `#` from being chewed up by querystring | ||
| /// parsing on the way in. | ||
| String _encodeStringElement(String value) { | ||
| final String jsonString = jsonEncode(value); | ||
| return Uri.encodeComponent( | ||
| jsonString.substring(1, jsonString.length - 1), | ||
| ); | ||
| } |
There was a problem hiding this comment.
The new _encodeStringElement correctly JSON-escapes before percent-encoding so values with ", \\, or newlines survive server URL-decoding as valid JSON. However, the scalar string constraints in this same class (e.g. whereEqualTo, whereNotEqualTo, whereStartsWith/EndsWith, whereContains) still call Uri.encodeComponent directly on the raw String; if the original value contains " or \\, server URL-decoding will reintroduce those characters unescaped inside the JSON and can make the where={...} value invalid JSON. Consider reusing this helper (or a shared encoder) for all String constraint builders to keep behavior consistent and avoid malformed-JSON queries for quoted/backslashed input.
Replace direct Uri.encodeComponent calls with a centralized _encodeStringElement in QueryBuilder string-handling methods (whereStartsWith, whereEqualTo, whereNotEqualTo, whereContains, and search) to ensure quotes and backslashes are JSON-escaped correctly when building queries. Add a unit test to verify percent-encoded sequences (%5C%22 and %5C%5C) appear in the built query and that raw `"`/`\` do not, preventing invalid JSON after server URL-decoding.
|
Worth noting as a security upside of this change: the JSON-escape-then-percent-encode pipeline also closes a latent NoSQL-operator-injection path. Previously, a string value containing With Not the primary motivation for the fix, but a nice side effect. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #1134 +/- ##
===========================================
- Coverage 44.42% 12.42% -32.01%
===========================================
Files 62 8 -54
Lines 3730 322 -3408
===========================================
- Hits 1657 40 -1617
+ Misses 2073 282 -1791 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Simplify the Uri.encodeComponent call in _encodeStringElement to a single line and reformat test files for consistent indentation and argument/closure layout. These are whitespace/formatting changes only and do not alter runtime behavior or test logic.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/dart/lib/src/network/parse_query.dart (1)
339-348:regExis the last where-method that still bypasses_encodeStringElement.Every other where-method that takes a user-supplied
Stringvalue (whereStartsWith,whereEndsWith,whereEqualTo/whereNotEqualTo,whereContainedIn/whereNotContainedIn,whereArrayContainsAll,whereContains,whereContainsWholeWord) was updated in this PR to flow through the new helper, butregEx(column, value)still insertsvaluedirectly into the JSON via_buildQueryWithColumnValueAndOperator→jsonEncode. JSON-encoding alone does not protect against the&/=/+/#querystring-splitting issue this PR is fixing, so a user callingregEx('field', 'a&b=c')will hit the samewhere=-parsing bug thatwhereContainedInis being fixed for here.It's fair to argue regex inputs are advanced usage, but for consistency with the rest of the encoding policy you may want to either route
regExthrough_encodeStringElementas well or document explicitly that callers ofregExmust pre-encode. Flagging as recommended rather than blocking since (a) it's pre-existing behavior and (b) regex escaping has its own subtleties orthogonal to this fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dart/lib/src/network/parse_query.dart` around lines 339 - 348, The regEx method currently inserts the raw user string into the JSON and should be updated to use the same encoding helper as other where-methods; modify regEx(String column, String value) so the value is passed through _encodeStringElement before calling _buildQueryWithColumnValueAndOperator (use the same encoding flow as whereStartsWith/whereEndsWith/whereContainedIn etc.), ensuring special characters like &, =, +, # are safely encoded in the query JSON rather than relying solely on jsonEncode.packages/dart/test/src/network/parse_query_test.dart (1)
731-752: Optional: also assert end-to-end JSON roundtrip.The current assertions verify presence/absence of substrings, which is sufficient for the immediate bug. As a nice-to-have, consider adding one assertion that mimics the server: extract the
where=value,Uri.decodeComponentit, andjsonDecodethe result — then check that the decoded$inlist contains the original'He said "hi"'andr'C:\path'strings verbatim. That would catch any future regression where the encoding produces something that "looks right" by substring but fails actual JSON parsing. Not required for this PR.♻️ Sketch
final whereParam = Uri.parse('http://x?$queryString').queryParameters['where']!; final decoded = jsonDecode(whereParam) as Map<String, dynamic>; expect(decoded['topics']['\$in'], ['He said "hi"', r'C:\path']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dart/test/src/network/parse_query_test.dart` around lines 731 - 752, Add an end-to-end JSON roundtrip assertion in the test 'list-based where methods JSON-escape quotes and backslashes in String elements': after building queryString from QueryBuilder.name('Diet_Plans') with whereContainedIn('topics', ...) and calling buildQuery(), extract the where parameter (e.g. via Uri.parse('http://x?$queryString').queryParameters['where']), Uri.decodeComponent it, jsonDecode the resulting string into a Map, and assert that decoded['topics']['\$in'] equals the original list ['He said "hi"', r'C:\path'] to ensure the encoded value parses back to the exact original strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/dart/lib/src/network/parse_query.dart`:
- Around line 339-348: The regEx method currently inserts the raw user string
into the JSON and should be updated to use the same encoding helper as other
where-methods; modify regEx(String column, String value) so the value is passed
through _encodeStringElement before calling
_buildQueryWithColumnValueAndOperator (use the same encoding flow as
whereStartsWith/whereEndsWith/whereContainedIn etc.), ensuring special
characters like &, =, +, # are safely encoded in the query JSON rather than
relying solely on jsonEncode.
In `@packages/dart/test/src/network/parse_query_test.dart`:
- Around line 731-752: Add an end-to-end JSON roundtrip assertion in the test
'list-based where methods JSON-escape quotes and backslashes in String
elements': after building queryString from QueryBuilder.name('Diet_Plans') with
whereContainedIn('topics', ...) and calling buildQuery(), extract the where
parameter (e.g. via
Uri.parse('http://x?$queryString').queryParameters['where']),
Uri.decodeComponent it, jsonDecode the resulting string into a Map, and assert
that decoded['topics']['\$in'] equals the original list ['He said "hi"',
r'C:\path'] to ensure the encoded value parses back to the exact original
strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e063a41e-e4c6-47d4-b49e-32fd3549111c
📒 Files selected for processing (2)
packages/dart/lib/src/network/parse_query.dartpackages/dart/test/src/network/parse_query_test.dart
Problem
QueryBuildermethods that accept aList<dynamic>—whereContainedIn,whereNotContainedIn, andwhereArrayContainsAll— do not URL-encode String elements before embedding them into thewhere={...}querystring built bybuildQuery().The resulting string is passed to
Uri(query: ...)ingetSanitisedUri(parse_utils.dart). Dart'sUriconstructor treats&,=,+, and#as valid query sub-delimiters per RFC 3986 and does not percent-encode them when they appear inside thequery:argument. As a consequence, a list element like"Peanut Butter & Jelly"ends up on the wire as a literal&, causing the server-side querystring parser (Parse Server / Express /qs) to split thewherevalue at the unescaped delimiter. The JSON becomes malformed and the server rejects the request:Minimal reproducer
Fix
Follows the pattern introduced in #866 for the scalar
whereEqualTo/whereNotEqualTo/whereStartsWith/whereEndsWith/whereContains/whereContainsWholeWordmethods: applyUri.encodeComponentto each String element at constraint-build time. Non-String elements (numbers, pointers, embedded maps, etc.) are passed through unchanged.A small private helper
_encodeStringElementsis added to avoid duplicating the map-and-encode logic across the three call sites.Tests
Two new tests added to
packages/dart/test/src/network/parse_query_test.dart:&,=,+, and#are percent-encoded in thewhere={...}string produced bywhereContainedIn,whereNotContainedIn, andwhereArrayContainsAll, and that the raw unescaped delimiters do not appear inside the JSON values.whereContainedIn('nums', [1, 2, 3])still produces"$in":[1,2,3]with numeric elements intact.Full
dart testsuite (208 tests) passes locally.Related
ParseQuerycontains special characters #866 — original fix applyingUri.encodeComponentto scalar String constraint methods. This PR extends the same approach to the list-based methods that were missed.Summary by CodeRabbit
Bug Fixes
Tests