Skip to content

fix: Preserve UTF-8 GraphQL upload filenames with latest graphql-upload#10478

Open
coratgerl wants to merge 4 commits into
parse-community:alphafrom
coratgerl:fix-file-encodage
Open

fix: Preserve UTF-8 GraphQL upload filenames with latest graphql-upload#10478
coratgerl wants to merge 4 commits into
parse-community:alphafrom
coratgerl:fix-file-encodage

Conversation

@coratgerl
Copy link
Copy Markdown
Contributor

@coratgerl coratgerl commented May 26, 2026

Summary

  • upgrade graphql-upload to the latest ESM-only release while keeping Parse Server on its current public module model via a small private compatibility helper
  • preserve accented GraphQL upload filenames end-to-end by forcing UTF-8 multipart filename decoding, encoding the internal /files handoff safely, and allowing accented characters through filename validation
  • add a GraphQL regression test covering café.txt uploads and keep existing GraphQL upload coverage green

This PR was done in TDD.

Test plan

  • Run the focused GraphQL regression for accented filenames
  • Run spec/ParseGraphQLServer.spec.js
  • Run targeted spec/ParseFile.spec.js filename checks
  • Run npm run lint
  • Run npm test

Notes

  • npm test finishes with one unrelated pre-existing failure in Parse.Query testing order by _updated_at

Made with Cursor

Summary by CodeRabbit

  • New Features

    • File handling now normalizes Unicode filenames across upload, storage, routing and retrieval; returned URLs use percent-encoding for non-ASCII segments.
    • GraphQL file uploads integrated via a lazy-loaded upload middleware for multipart requests.
  • Bug Fixes

    • Safer upload/error handling and consistent JSON error responses from file proxying.
  • Tests

    • Added end-to-end and unit tests for Unicode filename normalization and multipart uploads.
  • Chores

    • Upgraded graphql-upload to 17.0.0.

Review Change Stack

Load the latest ESM-only graphql-upload through a small compatibility layer so GraphQL uploads keep accented filenames intact without changing Parse Server's public module model.

Co-authored-by: Cursor <cursoragent@cursor.com>
@parse-github-assistant
Copy link
Copy Markdown

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant Bot changed the title fix: preserve UTF-8 GraphQL upload filenames with latest graphql-upload fix: Preserve UTF-8 GraphQL upload filenames with latest graphql-upload May 26, 2026
@parse-github-assistant
Copy link
Copy Markdown

parse-github-assistant Bot commented May 26, 2026

🚀 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

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

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.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e9b65ba-976c-4081-87a0-41e95a16d692

📥 Commits

Reviewing files that changed from the base of the PR and between 828d0e0 and 8193231.

📒 Files selected for processing (15)
  • package-lock.json
  • package.json
  • spec/FileNameNormalization.spec.js
  • spec/FilesController.spec.js
  • spec/ParseFile.spec.js
  • spec/ParseGraphQLServer.spec.js
  • src/Adapters/Files/FilesAdapter.js
  • src/Adapters/Files/GridFSBucketAdapter.js
  • src/Controllers/FilesController.js
  • src/GraphQL/ParseGraphQLSchema.js
  • src/GraphQL/ParseGraphQLServer.js
  • src/GraphQL/helpers/graphqlUpload.js
  • src/GraphQL/loaders/defaultGraphQLTypes.js
  • src/GraphQL/loaders/filesMutations.js
  • src/Routers/FilesRouter.js

📝 Walkthrough

Walkthrough

Upgrades graphql-upload to v17 with an ESM-aware dynamic-import adapter, defers GraphQL upload type initialization to schema load, URL-encodes proxied file paths, adds Unicode filename normalization across adapters/controllers/routers, and adds tests for accented filenames.

Changes

GraphQL Upload ESM Migration and Unicode Support

Layer / File(s) Summary
Dependency upgrade to graphql-upload v17
package.json, package-lock.json
graphql-upload bumped from 15.0.2 to 17.0.0; fs-capacitor updated to 8.0.0; lockfile metadata (engines/peer deps/integrity/resolved) updated.
ESM dynamic-import adapter for graphql-upload
src/GraphQL/helpers/graphqlUpload.js
New helper caches dynamic imports of ESM-only graphql-upload modules (GraphQLUpload, processRequest); exports createGraphQLUploadMiddleware() and getGraphQLUpload(); middleware checks multipart/form-data, uses defParamCharset: 'utf8', populates req.body, and forwards errors via next(error).
Defer GraphQL type initialization to async schema load
src/GraphQL/loaders/defaultGraphQLTypes.js, src/GraphQL/ParseGraphQLSchema.js
defaultGraphQLTypes.load() is now async and awaits getGraphQLUpload() to initialize GraphQLUpload and FILE_INPUT on first schema construction; ParseGraphQLSchema.load() awaits the loader.
Wire ESM adapter into GraphQL server middleware
src/GraphQL/ParseGraphQLServer.js
Replaces direct graphqlUploadExpress() usage with createGraphQLUploadMiddleware() while preserving maxUploadSize configuration.
Unicode normalization and URL-encoded filenames
src/Adapters/Files/FilesAdapter.js, src/Adapters/Files/GridFSBucketAdapter.js, src/Controllers/FilesController.js, src/Routers/FilesRouter.js, src/GraphQL/loaders/filesMutations.js
Adds normalizeFilename() and updates validateFilename() to use Unicode property escapes; normalizes filenames before adapter/controller/router operations; URL-encodes each path segment when proxying uploads and improves non-2xx response parsing into Parse.Error.
Test accented filename preservation in uploads
spec/ParseGraphQLServer.spec.js, spec/FilesController.spec.js, spec/ParseFile.spec.js, spec/FileNameNormalization.spec.js
Adds tests that upload café.txt via GraphQL and REST flows, assert stored name and URL-encoded paths, validate normalized storage behavior across GridFS operations, and confirm round-trip content equality.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ParseGraphQLServer
  participant createGraphQLUploadMiddleware
  participant getGraphQLUpload
  participant graphql_upload_ESM
  Client->>ParseGraphQLServer: POST multipart/form-data (café.txt)
  ParseGraphQLServer->>createGraphQLUploadMiddleware: invoke middleware(req, res, next)
  createGraphQLUploadMiddleware->>getGraphQLUpload: ensure dynamic modules loaded (cached)
  getGraphQLUpload->>graphql_upload_ESM: dynamic import (GraphQLUpload, processRequest)
  createGraphQLUploadMiddleware->>graphql_upload_ESM: processRequest(req, defParamCharset: 'utf8')
  graphql_upload_ESM-->>createGraphQLUploadMiddleware: populate req.body with file fields (filename decoded to café.txt)
  createGraphQLUploadMiddleware->>ParseGraphQLServer: next()
  ParseGraphQLServer->>FilesController: validateFilename(normalizeFilename(_café.txt))
  ParseGraphQLServer->>FilesController: createFile -> adapter.createFile(normalized filename)
  FilesController->>GridFSBucketAdapter: createFile(normalized filename)
  GridFSBucketAdapter-->>FilesController: response (stored name, url with encoded path caf%C3%A9.txt)
  ParseGraphQLServer-->>Client: GraphQL response with name and encoded URL
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • mtrezza
  • Moumouls
🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description provides a clear summary, test plan with checkmarks, and notes, but lacks explicit sections for 'Issue' and 'Approach' as specified in the template. Add explicit 'Issue' and 'Approach' sections to match the repository template structure, or clarify whether the 'Summary' and content serve these purposes.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title begins with the required 'fix:' prefix and clearly describes the main change: preserving UTF-8 GraphQL upload filenames with the latest graphql-upload.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed Graphql-upload 17.0.0 upgrade is secure. Filename validation blocks path traversal and injection. UTF-8 decoding only affects accented characters. URL encoding and error handling are safe.
Engage In Review Feedback ✅ Passed No reviewer feedback comments are present on the PR; only an automated full review request by the user and bot acknowledgment exist per the PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.22.0)

OpenGrep fatal error (exit code 2): [00.18][ERROR]: Error: exception Unix_error: No such file or directory stat package.json
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called from Scan_CLI.replac

🔧 Checkov (3.2.529)
package.json

2026-05-26 08:12:50,857 [MainThread ] [ERROR] Template file not found: package.json
2026-05-26 08:12:50,862 [MainThread ] [ERROR] Template file not found: package.json
2026-05-26 08:12:50,864 [MainThread ] [ERROR] Template file not found: package.json
2026-05-26 08:12:50,922 [MainThread ] [ERROR] Failed to invoke function /usr/local/lib/python3.11/dist-packages/checkov/common/runners/object_runner. with package.json
Traceback (most recent call last):
File "/usr/local/lib/python3.11/dist-packages/checkov/common/parallelizer/parallel_runner.py", line 88, in func_wrapper
result = original_func(item)
^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/checkov/common/runners/object_runner.py", line 74, in
results = parallel_runner.run_function(lambda f: (f, self._parse_file(f)), files_to_load)
^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/checkov/ope

... [truncated 2548 characters] ...

k__)
FileNotFoundError: [Errno 2] No such file or directory: 'package.json'
2026-05-26 08:12:50,973 [MainThread ] [ERROR] Exception traceback:
Traceback (most recent call last):
File "/usr/local/lib/python3.11/dist-packages/checkov/main.py", line 647, in run
self.scan_reports = runner_registry.run(
^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/checkov/common/runners/runner_registry.py", line 177, in run
for result in parallel_runner_results:
File "/usr/local/lib/python3.11/dist-packages/checkov/common/parallelizer/parallel_runner.py", line 118, in _run_function_multiprocess_fork
raise v.internal_exception.with_traceback(v.internal_exception.traceback)
FileNotFoundError: [Errno 2] No such file or directory: 'package.json'


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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/GraphQL/loaders/filesMutations.js`:
- Around line 41-53: The response handling treats 3xx as success and uses a
nonexistent Parse.error constant in the catch; change the status check to reject
for any non-2xx (use if (res.statusCode < 200 || res.statusCode >= 300)) and
update the catch to capture the thrown error and reject with a real Parse.Error
code constant (e.g., use Parse.Error.FILE_SAVE_ERROR or another appropriate
Parse.Error.* code) so the reject call uses new
Parse.Error(Parse.Error.FILE_SAVE_ERROR, errorOrData) and includes the caught
error details; locate these changes around the parsedData/res.statusCode
handling and the catch that currently calls reject(new Parse.Error(Parse.error,
data)).
🪄 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: a99d2150-8ad0-460f-ac62-c9b90904bbd1

📥 Commits

Reviewing files that changed from the base of the PR and between 828d0e0 and 2b87ddb.

📒 Files selected for processing (9)
  • package-lock.json
  • package.json
  • spec/ParseGraphQLServer.spec.js
  • src/Adapters/Files/FilesAdapter.js
  • src/GraphQL/ParseGraphQLSchema.js
  • src/GraphQL/ParseGraphQLServer.js
  • src/GraphQL/helpers/graphqlUpload.js
  • src/GraphQL/loaders/defaultGraphQLTypes.js
  • src/GraphQL/loaders/filesMutations.js

Comment thread src/GraphQL/loaders/filesMutations.js Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 94.02985% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.58%. Comparing base (155123a) to head (8193231).
⚠️ Report is 1 commits behind head on alpha.

Files with missing lines Patch % Lines
src/GraphQL/loaders/filesMutations.js 57.14% 3 Missing ⚠️
src/GraphQL/helpers/graphqlUpload.js 95.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            alpha   #10478   +/-   ##
=======================================
  Coverage   92.57%   92.58%           
=======================================
  Files         193      194    +1     
  Lines       16884    16932   +48     
  Branches      234      234           
=======================================
+ Hits        15631    15676   +45     
- Misses       1230     1233    +3     
  Partials       23       23           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Reject non-2xx responses from the internal /files handoff and use
Parse.Error.FILE_SAVE_ERROR when the response body is not valid JSON.
Add regression tests for accented filenames in FilesController and Parse.File.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coratgerl
Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Adapters/Files/FilesAdapter.js`:
- Line 115: Filename normalization is missing: validateFilename only tests the
regex but never normalizes, causing NFC/NFD mismatches across storage and
lookups. Normalize filenames (and each path segment) to a consistent Unicode
form (e.g., NFC) at the point the storage key is finalized and always pass that
normalized value into GridFS operations; update FilesAdapter (add or change
validateFilename to return/produce a normalized filename or add a new
normalizeFilename helper), ensure FilesRouter/FilesController call that
normalizer before upload/download/delete/metadata, and make GridFSBucketAdapter
use the normalized name for openUploadStream(filename),
openDownloadStreamByName, bucket.find({ filename }), delete, and any other
filename lookups.
🪄 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: bfd78a56-4652-4ed2-b00c-ac8ba20f9045

📥 Commits

Reviewing files that changed from the base of the PR and between 828d0e0 and 063b85c.

📒 Files selected for processing (11)
  • package-lock.json
  • package.json
  • spec/FilesController.spec.js
  • spec/ParseFile.spec.js
  • spec/ParseGraphQLServer.spec.js
  • src/Adapters/Files/FilesAdapter.js
  • src/GraphQL/ParseGraphQLSchema.js
  • src/GraphQL/ParseGraphQLServer.js
  • src/GraphQL/helpers/graphqlUpload.js
  • src/GraphQL/loaders/defaultGraphQLTypes.js
  • src/GraphQL/loaders/filesMutations.js

Comment thread src/Adapters/Files/FilesAdapter.js
Normalize file names to NFC before finalizing storage keys so uploads, reads, metadata lookups, and deletes resolve the same GridFS record across Unicode forms.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coratgerl
Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

✅ Actions performed

Full review triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant