diff --git a/src/runtime/webcore/S3Client.zig b/src/runtime/webcore/S3Client.zig index 941b8afec67..966db81d016 100644 --- a/src/runtime/webcore/S3Client.zig +++ b/src/runtime/webcore/S3Client.zig @@ -145,7 +145,7 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(2).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { if (args.len() == 0) { return globalThis.ERR(.MISSING_ARGS, "Expected a path to presign", .{}).throw(); } @@ -155,6 +155,7 @@ pub const S3Client = struct { const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); return S3File.getPresignUrlFrom(&blob, globalThis, options); } @@ -163,7 +164,7 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(2).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { if (args.len() == 0) { return globalThis.ERR(.MISSING_ARGS, "Expected a path to check if it exists", .{}).throw(); } @@ -172,6 +173,7 @@ pub const S3Client = struct { errdefer path.deinit(); const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); return S3File.S3BlobStatTask.exists(globalThis, &blob); } @@ -180,7 +182,7 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(2).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { if (args.len() == 0) { return globalThis.ERR(.MISSING_ARGS, "Expected a path to check the size of", .{}).throw(); } @@ -189,6 +191,7 @@ pub const S3Client = struct { errdefer path.deinit(); const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); return S3File.S3BlobStatTask.size(globalThis, &blob); } @@ -197,7 +200,7 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(2).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { if (args.len() == 0) { return globalThis.ERR(.MISSING_ARGS, "Expected a path to check the stat of", .{}).throw(); } @@ -206,6 +209,7 @@ pub const S3Client = struct { errdefer path.deinit(); const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); return S3File.S3BlobStatTask.stat(globalThis, &blob); } @@ -214,7 +218,7 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(3).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { return globalThis.ERR(.MISSING_ARGS, "Expected a path to write to", .{}).throw(); }; errdefer path.deinit(); @@ -224,6 +228,7 @@ pub const S3Client = struct { const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); var blob_internal: PathOrBlob = .{ .blob = blob }; return Blob.writeFileInternal(globalThis, &blob_internal, data, .{ @@ -248,12 +253,13 @@ pub const S3Client = struct { const arguments = callframe.arguments_old(2).slice(); var args = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments); defer args.deinit(); - const path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { + var path: jsc.Node.PathLike = try jsc.Node.PathLike.fromJS(globalThis, &args) orelse { return globalThis.ERR(.MISSING_ARGS, "Expected a path to unlink", .{}).throw(); }; errdefer path.deinit(); const options = args.nextEat(); var blob = try S3File.constructS3FileWithS3CredentialsAndOptions(globalThis, path, options, ptr.credentials, ptr.options, ptr.acl, ptr.storage_class, ptr.request_payer); + path = .{ .string = bun.PathString.empty }; defer blob.detach(); return blob.store.?.data.s3.unlink(blob.store.?, globalThis, options); } diff --git a/src/runtime/webcore/S3File.zig b/src/runtime/webcore/S3File.zig index 41e900fb987..c8b645512b7 100644 --- a/src/runtime/webcore/S3File.zig +++ b/src/runtime/webcore/S3File.zig @@ -84,6 +84,9 @@ pub fn presign(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.J } const options = args.nextEat(); var blob = try constructS3FileInternalStore(globalThis, path.path, options); + // Path ownership transferred to blob's store via toThreadSafe(). + // Prevent errdefer from double-freeing the underlying string. + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return try getPresignUrlFrom(&blob, globalThis, options); }, @@ -114,6 +117,7 @@ pub fn unlink(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS } const options = args.nextEat(); var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return try blob.store.?.data.s3.unlink(blob.store.?, globalThis, options); }, @@ -151,6 +155,7 @@ pub fn write(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSE return globalThis.throwInvalidArguments("Expected a S3 or path to upload", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); var blob_internal: PathOrBlob = .{ .blob = blob }; @@ -190,6 +195,7 @@ pub fn size(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSEr return globalThis.throwInvalidArguments("Expected a S3 or path to get size", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.size(globalThis, &blob); @@ -223,6 +229,7 @@ pub fn exists(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JS return globalThis.throwInvalidArguments("Expected a S3 or path to check if it exists", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.exists(globalThis, &blob); @@ -574,6 +581,7 @@ pub fn stat(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSEr return globalThis.throwInvalidArguments("Expected a S3 or path to get size", .{}); } var blob = try constructS3FileInternalStore(globalThis, path.path, options); + path_or_blob = .{ .path = .{ .fd = bun.invalid_fd } }; defer blob.deinit(); return S3BlobStatTask.stat(globalThis, &blob); diff --git a/test/js/bun/s3/s3-presign-path-deinit.test.ts b/test/js/bun/s3/s3-presign-path-deinit.test.ts new file mode 100644 index 00000000000..f6606e2df4f --- /dev/null +++ b/test/js/bun/s3/s3-presign-path-deinit.test.ts @@ -0,0 +1,20 @@ +import { expect, test } from "bun:test"; + +// Regression test for double-free of path in S3 static and instance methods. +// The bug crashes under ASAN/debug builds when signRequest fails after +// constructS3FileInternalStore/constructS3FileWithS3CredentialsAndOptions +// transfers path ownership to the blob store via toThreadSafe(). +// Both errdefer and defer would deref the same WTFStringImpl. + +// Instance method path (S3Client.zig) — uses explicit empty credentials +// to ensure missing-credentials path regardless of ambient AWS/S3 env vars. +test("S3Client instance presign does not crash on missing credentials", () => { + const client = new Bun.S3Client({ accessKeyId: "", secretAccessKey: "" }); + expect(() => client.presign("a")).toThrow("Missing S3 credentials"); + expect(() => client.presign("some-key", {})).toThrow("Missing S3 credentials"); +}); + +// Static method path (S3File.zig) — exercises the separate static entrypoint. +test("S3Client static presign does not crash on missing credentials", () => { + expect(() => Bun.S3Client.presign("a")).toThrow("Missing S3 credentials"); +});