parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

fix: Server crashes due to insufficient schema validation when schema type is not a string

Open rgunindi opened this issue 3 months ago • 7 comments

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 #9847

Pull Request

Issue

Approach

Tasks

  • [x] Add tests

Summary by CodeRabbit

  • Bug Fixes

    • Improved schema validation for Mongo-backed storage: invalid, empty, or non-string field types now produce clear, user-facing errors instead of crashes.
  • Tests

    • Added comprehensive tests covering valid field-type mappings and verifying invalid inputs reliably throw the expected errors.
  • Chores

    • Strengthened input guards to reduce ambiguous failures and improve reliability during schema setup and migrations.

rgunindi avatar Aug 24 '25 17:08 rgunindi

🚀 Thanks for opening this pull request!

📝 Walkthrough

Walkthrough

Adds input validation to mongoFieldToParseSchemaField to throw a Parse.Error when type is falsy or not a string; updates the caller to pass field and class names; adds tests covering valid mappings and invalid/non-string inputs. No public API signatures changed.

Changes

Cohort / File(s) Summary
Mongo schema field type validation
src/Adapters/Storage/Mongo/MongoSchemaCollection.js
Added an upfront input guard in mongoFieldToParseSchemaField(type, fieldName, className) that throws Parse.Error (code INVALID_SCHEMA_OPERATION) with a descriptive message when type is falsy or not a string; mapping logic remains unchanged. Updated call site to pass fieldName and schema._id.
Tests: mongoFieldToParseSchemaField
spec/MongoSchemaCollectionAdapter.spec.js
Added tests exercising mappings of many Mongo field type representations to Parse schema fields and tests asserting invalid inputs (null, undefined, non-string, empty string, numeric, boolean, object, array) throw Parse.Error with code 255 and message containing "Invalid field type".

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

Objective Addressed Explanation
Prevent TypeError by ensuring type is a string before calling startsWith (#9847) Validation prevents non-string type from reaching startsWith.
Handle malformed or unexpected schema type values gracefully on startup (#9847) Function now throws a controlled Parse.Error with a descriptive message instead of raising a TypeError.

Pre-merge checks (4 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately and directly summarizes the primary change: adding validation to prevent server crashes when a schema field's type is not a string, and it maps to the changes in MongoSchemaCollection.js and the added tests.
Linked Issues Check ✅ Passed The changes implement the coding objective from issue [#9847] by validating that schema field type is a string before using string methods, throwing a Parse.Error when it is not, updating the call site to pass field and class identifiers, and adding tests that assert correct mappings and error behavior, which satisfies the linked issue's requirements.
Out of Scope Changes Check ✅ Passed The diff and provided summaries show changes confined to MongoSchemaCollection.js and its spec file with no modifications to unrelated modules, so there are no detected out-of-scope changes outside the linked issue objectives.
Description Check ✅ Passed The description includes a clear summary of the fix, links the PR to issue #9847, and notes that tests were added, so it covers the core template requirements; however the repository template's "Approach" section is empty and could be populated with a brief description of the validation logic and error behavior.

[!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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Aug 24 '25 17:08 coderabbitai[bot]

:tada: Snyk checks have passed. No issues have been found so far.

:white_check_mark: security/snyk check is complete. No issues have been found. (View Details)

parseplatformorg avatar Aug 24 '25 17:08 parseplatformorg

Please add a test to demonstrate the issue. You experienced this in Parse Server 6, we also want to make sure this bug still exists in PS 8.

@mtrezza Thanks for the suggestion. I’ve added tests that demonstrate the issue and verified them against also current version ps8.

image

rgunindi avatar Aug 28 '25 18:08 rgunindi

Thanks, did you mean that you verified that the bug exists in PS8?

mtrezza avatar Aug 28 '25 21:08 mtrezza

Thanks, did you mean that you verified that the bug exists in PS8?

Yes, of course. This bug exists in Ps8.

rgunindi avatar Aug 28 '25 21:08 rgunindi

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 93.01%. Comparing base (4b3f10b) to head (a882ffe). :warning: Report is 4 commits behind head on alpha.

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           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Sep 10 '25 14:09 codecov[bot]