From e9d0ddf8aa3738fea94201c5d246fdad6de8bcbc Mon Sep 17 00:00:00 2001 From: Ciro Spaciari MacBook Date: Fri, 16 Jan 2026 14:44:10 -0800 Subject: [PATCH] fix(s3): address code review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - S3Stat.zig: Use >= instead of > for prefix length check and skip empty keys - credentials.zig: Fix memory leak on partial allocation failure in MetadataMap.dupe - credentials.zig: Throw error for non-string metadata values instead of silent fallback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/bun.js/webcore/S3Stat.zig | 3 ++- src/s3/credentials.zig | 25 +++++++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/bun.js/webcore/S3Stat.zig b/src/bun.js/webcore/S3Stat.zig index 539cf16bcb..3e22a98a3e 100644 --- a/src/bun.js/webcore/S3Stat.zig +++ b/src/bun.js/webcore/S3Stat.zig @@ -55,11 +55,12 @@ pub const S3Stat = struct { for (headers) |header| { // Case-insensitive check for x-amz-meta- prefix - if (header.name.len > prefix_len and + if (header.name.len >= prefix_len and strings.eqlCaseInsensitiveASCII(header.name[0..prefix_len], prefix, true)) { // Strip the prefix to get the user's key name const key = header.name[prefix_len..]; + if (key.len == 0) continue; // Skip empty keys if any const value_js = try bun.String.createUTF8ForJS(globalThis, header.value); // put() accepts []const u8 directly and wraps it in ZigString diff --git a/src/s3/credentials.zig b/src/s3/credentials.zig index f41b97ec1b..11660bb7c5 100644 --- a/src/s3/credentials.zig +++ b/src/s3/credentials.zig @@ -81,8 +81,21 @@ pub const MetadataMap = struct { }; for (0..n) |i| { - new_keys[i] = allocator.dupe(u8, this.keys[i]) catch return null; - new_values[i] = allocator.dupe(u8, this.values[i]) catch return null; + new_keys[i] = allocator.dupe(u8, this.keys[i]) catch { + // Free previously allocated keys + for (0..i) |j| allocator.free(new_keys[j]); + allocator.free(new_keys); + allocator.free(new_values); + return null; + }; + new_values[i] = allocator.dupe(u8, this.values[i]) catch { + // Free previously allocated keys and values + for (0..i + 1) |j| allocator.free(new_keys[j]); + for (0..i) |j| allocator.free(new_values[j]); + allocator.free(new_keys); + allocator.free(new_values); + return null; + }; } return .{ @@ -409,8 +422,12 @@ pub const S3Credentials = struct { values[i] = bun.default_allocator.dupe(u8, val_slice.slice()) catch bun.outOfMemory(); val_slice.deinit(); } else { - // Non-string value, use empty string or skip - values[i] = ""; + // Clean up previously allocated keys/values before throwing + for (0..i + 1) |j| bun.default_allocator.free(keys[j]); + for (0..i) |j| bun.default_allocator.free(values[j]); + bun.default_allocator.free(keys); + bun.default_allocator.free(values); + return globalObject.throwInvalidArgumentTypeValue("metadata value", "string", val); } i += 1; }