-
-
Notifications
You must be signed in to change notification settings - Fork 213
fix: ParseXFile uploads file with content-type application/octet-stream if not explicitly set
#1048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
Modified ParseXFile and ParseFile to only send content-type: application/octet-stream when the filename has no extension. When a file extension exists, the server will infer the correct MIME type from the extension, fixing the issue where images were incorrectly saved as application/octet-stream. Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
Added tests to verify that files with extensions and without extensions are correctly identified, which is crucial for the content-type header logic. Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1048 +/- ##
==========================================
- Coverage 43.98% 43.55% -0.44%
==========================================
Files 61 61
Lines 3469 3577 +108
==========================================
+ Hits 1526 1558 +32
- Misses 1943 2019 +76 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot remove the logic to only send content-type: application/octet-stream when filename has no extension. Instead, there should be no default content type. A content type should only be set if one is explicitly provided. |
Changed ParseXFile and ParseFile to not set any default content-type header. Content-type is now only set if explicitly provided (e.g., via file.mimeType in XFile). This allows Parse Server to always infer the MIME type from the file extension, addressing feedback from @mtrezza. Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
Done in commit ae68a7a. Removed all default content-type logic. Now:
All 133 tests pass. |
Restored the lookupMimeType logic for MIME type detection (similar to JS SDK's base64 identification), but removed the fallback to application/octet-stream. Now the chain is: 1. file.mimeType (if provided) 2. lookupMimeType from filename/path/data 3. If still null, no content-type header sent (server infers from filename) Addresses feedback from @mtrezza to keep lookupMimeType logic. Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
application/octet-stream instead of letting Parse Server infer from filename
|
🚀 Thanks for opening this pull request! |
application/octet-stream instead of letting Parse Server infer from filenameParseXFile uploads file with default content-type application/octet-stream instead of letting Parse Server infer from filename
ParseXFile uploads file with default content-type application/octet-stream instead of letting Parse Server infer from filenameParseXFile uploads file with content-type application/octet-stream if not explicitly set instead of letting Parse Server infer from filename
ParseXFile uploads file with content-type application/octet-stream if not explicitly set instead of letting Parse Server infer from filenameParseXFile uploads file with content-type application/octet-stream if not explicitly set
2shrestha22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, however we need to test it once, I will do a test tomorrow.
|
@copilot fix lint errors |
Applied dart format to fix formatting issues across the codebase. All code now follows Dart formatting standards. No functional changes, only whitespace and formatting adjustments. Co-authored-by: mtrezza <5673677+mtrezza@users.noreply.github.com>
|
It is working. Had to spend half an hour fixing dependency issues. |
|
I just want to request you to upgrade dependencies. I have been using fork of this package for more that a year. Because some packages are not compatible with lower version pinned in Parse SDK. |
|
Could you please open a new issue for dependencies? |
Fix: Remove default content-type to let server infer from filename
Summary
This PR fixes an issue where files were being uploaded with incorrect MIME types. The SDK now uses a smart fallback chain for content-type detection while ultimately letting Parse Server infer from filename when needed.
Problem
Files were being uploaded with
content-type: application/octet-stream, causing browsers to download files instead of previewing them.Solution
Modified both
ParseXFile.upload()andParseFile.upload()to use a detection chain:file.mimeTypefrom XFile)lookupMimeType()from filename/path/data (similar to JS SDK's base64 identification)This aligns with Parse Server's behavior (FilesController.js) which uses
mime.getType(filename)to infer MIME type when no content-type is provided.Changes Made
packages/dart/lib/src/objects/parse_x_file.dart): Usesfile.mimeType→lookupMimeType()→ null (server infers)packages/dart/lib/src/objects/parse_file.dart): UseslookupMimeType()→ null (server infers)application/octet-streamExample
Before:
After:
If lookupMimeType can't determine type:
Testing
Closes #1097
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.