Fix crash in Blob.writer() when options contain invalid path or fd - #30253
Closed
robobun wants to merge 1 commit into
Closed
Fix crash in Blob.writer() when options contain invalid path or fd#30253robobun wants to merge 1 commit into
robobun wants to merge 1 commit into
Claude / Claude Code Review
completed
May 4, 2026 in 12m 44s
Code review found 1 important issue
Found 4 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | test/js/bun/util/filesink.test.ts:272-280 |
New writer() option-validation tests will fail on Windows |
| 🟡 Nit | src/runtime/webcore/Blob.zig:2977 |
FileSink leaked when fromJSWithTag throws JSError |
Annotations
Check failure on line 280 in test/js/bun/util/filesink.test.ts
claude / Claude Code Review
New writer() option-validation tests will fail on Windows
These two new tests will fail on Windows CI: `getWriter` has an `if (Environment.isWindows)` block (Blob.zig:2890-2952) that opens the blob's own path and returns at line 2951 without ever reading `arguments[0]` or calling `fromJSWithTag`, so on Windows `file.writer({ path: Int32Array })` / `file.writer({ fd: "not-a-number" })` succeed and return a valid FileSink instead of throwing. Either gate these with `it.skipIf(isWindows)` or move the option validation above the Windows early-return.
Check warning on line 2977 in src/runtime/webcore/Blob.zig
claude / Claude Code Review
FileSink leaked when fromJSWithTag throws JSError
Pre-existing, but while you're here: the `try` on `fromJSWithTag` at line 2977 can also propagate a `JSError` (e.g. a throwing getter for `highWaterMark`/`path`/`fd` on the options object), in which case the `sink` allocated at line 2954 still leaks. An `errdefer sink.deref();` right after line 2954 would cover both this and the `.err` case you added, in one line.
Loading