feat: Add support for Parse.File.setDirectory() with master key to save file in directory - #2929
Merged
Merged
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdded optional directory support to file saves: Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (ParseFile)
participant API as API Controller
participant Store as File Storage Adapter
Client->>Client: setDirectory(dir) / prepare file
Client->>API: POST /files with file data + options (includes directory)
note right of API: options.directory is read and included\nin the fileData payload for Base64 saves
API->>Store: saveFile(fileData { name, data/base64, directory? })
Store-->>API: save result (url, name)
API-->>Client: save response (file metadata)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2929 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 6318 6327 +9
Branches 1517 1532 +15
=========================================
+ Hits 6318 6327 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mtrezza
changed the title
Parse.FileParse.File
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ParseFile.ts (1)
319-365:⚠️ Potential issue | 🔴 Critical
hasMetadataOrTagsdoesn't account for_directory, silently dropping directory on buffer-backed files.When a
Buffer-backed file has a directory set but no metadata/tags:
hasMetadataOrTagsisfalse(Lines 324–326 don't check_directory).- The
!hasMetadataOrTags && controller.saveBinarybranch (Line 339) is taken.saveBinaryuploads via raw binary headers and never includesdirectory— the value is silently discarded.The fix is to include
_directoryin the condition that decides whether to use the base64/JSON encoding path:🐛 Proposed fix
- const hasMetadataOrTags = + const hasMetadataOrTagsOrDirectory = (this._metadata && Object.keys(this._metadata).length > 0) || - (this._tags && Object.keys(this._tags).length > 0); + (this._tags && Object.keys(this._tags).length > 0) || + !!this._directory; - if (this._source.format === 'stream' && hasMetadataOrTags) { + if (this._source.format === 'stream' && hasMetadataOrTagsOrDirectory) { throw new Error( 'Cannot save a stream-based file with metadata or tags. Use a Buffer instead.' ); } ... - if (!hasMetadataOrTags && controller.saveBinary) { + if (!hasMetadataOrTagsOrDirectory && controller.saveBinary) { // Binary upload via ajax ... - } else if (this._source.format === 'buffer') { + } else if (this._source.format === 'buffer') {Note: The stream + directory combination will now throw the existing "Cannot save a stream-based file with metadata or tags" error, which is appropriate since streams also can't carry directory to the server. Consider updating that error message to mention directory as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 319 - 365, hasMetadataOrTags currently omits this._directory so buffer-backed files with a directory can be routed to the saveBinary path and lose their directory; update the hasMetadataOrTags computation (used in the save logic around the variable name hasMetadataOrTags and the branches that call controller.saveBinary and controller.saveBase64) to also consider a non-empty this._directory (e.g. include (this._directory && this._directory.length > 0) in the OR chain), and while here update the stream error thrown in the this._source.format === 'stream' check to mention directory as well so it reads that streams cannot be saved with metadata, tags, or directory.
🤖 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/__tests__/ParseFile-test.js`:
- Around line 808-868: The test points out that ParseFile instances constructed
from a Buffer that have setDirectory(...) end up using the saveBinary path which
never includes the directory in the request; update ParseFile.save (and/or
saveBinary) so that when this.directory is set (and metadata/tags are empty) the
file is uploaded via the same code path that injects fileData.directory (i.e.,
fall back to the base64/json upload or add directory to the binary payload),
specifically modify the logic in ParseFile.save and the helper saveBinary method
in ParseFile.ts to include this.directory in the outgoing request (or route to
saveBase64) so the request body contains fileData.directory for buffer-backed
files with setDirectory().
In `@src/ParseFile.ts`:
- Around line 519-529: setDirectory currently accepts any string (including
empty) and save() does not enforce the JSDoc requirement that saving with a
custom directory requires the master key; update setDirectory(directory: string)
to validate that directory is a non-empty string (throw a synchronous Error if
invalid) and modify save(options?) to synchronously check if this._directory is
set and options?.useMasterKey !== true, throwing a clear Error (e.g. "Saving to
a custom directory requires useMasterKey: true") before any async work;
reference the setDirectory, save, _directory and useMasterKey symbols when
making the changes.
- Around line 596-602: The payload construction omits an explicitly set empty
string directory because the code uses a truthy check on options.directory;
update the merge to use an explicit undefined check so empty strings are
preserved: replace the ternary that builds { ...(options.directory ? {
directory: options.directory } : {}) } with an explicit check such as {
...(options.directory !== undefined ? { directory: options.directory } : {}) }
(or alternately enforce non-empty strings in setDirectory by changing its guard
to typeof directory === 'string' && directory.length > 0); refer to the
setDirectory setter/_directory and the payload construction where
options.directory is merged into fileData.
---
Outside diff comments:
In `@src/ParseFile.ts`:
- Around line 319-365: hasMetadataOrTags currently omits this._directory so
buffer-backed files with a directory can be routed to the saveBinary path and
lose their directory; update the hasMetadataOrTags computation (used in the save
logic around the variable name hasMetadataOrTags and the branches that call
controller.saveBinary and controller.saveBase64) to also consider a non-empty
this._directory (e.g. include (this._directory && this._directory.length > 0) in
the OR chain), and while here update the stream error thrown in the
this._source.format === 'stream' check to mention directory as well so it reads
that streams cannot be saved with metadata, tags, or directory.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@types/ParseFile.d.ts`:
- Around line 227-233: Update the JSDoc for ParseFile.save() to document the
"directory" save option (matching setDirectory(directory: string): void) and
note that providing a directory requires the Master Key; ensure the save()
options list includes the "directory" field, its type (string), purpose
(directory path where the file will be stored), and the same master-key
constraint as setDirectory so API docs remain consistent with setDirectory and
ParseFile.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/ParseFile.ts (1)
314-319:⚠️ Potential issue | 🟠 Major
save()still doesn't enforceuseMasterKeywhen_directoryis set.The JSDoc on
setDirectory(Line 522) states "Requires the Master Key when saving", butsave()has no client-side guard. A caller who sets a directory withoutuseMasterKey: truewill only get a server-side rejection instead of an immediate, clear error.🛡️ Proposed fix — early guard in save()
save(options?: FileSaveOptions & { requestTask?: any }): Promise<ParseFile> | undefined { options = options || {}; + if (this._directory && !options.useMasterKey) { + throw new Error('Saving a Parse.File with a directory requires the Master Key.'); + } options.requestTask = task => (this._requestTask = task);Based on learnings: In
src/ParseFile.ts, follow the existing pattern of throwing synchronous errors for validation checks that occur before any asynchronous operations begin.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 314 - 319, The save() method must enforce the "requires master key" rule for files with a directory set: inside ParseFile.save (before any async work), if this._directory is truthy and the passed options (or defaulted options) do not have useMasterKey === true, throw a synchronous Error with a clear message; mirror the validation pattern used elsewhere (e.g., setDirectory JSDoc) so callers get an immediate failure instead of relying on a server-side rejection. Ensure you reference the same options object used later (options.requestTask assignment) and use this._directory to detect the requirement.
🧹 Nitpick comments (2)
src/ParseFile.ts (1)
596-599: Consider normalizing/validatingdirectoryvalue before sending to server.The
directoryvalue from the user is passed straight into the REST payload without any sanitization (e.g., stripping leading/trailing slashes, blocking..path traversal sequences, or rejecting special characters). While server-side validation is the ultimate safeguard, a client-side check insetDirectorycould catch obvious mistakes early — for example, rejecting paths containing..segments.💡 Optional: basic client-side validation in setDirectory
setDirectory(directory: string) { - if (typeof directory === 'string' && directory.length > 0) { + if (typeof directory === 'string' && directory.length > 0 && !directory.includes('..')) { this._directory = directory; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 596 - 599, The directory value is currently passed directly into the REST payload; add client-side normalization and validation in the setter (setDirectory) so the value sent in metadata/tags/payload is safe: trim whitespace, remove leading/trailing slashes, collapse duplicate slashes, reject or throw on any '..' path segments or disallowed chars (e.g., control chars or null bytes), and optionally reject empty results; ensure the normalizedDirectory (the cleaned value from setDirectory) is used where the payload assembles directory (the object that currently spreads ...(options.directory ? { directory: options.directory } : {})) so only the validated/normalized value is sent.src/__tests__/ParseFile-test.js (1)
814-874: Consider adding a test for directory propagation through the URI save path.The URI path (
{ uri: '...' }) downloads and then callssaveBase64with the sameoptionsobject. There's no test verifying thatdirectorysurvives the download →saveBase64chain for URI-backed files. A test like this would close the gap:it('should include directory in fileData payload when saving with uri', async () => { const request = jest.fn((method, path) => { const name = path.substr(path.indexOf('/') + 1); return Promise.resolve({ name, url: 'https://files.example.com/a/' + name }); }); CoreManager.setRESTController({ request, ajax: jest.fn() }); jest.spyOn(defaultController, 'download').mockResolvedValueOnce({ base64: 'ParseA==', contentType: 'image/png', }); const file = new ParseFile('parse.png', { uri: 'https://example.com/image.png' }); file.setDirectory('user-uploads/photos'); await file.save(); expect(request).toHaveBeenCalledWith( 'POST', 'files/parse.png', expect.objectContaining({ fileData: expect.objectContaining({ directory: 'user-uploads/photos' }), }), expect.any(Object) ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/ParseFile-test.js` around lines 814 - 874, Missing test: ensure directory set via ParseFile.setDirectory propagates through the URI save path which downloads and calls saveBase64; add a unit test that creates a ParseFile with { uri: '...' }, calls setDirectory('user-uploads/...'), mocks defaultController.download to return base64 and contentType, sets CoreManager RESTController with a jest.fn() request, calls file.save(), and asserts the POST to 'files/<name>' includes fileData.directory in the payload (use expect.objectContaining or similar to only assert directory) so the directory survives download -> saveBase64 chain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/ParseFile.ts`:
- Around line 314-319: The save() method must enforce the "requires master key"
rule for files with a directory set: inside ParseFile.save (before any async
work), if this._directory is truthy and the passed options (or defaulted
options) do not have useMasterKey === true, throw a synchronous Error with a
clear message; mirror the validation pattern used elsewhere (e.g., setDirectory
JSDoc) so callers get an immediate failure instead of relying on a server-side
rejection. Ensure you reference the same options object used later
(options.requestTask assignment) and use this._directory to detect the
requirement.
---
Nitpick comments:
In `@src/__tests__/ParseFile-test.js`:
- Around line 814-874: Missing test: ensure directory set via
ParseFile.setDirectory propagates through the URI save path which downloads and
calls saveBase64; add a unit test that creates a ParseFile with { uri: '...' },
calls setDirectory('user-uploads/...'), mocks defaultController.download to
return base64 and contentType, sets CoreManager RESTController with a jest.fn()
request, calls file.save(), and asserts the POST to 'files/<name>' includes
fileData.directory in the payload (use expect.objectContaining or similar to
only assert directory) so the directory survives download -> saveBase64 chain.
In `@src/ParseFile.ts`:
- Around line 596-599: The directory value is currently passed directly into the
REST payload; add client-side normalization and validation in the setter
(setDirectory) so the value sent in metadata/tags/payload is safe: trim
whitespace, remove leading/trailing slashes, collapse duplicate slashes, reject
or throw on any '..' path segments or disallowed chars (e.g., control chars or
null bytes), and optionally reject empty results; ensure the normalizedDirectory
(the cleaned value from setDirectory) is used where the payload assembles
directory (the object that currently spreads ...(options.directory ? {
directory: options.directory } : {})) so only the validated/normalized value is
sent.
Parse.FileParse.File.setDirectory() with master key to set a directory to save the file in
mtrezza
changed the title
Parse.File.setDirectory() with master key to set a directory to save the file inParse.File.setDirectory() with master key to save the file in a directory
mtrezza
changed the title
Parse.File.setDirectory() with master key to save the file in a directoryParse.File.setDirectory() with master key to save file in directory
parseplatformorg
pushed a commit
that referenced
this pull request
Feb 25, 2026
# [8.3.0-alpha.1](8.2.0...8.3.0-alpha.1) (2026-02-25) ### Features * Add support for `Parse.File.setDirectory()` with master key to save file in directory ([#2929](#2929)) ([1923db0](1923db0))
Contributor
|
🎉 This change has been released in version 8.3.0-alpha.1 |
2 tasks
parseplatformorg
pushed a commit
that referenced
this pull request
Feb 25, 2026
# [8.3.0](8.2.0...8.3.0) (2026-02-25) ### Features * Add support for `Parse.File.setDirectory()` with master key to save file in directory ([#2929](#2929)) ([1923db0](1923db0))
Contributor
|
🎉 This change has been released in version 8.3.0 |
4 tasks
2 tasks
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.
Pull Request
Issue
Setting a directory in which to store a
Parse.Fileis currently not possible, as the filename validation in Parse Server as part of the file name. The Parse Server S3 storage adapter allows to set a bucket prefix for all files, but not on a per-file level.Approach
Add support for directory using master key when saving a
Parse.File. Adds a directory property toParse.Filethat allows specifying a storage directory path when saving files. The directory is prepended to the filename before passing it to the storage adapter.The
beforeSaveFiletrigger can also set the directory server-side. No S3 adapter changes are needed as it already handles/in filenames.Requires additional PR for parse-server, where integration tests will be added.
Tasks
Summary by CodeRabbit
New Features
Bug Fixes / Improvements