fix: Server crashes due to insufficient schema validation when schema type is not a string #9848
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds input validation to mongoFieldToParseSchemaField to throw a Parse.Error when Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Init as Initializer
participant MSC as MongoSchemaCollection
participant Mapper as Mapping logic
Init->>MSC: mongoFieldToParseSchemaField(type, fieldName, className)
alt type is falsy or not a string
MSC-->>Init: throw Parse.Error(INVALID_SCHEMA_OPERATION, "Invalid field type: ${type} for field '...'. Expected a string.")
else
MSC->>Mapper: evaluate mapping branches (relation<>, pointer, basic types, arrays, geo, file, bytes, polygon)
Mapper-->>MSC: return parsed field descriptor
MSC-->>Init: return parsed field descriptor
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js (4)
54-55: Optional: add field context to the error for easier debuggingIf we keep throwing inside
mongoFieldToParseSchemaField, wrapping this call to appendclassName/fieldNamecontext improves operator UX when boot fails on bad schemas.- obj[fieldName] = mongoFieldToParseSchemaField(schema[fieldName]); + try { + obj[fieldName] = mongoFieldToParseSchemaField(schema[fieldName]); + } catch (e) { + if (e && e.code != null) { + // Preserve code; enrich message + e.message = `Invalid field type for "${fieldName}" in class "${schema._id}": ${e.message}`; + } + throw e; + }
5-11: Add regression tests for non-string/whitespace input typesTo lock this down, please add tests covering:
- type: undefined, null, number, boolean, object, array, function
- type: '' and ' ' (whitespace)
- type: 'number ' (trailing space) → with the trim change, should map to Number; without it, it silently fails
If helpful, I can draft a spec in
spec/schemas.spec.jsthat exercisesmongoSchemaFieldsToParseSchemaFieldswith a mocked schema input.
25-47: Add a default case inmongoFieldToParseSchemaFieldto reject unsupported typesI’ve confirmed that the only call site for
mongoFieldToParseSchemaFieldis withinmongoSchemaFieldsToParseSchemaFields, and no code relies on it returningundefined. Failing fast on unrecognized types will surface schema misconfigurations immediately without breaking existing behavior.• File:
src/Adapters/Storage/Mongo/MongoSchemaCollection.js
• Location: inside theswitch (type)inmongoFieldToParseSchemaFieldRecommended diff:
switch (type) { // …existing cases… case 'polygon': return { type: 'Polygon' }; + default: + throw new Parse.Error( + Parse.Error.INVALID_SCHEMA_OPERATION, + `Unsupported field type: "${type}".` + ); }
5-13: Tighten type validation and normalize inputGreat addition to guard against
TypeError. Two optional hardening tweaks:
- Reject empty or whitespace-only strings by using
type.trim().- Trim
typebefore downstream checks so trailing (or leading) spaces can’t sneak through.Proposed diff in src/Adapters/Storage/Mongo/MongoSchemaCollection.js (lines 5–13):
// Add type validation to prevent TypeError - if (!type || typeof type !== 'string') { + if (typeof type !== 'string' || type.trim() === '') { throw new Parse.Error( Parse.Error.INVALID_SCHEMA_OPERATION, - `Invalid field type: ${type}. Expected a string. Field type must be one of: string, number, boolean, date, map, object, array, geopoint, file, bytes, polygon, or a valid relation/pointer format.` + `Invalid field type: ${String(type)}. Expected a non-empty string. Field type must be one of: string, number, boolean, date, map, object, array, geopoint, file, bytes, polygon, or a valid relation/pointer format.` ); } + + // Normalize input + type = type.trim();Follow-up question:
- All other “invalid field type” guards (e.g. in SchemaController) use
Parse.Error.INCORRECT_TYPE. Should we switch fromINVALID_SCHEMA_OPERATIONtoINCORRECT_TYPEhere for consistency?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js (7)
spec/schemas.spec.js (1)
Parse(3-3)src/Controllers/SchemaController.js (1)
Parse(18-18)src/Adapters/Auth/apple.js (1)
Parse(48-48)src/Adapters/Auth/facebook.js (1)
Parse(62-62)src/Adapters/Auth/ldap.js (1)
Parse(77-77)src/SchemaMigrations/DefinedSchemas.js (1)
Parse(3-3)spec/ReadPreferenceOption.spec.js (1)
Parse(3-3)
Added a type check in mongoFieldToParseSchemaField to ensure `type` is a string before calling `startsWith`. This prevents crashes when Parse Server processes MongoDB schema fields with undefined, null, or unexpected type values. Closes parse-community#9847
rgunindi
force-pushed
the
fix/type-validation-mongo-schema
branch
from
8d90d30 to
1f9b6fe
Compare
rgunindi
changed the title
mtrezza
changed the title
mtrezza
changed the title
@mtrezza 
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
spec/MongoSchemaCollectionAdapter.spec.js (2)
3-3: Align Parse import with repo convention to ensureParse.Erroris availableOther specs import
Parsevia.Parse; this avoids ambiguity with the module export shape and is consistent across the repo.-const Parse = require('parse/node'); +const Parse = require('parse/node').Parse;
101-131: Good coverage of valid type mappingsThe table-driven checks for string/number/boolean/date/object/array/geo/file/bytes/polygon/pointer/relation look solid.
Optional: add role-based cases to catch regressions around special classes.
const testCases = [ { input: 'string', expected: { type: 'String' } }, @@ { input: 'relation<_User>', expected: { type: 'Relation', targetClass: '_User' } }, { input: 'relation<Post>', expected: { type: 'Relation', targetClass: 'Post' } }, + { input: '*_Role', expected: { type: 'Pointer', targetClass: '_Role' } }, + { input: 'relation<_Role>', expected: { type: 'Relation', targetClass: '_Role' } }, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
spec/MongoSchemaCollectionAdapter.spec.js(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
PR: parse-community/parse-server#9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Applied to files:
spec/MongoSchemaCollectionAdapter.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/MongoSchemaCollectionAdapter.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/MongoSchemaCollectionAdapter.spec.js
🧬 Code graph analysis (1)
spec/MongoSchemaCollectionAdapter.spec.js (2)
spec/schemas.spec.js (1)
Parse(3-3)src/Controllers/SchemaController.js (1)
Parse(18-18)
🔇 Additional comments (1)
spec/MongoSchemaCollectionAdapter.spec.js (1)
155-189: The script is running; I’ll review the full implementation once available.
rgunindi
force-pushed
the
fix/type-validation-mongo-schema
branch
from
f629526 to
8710966
Compare
|
Thanks, did you mean that you verified that the bug exists in PS8? |
Yes, of course. This bug exists in Ps8. |
…alidation and clear error message - Add tests for valid field type conversions - Add tests for invalid type handling (null, undefined, non-string values) - Ensure Parse.Error code is thrown for invalid inputs - Test coverage for the type validation improvements - Add a clear error explanation
rgunindi
force-pushed
the
fix/type-validation-mongo-schema
branch
from
8710966 to
a882ffe
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9848 +/- ##
=======================================
Coverage 93.01% 93.01%
=======================================
Files 187 187
Lines 15096 15098 +2
Branches 174 174
=======================================
+ Hits 14041 14043 +2
Misses 1043 1043
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js (2)
25-47: Fail fast on unknown string types (optional but safer)Right now unknown strings fall through and return
undefined, which may propagate subtly. Consider throwing to surface bad data early. Backward-compat risk is low if such values are already invalid.Apply:
case 'polygon': return { type: 'Polygon' }; + default: + throw new Parse.Error( + Parse.Error.INVALID_SCHEMA_OPERATION, + `Unsupported field type: '${type}' for field '${fieldName}' in class '${className}'.` + );
4-12: Apply: coerce invalid type to readable string and report class nameSearch found only this occurrence of INVALID_SCHEMA_OPERATION in src; change is localized—apply the diff below.
- if (!type || typeof type !== 'string') { - throw new Parse.Error( - Parse.Error.INVALID_SCHEMA_OPERATION, - `Invalid field type: ${type} for field '${fieldName}' in class '_SCHEMA' (id: ${className}). Expected a string. Fix the type mismatch in your schema configuration.` - ); - } + if (!type || typeof type !== 'string') { + const received = typeof type === 'undefined' ? 'undefined' : String(type); + throw new Parse.Error( + Parse.Error.INVALID_SCHEMA_OPERATION, + `Invalid field type: ${received} for field '${fieldName}' in class '${className}'. Expected a string. Fix the type mismatch in your schema configuration.` + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spec/MongoSchemaCollectionAdapter.spec.js(1 hunks)src/Adapters/Storage/Mongo/MongoSchemaCollection.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/MongoSchemaCollectionAdapter.spec.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js (6)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (1)
fieldName(1434-1434)src/RestWrite.js (2)
className(1742-1742)schema(343-343)src/Routers/SchemasRouter.js (3)
className(24-24)className(86-86)className(105-105)src/GraphQL/loaders/parseClassMutations.js (1)
className(40-40)src/GraphQL/loaders/parseClassTypes.js (1)
className(112-112)src/GraphQL/loaders/parseClassQueries.js (1)
className(46-46)
🔇 Additional comments (1)
src/Adapters/Storage/Mongo/MongoSchemaCollection.js (1)
54-54: Good: caller now passes field and class for richer errorsPassing
fieldNameandschema._idaddresses prior feedback and makes debugging much easier. LGTM.
… and class details - Add fieldName and className parameters to mongoFieldToParseSchemaField function - Update error message to include specific field name and class information - Show collection name (_SCHEMA) and document ID for better debugging - Pass field name and class ID from mongoSchemaFieldsToParseSchemaFields - Enhanced error message format: 'Invalid field type: X for field Y in class _SCHEMA (id: Z)' - Improves developer experience by providing clear context for schema validation errors
Added a type check in mongoFieldToParseSchemaField to ensure
typeis a string before callingstartsWith. This prevents crashes when Parse Server processes MongoDB schema fields with undefined, null, or unexpected type values.Closes #9847
Pull Request
Issue
Approach
Tasks
Summary by CodeRabbit
Bug Fixes
Tests
Chores