Blob/File type: preserve user-supplied MIME verbatim (WHATWG File API) #29258
Code review found 1 important issue
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/bun.js/webcore/S3File.zig:286-292 |
s3.test.ts not updated: still expects old charset-appended type, causing CI failures on all platforms |
| 🟡 Nit | src/bun.js/webcore/Blob.zig:3553-3555 |
Dead branch in dupeWithContentType still calls canonicalizing mimeType() instead of mimeTypeInternedValue() |
| 🟣 Pre-existing | src/bun.js/webcore/Blob.zig:2854-2866 |
getSlice early return for empty blobs ignores caller-supplied content_type |
Annotations
Check failure on line 292 in src/bun.js/webcore/S3File.zig
claude / Claude Code Review
s3.test.ts not updated: still expects old charset-appended type, causing CI failures on all platforms
test/js/bun/s3/s3.test.ts still asserts type: "text/plain;charset=utf-8" for an S3File constructed with { type: 'text/plain' }, but the PR's fix to S3File.zig now stores the type verbatim as "text/plain", causing this test to fail on all CI platforms. Update the assertion to expect "text/plain" to match the corrected behavior.
Check warning on line 3555 in src/bun.js/webcore/Blob.zig
claude / Claude Code Review
Dead branch in dupeWithContentType still calls canonicalizing mimeType() instead of mimeTypeInternedValue()
The dead \!include_content_type branch in dupeWithContentType still calls jsc.VirtualMachine.get().mimeType(duped.content_type) (the canonicalizing helper), while all other call sites in this PR were updated to mimeTypeInternedValue(). When the deferred follow-up fixes the guards by changing duped.isHeapAllocated() to this.isHeapAllocated(), this branch will become live and silently re-introduce the exact WHATWG charset-canonicalization violation this PR was written to fix. The fix is to change
Check notice on line 2866 in src/bun.js/webcore/Blob.zig
claude / Claude Code Review
getSlice early return for empty blobs ignores caller-supplied content_type
Pre-existing WHATWG spec violation: new Blob([]).slice(0, 0, 'text/plain').type returns '' instead of 'text/plain' because getSlice returns early with Blob.initEmpty() when this.size == 0, before ever reaching the content_type parsing logic. This PR modifies the content_type handling code inside getSlice and adds a regression test for .slice(start, end, type), but the test only exercises a non-empty parent blob, leaving this edge case uncovered.