Skip to content

s3: don't double-free path when presign throws after blob creation - #30467

Closed
robobun wants to merge 2 commits into
mainfrom
farm/d3e5cd36/fix-s3-path-double-deref
Closed

s3: don't double-free path when presign throws after blob creation#30467
robobun wants to merge 2 commits into
mainfrom
farm/d3e5cd36/fix-s3-path-double-deref

s3: don't double-free path when presign throws after blob creation

ffe2a78
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 10, 2026 in 22m 51s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/runtime/webcore/Blob.zig:2072-2073 errdefer deinitAndUnprotect double-unprotects Buffer-backed s3:// paths

Annotations

Check warning on line 2073 in src/runtime/webcore/Blob.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

errdefer deinitAndUnprotect double-unprotects Buffer-backed s3:// paths

nit: This errdefer uses `path.deinitAndUnprotect()` while every other new errdefer in this PR (`S3Client.staticFile`, `S3File.constructInternal`, the `S3Client` instance methods) uses `path.deinit()`. For a `.buffer`-backed path (e.g. `Bun.file(Buffer.from("s3://b/k"), {accessKeyId: 123})`) where `getCredentialsWithOptions` throws before the neuter, this unprotects the buffer once here and again in `defer args.deinit()` from the `protectEat` bitset (+1/-2). JSC's `HashCountedSet::remove` tolerat