From f81a57f1a8c573c013d094ee79c8be7b91577e61 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Fri, 7 Feb 2025 19:17:21 -0800 Subject: [PATCH 1/3] support supabase s3 --- src/env_loader.zig | 4 ++-- src/s3/credentials.zig | 40 +++++++++++++++++++++++---------------- src/url.zig | 14 ++++++++++++++ test/js/bun/s3/s3.test.ts | 7 +++++++ 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/env_loader.zig b/src/env_loader.zig index adb7cd8ac4264e..954f9164d90842 100644 --- a/src/env_loader.zig +++ b/src/env_loader.zig @@ -144,9 +144,9 @@ pub const Loader = struct { region = region_; } if (this.get("S3_ENDPOINT")) |endpoint_| { - endpoint = bun.URL.parse(endpoint_).host; + endpoint = bun.URL.parse(endpoint_).hostWithPath(); } else if (this.get("AWS_ENDPOINT")) |endpoint_| { - endpoint = bun.URL.parse(endpoint_).host; + endpoint = bun.URL.parse(endpoint_).hostWithPath(); } if (this.get("S3_BUCKET")) |bucket_| { bucket = bucket_; diff --git a/src/s3/credentials.zig b/src/s3/credentials.zig index b0cd832a5620d1..8c07a3ff76fcd2 100644 --- a/src/s3/credentials.zig +++ b/src/s3/credentials.zig @@ -113,7 +113,7 @@ pub const S3Credentials = struct { new_credentials._endpointSlice = str.toUTF8(bun.default_allocator); const endpoint = new_credentials._endpointSlice.?.slice(); const url = bun.URL.parse(endpoint); - const normalized_endpoint = url.host; + const normalized_endpoint = url.hostWithPath(); if (normalized_endpoint.len > 0) { new_credentials.credentials.endpoint = normalized_endpoint; @@ -524,7 +524,29 @@ pub const S3Credentials = struct { var bucket_buffer: [63]u8 = undefined; bucket = encodeURIComponent(bucket, &bucket_buffer, false) catch return error.InvalidPath; path = encodeURIComponent(path, &path_buffer, false) catch return error.InvalidPath; - const normalizedPath = std.fmt.bufPrint(&normalized_path_buffer, "/{s}/{s}", .{ bucket, path }) catch return error.InvalidPath; + // Default to https. Only use http if they explicit pass "http://" as the endpoint. + const protocol = if (this.insecure_http) "http" else "https"; + + // detect service name and host from region or endpoint + var endpoint = this.endpoint; + var extra_path: []const u8 = ""; + const host = brk_host: { + if (this.endpoint.len > 0) { + if (this.endpoint.len >= 2048) return error.InvalidEndpoint; + var host = this.endpoint; + if (bun.strings.indexOf(this.endpoint, "/")) |index| { + host = this.endpoint[0..index]; + extra_path = this.endpoint[index..]; + } + // only the host part is needed here + break :brk_host try bun.default_allocator.dupe(u8, host); + } else { + endpoint = try std.fmt.allocPrint(bun.default_allocator, "s3.{s}.amazonaws.com", .{region}); + break :brk_host endpoint; + } + }; + errdefer bun.default_allocator.free(host); + const normalizedPath = std.fmt.bufPrint(&normalized_path_buffer, "{s}/{s}/{s}", .{ extra_path, bucket, path }) catch return error.InvalidPath; const date_result = getAMZDate(bun.default_allocator); const amz_date = date_result.date; @@ -595,22 +617,8 @@ pub const S3Credentials = struct { } }; - // Default to https. Only use http if they explicit pass "http://" as the endpoint. - const protocol = if (this.insecure_http) "http" else "https"; - - // detect service name and host from region or endpoint - const host = brk_host: { - if (this.endpoint.len > 0) { - if (this.endpoint.len >= 512) return error.InvalidEndpoint; - break :brk_host try bun.default_allocator.dupe(u8, this.endpoint); - } else { - break :brk_host try std.fmt.allocPrint(bun.default_allocator, "s3.{s}.amazonaws.com", .{region}); - } - }; const service_name = "s3"; - errdefer bun.default_allocator.free(host); - const aws_content_hash = if (content_hash) |hash| hash else ("UNSIGNED-PAYLOAD"); var tmp_buffer: [4096]u8 = undefined; diff --git a/src/url.zig b/src/url.zig index c3a31bc9206d95..bba7705dda6b2a 100644 --- a/src/url.zig +++ b/src/url.zig @@ -39,6 +39,20 @@ pub const URL = struct { pub fn isFile(this: *const URL) bool { return strings.eqlComptime(this.protocol, "file"); } + /// host + path without the ending slash, protocol, searchParams and hash + pub fn hostWithPath(this: *const URL) []const u8 { + if (this.host.len > 0) { + if (this.path.len > 1) { + const end = @intFromPtr(this.path.ptr) + this.path.len; + const start = @intFromPtr(this.host.ptr); + const len: usize = end - start - (if (bun.strings.endsWithComptime(this.path, "/")) @as(usize, 1) else @as(usize, 0)); + const ptr: [*]u8 = @ptrFromInt(start); + return ptr[0..len]; + } + return this.host; + } + return ""; + } pub fn isBlob(this: *const URL) bool { return this.href.len == JSC.WebCore.ObjectURLRegistry.specifier_len and strings.hasPrefixComptime(this.href, "blob:"); diff --git a/test/js/bun/s3/s3.test.ts b/test/js/bun/s3/s3.test.ts index d779a6f6fd3a3c..a825e923bc8d08 100644 --- a/test/js/bun/s3/s3.test.ts +++ b/test/js/bun/s3/s3.test.ts @@ -30,6 +30,13 @@ const allCredentials = [ bucket: getSecret("S3_R2_BUCKET"), service: "R2" as string, }, + { + accessKeyId: getSecret("SUPABASE_ACCESS_KEY"), + secretAccessKey: getSecret("SUPABASE_SECRET_KEY"), + endpoint: getSecret("SUPABASE_ENDPOINT"), + bucket: getSecret("SUPABASE_BUCKET"), + service: "supabase" as string, + }, ]; if (isDockerEnabled()) { From 33c63c1486d995b2a370473f3709c47096ef57ab Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Mon, 10 Feb 2025 18:37:25 -0800 Subject: [PATCH 2/3] rename --- test/js/bun/s3/s3.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/js/bun/s3/s3.test.ts b/test/js/bun/s3/s3.test.ts index a825e923bc8d08..a1b3cae764a05c 100644 --- a/test/js/bun/s3/s3.test.ts +++ b/test/js/bun/s3/s3.test.ts @@ -31,10 +31,11 @@ const allCredentials = [ service: "R2" as string, }, { - accessKeyId: getSecret("SUPABASE_ACCESS_KEY"), - secretAccessKey: getSecret("SUPABASE_SECRET_KEY"), - endpoint: getSecret("SUPABASE_ENDPOINT"), - bucket: getSecret("SUPABASE_BUCKET"), + accessKeyId: getSecret("S3_SUPABASE_ACCESS_KEY"), + secretAccessKey: getSecret("S3_SUPABASE_SECRET_KEY"), + endpoint: getSecret("S3_SUPABASE_ENDPOINT"), + bucket: getSecret("S3_SUPABASE_BUCKET"), + region: getSecret("S3_SUPABASE_REGION"), service: "supabase" as string, }, ]; From 433cce01cd0918233f5854c70cd90e8fb3878b64 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Mon, 10 Feb 2025 19:44:01 -0800 Subject: [PATCH 3/3] improve tests --- test/js/bun/s3/s3.test.ts | 107 ++++++++++++++++++++++++++------------ 1 file changed, 75 insertions(+), 32 deletions(-) diff --git a/test/js/bun/s3/s3.test.ts b/test/js/bun/s3/s3.test.ts index a1b3cae764a05c..60492b810f9ce1 100644 --- a/test/js/bun/s3/s3.test.ts +++ b/test/js/bun/s3/s3.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeAll, afterAll } from "bun:test"; +import { describe, expect, it, beforeEach, afterEach } from "bun:test"; import { bunExe, bunEnv, getSecret, tempDirWithFiles, isLinux } from "harness"; import { randomUUID } from "crypto"; import { S3Client, s3 as defaultS3, file, which } from "bun"; @@ -115,7 +115,9 @@ for (let credentials of allCredentials) { describe(bucketInName ? "bucket in path" : "bucket in options", () => { var tmp_filename: string; const options = bucketInName ? s3Options : { ...s3Options, bucket: S3Bucket }; - beforeAll(async () => { + beforeEach(async () => { + // await a little bit so we dont change the filename before deleting it + await Bun.sleep(10); tmp_filename = bucketInName ? `s3://${S3Bucket}/${randomUUID()}` : `s3://${randomUUID()}`; const result = await fetch(tmp_filename, { method: "PUT", @@ -123,14 +125,21 @@ for (let credentials of allCredentials) { s3: options, }); expect(result.status).toBe(200); + await result.text(); }); - afterAll(async () => { - const result = await fetch(tmp_filename, { - method: "DELETE", - s3: options, - }); - expect(result.status).toBe(204); + afterEach(async () => { + try { + const result = await fetch(tmp_filename, { + method: "DELETE", + s3: options, + }); + expect([204, 200]).toContain(result.status); + await result.text(); + } catch (e) { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + } }); it("should download file via fetch GET", async () => { @@ -216,22 +225,30 @@ for (let credentials of allCredentials) { describe("Bun.S3Client", () => { describe(bucketInName ? "bucket in path" : "bucket in options", () => { - const tmp_filename = bucketInName ? `${S3Bucket}/${randomUUID()}` : `${randomUUID()}`; + let tmp_filename: string; const options = bucketInName ? null : { bucket: S3Bucket }; var bucket = S3(s3Options); - beforeAll(async () => { + beforeEach(async () => { + await Bun.sleep(10); + tmp_filename = bucketInName ? `${S3Bucket}/${randomUUID()}` : `${randomUUID()}`; const file = bucket.file(tmp_filename, options); await file.write("Hello Bun!"); }); - afterAll(async () => { - const file = bucket.file(tmp_filename, options); - await file.unlink(); + afterEach(async () => { + try { + const file = bucket.file(tmp_filename, options); + await file.unlink(); + } catch (e) { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + } }); it("should download file via Bun.s3().text()", async () => { const file = bucket.file(tmp_filename, options); + await file.write("Hello Bun!"); const text = await file.text(); expect(text).toBe("Hello Bun!"); }); @@ -365,16 +382,23 @@ for (let credentials of allCredentials) { describe("Bun.file", () => { describe(bucketInName ? "bucket in path" : "bucket in options", () => { - const tmp_filename = bucketInName ? `s3://${S3Bucket}/${randomUUID()}` : `s3://${randomUUID()}`; + let tmp_filename: string; const options = bucketInName ? s3Options : { ...s3Options, bucket: S3Bucket }; - beforeAll(async () => { + beforeEach(async () => { + await Bun.sleep(10); + tmp_filename = bucketInName ? `s3://${S3Bucket}/${randomUUID()}` : `s3://${randomUUID()}`; const s3file = file(tmp_filename, options); await s3file.write("Hello Bun!"); }); - afterAll(async () => { - const s3file = file(tmp_filename, options); - await s3file.unlink(); + afterEach(async () => { + try { + const s3file = file(tmp_filename, options); + await s3file.unlink(); + } catch (e) { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + } }); it("should download file via Bun.file().text()", async () => { @@ -458,16 +482,23 @@ for (let credentials of allCredentials) { describe("Bun.s3", () => { describe(bucketInName ? "bucket in path" : "bucket in options", () => { - const tmp_filename = bucketInName ? `${S3Bucket}/${randomUUID()}` : `${randomUUID()}`; + let tmp_filename: string; const options = bucketInName ? s3Options : { ...s3Options, bucket: S3Bucket }; - beforeAll(async () => { + beforeEach(async () => { + await Bun.sleep(10); + tmp_filename = bucketInName ? `${S3Bucket}/${randomUUID()}` : `${randomUUID()}`; const s3file = s3(tmp_filename, options); await s3file.write("Hello Bun!"); }); - afterAll(async () => { - const s3file = s3(tmp_filename, options); - await s3file.unlink(); + afterEach(async () => { + try { + const s3file = s3(tmp_filename, options); + await s3file.unlink(); + } catch (e) { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + } }); it("should download file via Bun.s3().text()", async () => { @@ -561,10 +592,20 @@ for (let credentials of allCredentials) { }, 10_000); describe("readable stream", () => { - afterAll(async () => { + afterEach(async () => { await Promise.all([ - s3(tmp_filename + "-readable-stream", options).unlink(), - s3(tmp_filename + "-readable-stream-big", options).unlink(), + s3(tmp_filename + "-readable-stream", options) + .unlink() + .catch(e => { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + }), + s3(tmp_filename + "-readable-stream-big", options) + .unlink() + .catch(e => { + // if error with NoSuchKey, it means the file does not exist and its fine + expect(e?.code || e).toBe("NoSuchKey"); + }), ]); }); it("should work with small files", async () => { @@ -584,7 +625,6 @@ for (let credentials of allCredentials) { } expect(bytes).toBe(10); expect(Buffer.concat(chunks)).toEqual(Buffer.from("Hello Bun!")); - s3file.delete(); }); it("should work with large files ", async () => { const s3file = s3(tmp_filename + "-readable-stream-big", options); @@ -610,14 +650,14 @@ for (let credentials of allCredentials) { const SHA1_2 = Bun.SHA1.hash(bigishPayload, "hex"); expect(SHA1).toBe(SHA1_2); } - s3file.delete(); }, 30_000); }); }); }); } describe("special characters", () => { - it("should allow special characters in the path", async () => { + // supabase will throw InvalidKey + it.skipIf(credentials.service === "supabase")("should allow special characters in the path", async () => { const options = { ...s3Options, bucket: S3Bucket }; const s3file = s3(`🌈🦄${randomUUID()}.txt`, options); await s3file.write("Hello Bun!"); @@ -724,7 +764,10 @@ for (let credentials of allCredentials) { const dir = tempDirWithFiles("fsr", { "hello.txt": "", }); - await Bun.write(s3("test.txt", { ...s3Options, bucket: S3Bucket }), file(path.join(dir, "hello.txt"))); + const tmp_filename = `${randomUUID()}.txt`; + + await Bun.write(s3(tmp_filename, { ...s3Options, bucket: S3Bucket }), file(path.join(dir, "hello.txt"))); + await s3(tmp_filename, { ...s3Options, bucket: S3Bucket }).unlink(); }); it("Bun.write(s3file, file) should throw if the file does not exist", async () => { try { @@ -747,7 +790,7 @@ for (let credentials of allCredentials) { ); expect.unreachable(); } catch (e: any) { - expect(["AccessDenied", "NoSuchBucket"]).toContain(e?.code); + expect(["AccessDenied", "NoSuchBucket", "NoSuchKey"]).toContain(e?.code); expect(e?.path).toBe("do-not-exist.txt"); expect(e?.name).toBe("S3Error"); } @@ -949,7 +992,7 @@ for (let credentials of allCredentials) { await Promise.all( [s3, (path, ...args) => S3(...args).file(path)].map(async fn => { let options = { ...s3Options, bucket: S3Bucket }; - options.endpoint = Buffer.alloc(1024, "a").toString(); + options.endpoint = Buffer.alloc(2048, "a").toString(); try { const s3file = fn(randomUUID(), options);