fix: Server crashes due to insufficient schema validation when schema type is not a string #9848
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds an upfront input guard in mongoFieldToParseSchemaField that validates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Init as Initializer
participant MSC as MongoSchemaCollection
participant Mapper as Mapping logic
Init->>MSC: mongoFieldToParseSchemaField(type)
alt type is falsy or not a string
MSC-->>Init: throw Parse.Error(INVALID_SCHEMA_OPERATION, "Invalid field type: ... 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
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 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
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