Skip to content

s3: fix path double-free when presign throws after blob creation - #30351

Closed
robobun wants to merge 4 commits into
mainfrom
farm/23ec0f9e/s3-presign-path-double-free
Closed

s3: fix path double-free when presign throws after blob creation#30351
robobun wants to merge 4 commits into
mainfrom
farm/23ec0f9e/s3-presign-path-double-free

s3: fix path double-free when presign throws after blob creation

97cc97e
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 7, 2026 in 22m 40s

Code review found 1 important issue

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

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/runtime/webcore/S3File.zig:86-87 Double-free still possible when constructS3File* throws after initS3

Annotations

Check failure on line 87 in src/runtime/webcore/S3File.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

Double-free still possible when constructS3File* throws after initS3

The fix is incomplete: the same double-free still occurs if `constructS3FileWithS3Credentials*` itself throws *after* `initS3` has taken ownership of the path — its internal `errdefer store.deinit()` derefs the path once, and since the new neutralization line sits *after* the `try`, the caller's `errdefer path.deinit()` derefs it a second time. The post-`initS3` `try opts.getTruthyComptime(globalObject, "type")` is reachable from JS via a stateful `type` getter (it must succeed once for `getCred