fix: Input type validation for query operators and batch path#10230
Merged
mtrezza merged 2 commits intoparse-community:alphafrom Mar 17, 2026
Merged
fix: Input type validation for query operators and batch path#10230mtrezza merged 2 commits intoparse-community:alphafrom
mtrezza merged 2 commits intoparse-community:alphafrom
Conversation
Add type validation to prevent uncaught TypeErrors from non-string inputs in query operators and batch subrequests, returning proper Parse errors instead of internal server errors. - Validate `$regex` is a string in `DatabaseController.validateQuery` - Validate `$options` is a string in `DatabaseController.validateQuery` - Catch invalid JSON in `where` parameter in `ClassesRouter` and `AggregateRouter` - Validate batch subrequest `path` is a string in `batch.js` - Validate `$in` element types match field schema type in Postgres adapter
|
🚀 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. |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds request-level and query operator input validation: guards JSON.parse for Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant Controller
participant PostgresAdapter
participant Database
Client->>Router: HTTP request (body.where / batch requests)
Router->>Router: parse body.where (try/catch) / validate subrequest paths
Router->>Controller: pass parsed query
Controller->>Controller: validate query keys ($regex, $options types)
Controller->>PostgresAdapter: build constraints
PostgresAdapter->>PostgresAdapter: validate $in/$nin elements for String fields
PostgresAdapter->>Database: execute SQL
Database-->>PostgresAdapter: results
PostgresAdapter-->>Controller: results
Controller-->>Router: response
Router-->>Client: HTTP response (200 or 4xx)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10230 +/- ##
==========================================
- Coverage 92.60% 92.19% -0.42%
==========================================
Files 192 192
Lines 16324 16341 +17
Branches 200 200
==========================================
- Hits 15117 15065 -52
- Misses 1190 1255 +65
- Partials 17 21 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/Controllers/DatabaseController.js (1)
163-168: Improve$regexvalidation to use property-presence checks instead of truthy guard.The condition at line 162 uses a truthy check (
query[key].$regex) which skips the validation block when$regexis an empty string. While empty-string regex patterns are uncommon in practice, the validation should check for property presence rather than truthiness to be defensive.Suggested improvement
Object.keys(query).forEach(key => { - if (query && query[key] && query[key].$regex) { + const fieldConstraint = query?.[key]; + if ( + fieldConstraint && + typeof fieldConstraint === 'object' && + Object.prototype.hasOwnProperty.call(fieldConstraint, '$regex') + ) { if (typeof query[key].$regex !== 'string') { throw new Parse.Error(Parse.Error.INVALID_QUERY, '$regex value must be a string'); } - if (query[key].$options !== undefined && typeof query[key].$options !== 'string') { + if ( + Object.prototype.hasOwnProperty.call(fieldConstraint, '$options') && + typeof fieldConstraint.$options !== 'string' + ) { throw new Parse.Error(Parse.Error.INVALID_QUERY, '$options value must be a string'); } if (typeof query[key].$options === 'string') { - if (!query[key].$options.match(/^[imxsu]+$/)) { + if (!fieldConstraint.$options.match(/^[imxsu]+$/)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Controllers/DatabaseController.js` around lines 163 - 168, Replace the truthy guard used to detect presence of $regex with an explicit property-presence check so empty-string patterns are validated: change the condition that currently checks query[key].$regex to use Object.prototype.hasOwnProperty.call(query[key], '$regex') (or an equivalent 'in' check) before enforcing the string type and throwing new Parse.Error(Parse.Error.INVALID_QUERY, '$regex value must be a string'); keep the existing $options validation but ensure it similarly treats explicit empty strings as valid strings by checking property presence if needed; reference the variables query and key and the Parse.Error/INVALID_QUERY usage when making the change.spec/ParseQuery.spec.js (1)
5577-5750: Consider tightening failure assertions and parameterizing invalid-type cases.Most negative tests only assert
status: 400; adding a lightweight assertion on Parse error payload (data.code/ error text) will reduce false positives. The repeated invalid-type cases can also be table-driven to lower maintenance cost.♻️ Example refactor pattern
+ async function expectBadWhere(where) { + await expectAsync( + request({ + method: 'GET', + url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`, + headers: restHeaders, + }) + ).toBeRejectedWith( + jasmine.objectContaining({ + status: 400, + data: jasmine.objectContaining({ code: jasmine.any(Number) }), + }) + ); + } + - it('rejects numeric $regex in query', async () => { - const where = JSON.stringify({ field: { $regex: 123 } }); - await expectAsync( - request({ - method: 'GET', - url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`, - headers: restHeaders, - }) - ).toBeRejectedWith(jasmine.objectContaining({ status: 400 })); - }); + it('rejects invalid $regex types in query', async () => { + const invalidValues = [{ a: 1 }, 123, ['test']]; + for (const value of invalidValues) { + await expectBadWhere(JSON.stringify({ field: { $regex: value } })); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/ParseQuery.spec.js` around lines 5577 - 5750, The tests in ParseQuery.spec.js only assert HTTP 400 for many invalid-query cases (e.g., the "rejects ... $regex" tests and the invalid JSON "where" tests using request + expectAsync(...).toBeRejectedWith(jasmine.objectContaining({ status: 400 }))); update each negative test to also assert the Parse error payload (e.g., check rejection.value.response.data.code or response.data.error/message) to ensure the specific Parse validation error is returned, and refactor repeated invalid-type cases (the three $regex rejects, the $options rejects, and other similar tests) into a table-driven loop (array of cases with name/where-payload) to reduce duplication while keeping one detailed assertion per case.spec/batch.spec.js (1)
597-684: Good path-type coverage; consider asserting specific parse error payload and table-driving invalid-path tests.The current checks validate
400correctly, but assertingerror.data.code/ message for invalidpathwould make the suite more regression-resistant, and a small invalid-case table would reduce repetition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/batch.spec.js` around lines 597 - 684, Update the batch subrequest path-type tests to assert the specific parse error payload rather than only status: when expecting rejection from request(...) in the 'subrequest path type validation' describe block, check that the thrown response contains the expected error body (e.g., error.data.code and/or error.data.error/message fields) so failures are deterministic; also consolidate the several invalid-path it tests into a small table-driven loop (e.g., an array of invalidPath cases iterated to send the same request and assert status 400 plus the specific error payload) while keeping the valid string-path test as-is; locate and change assertions around the request(...) calls in the tests inside this describe block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 489-499: The validator currently hardcodes "$in" in the
Parse.Error message even when validating a "$nin" operator; update the error
message to use the actual operator name instead of the literal "$in". In the
validation block that references fieldType, baseArray, Parse.Error and
fieldName, replace the hardcoded string with a dynamic operator variable (the
one used by the surrounding function, e.g., operator/operatorName/op) so the
error reads `$<operator> element type mismatch: expected string for field
"<fieldName>"` (preserving the Parse.Error.INVALID_QUERY and thrown behavior).
In `@src/batch.js`:
- Around line 70-74: The loop over req.body.requests dereferences
restRequest.path without ensuring restRequest is a defined object, which can
cause a TypeError for null/undefined items; update the validation in the for
(const restRequest of req.body.requests) loop to first check that restRequest is
an object (e.g., not null/undefined) and then verify typeof restRequest.path ===
'string', and if either check fails throw new
Parse.Error(Parse.Error.INVALID_JSON, 'batch request path must be a string') so
invalid/missing items produce INVALID_JSON instead of a 500.
---
Nitpick comments:
In `@spec/batch.spec.js`:
- Around line 597-684: Update the batch subrequest path-type tests to assert the
specific parse error payload rather than only status: when expecting rejection
from request(...) in the 'subrequest path type validation' describe block, check
that the thrown response contains the expected error body (e.g., error.data.code
and/or error.data.error/message fields) so failures are deterministic; also
consolidate the several invalid-path it tests into a small table-driven loop
(e.g., an array of invalidPath cases iterated to send the same request and
assert status 400 plus the specific error payload) while keeping the valid
string-path test as-is; locate and change assertions around the request(...)
calls in the tests inside this describe block.
In `@spec/ParseQuery.spec.js`:
- Around line 5577-5750: The tests in ParseQuery.spec.js only assert HTTP 400
for many invalid-query cases (e.g., the "rejects ... $regex" tests and the
invalid JSON "where" tests using request +
expectAsync(...).toBeRejectedWith(jasmine.objectContaining({ status: 400 })));
update each negative test to also assert the Parse error payload (e.g., check
rejection.value.response.data.code or response.data.error/message) to ensure the
specific Parse validation error is returned, and refactor repeated invalid-type
cases (the three $regex rejects, the $options rejects, and other similar tests)
into a table-driven loop (array of cases with name/where-payload) to reduce
duplication while keeping one detailed assertion per case.
In `@src/Controllers/DatabaseController.js`:
- Around line 163-168: Replace the truthy guard used to detect presence of
$regex with an explicit property-presence check so empty-string patterns are
validated: change the condition that currently checks query[key].$regex to use
Object.prototype.hasOwnProperty.call(query[key], '$regex') (or an equivalent
'in' check) before enforcing the string type and throwing new
Parse.Error(Parse.Error.INVALID_QUERY, '$regex value must be a string'); keep
the existing $options validation but ensure it similarly treats explicit empty
strings as valid strings by checking property presence if needed; reference the
variables query and key and the Parse.Error/INVALID_QUERY usage when making the
change.
🪄 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: fe22ad82-2d88-4cbd-ae60-f686437fa498
📒 Files selected for processing (7)
spec/ParseQuery.spec.jsspec/batch.spec.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Controllers/DatabaseController.jssrc/Routers/AggregateRouter.jssrc/Routers/ClassesRouter.jssrc/batch.js
- Use operator-specific error message ($in/$nin) in Postgres adapter - Guard batch items against null/undefined before dereferencing path
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 17, 2026
# [9.6.0-alpha.34](9.6.0-alpha.33...9.6.0-alpha.34) (2026-03-17) ### Bug Fixes * Input type validation for query operators and batch path ([#10230](#10230)) ([a628911](a628911))
Contributor
|
🎉 This change has been released in version 9.6.0-alpha.34 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Input type validation for query operators and batch path
Tasks