feat(s3Client): add support for ListObjectsV2 action #16948
Conversation
|
Hopefully a maintainer can get to this soon. I would love to see this added. |
| @@ -1813,6 +1813,106 @@ declare module "bun" { | |||
| * const url = bucket.presign("file.pdf"); | |||
| * await bucket.unlink("old.txt"); | |||
| */ | |||
| interface S3ListObjectsOptions { | |||
| /** Limits the response to keys that begin with the specified prefix. */ | |||
| Prefix?: string; | |||
There was a problem hiding this comment.
The names of fields in the types should start in lower cased.
| * | ||
| * You can use the request parameters as selection criteria to return a subset of the objects in a bucket. | ||
| */ | ||
| listObjects( |
There was a problem hiding this comment.
just list would be better instead of listObjects
There was a problem hiding this comment.
What about if we want to implement list buckets later ?
| const result = this.signRequest(.{ | ||
| .path = "", | ||
| .method = .GET, | ||
| .search_params = search_params.slice(), |
There was a problem hiding this comment.
search_params is leaking memory here, we need to free it after calling signRequest
| @@ -517,7 +536,7 @@ pub const S3Credentials = struct { | |||
| } | |||
|
|
|||
| // if we allow path.len == 0 it will list the bucket for now we disallow | |||
| if (path.len == 0) return error.InvalidPath; | |||
| if (!isComplexeRequest and path.len == 0) return error.InvalidPath; | |||
There was a problem hiding this comment.
this is not right we cannot allow bucket unless we are listing a bucket you can pass a comptime bool for this case
| } else { | ||
| break :brk_canonical try std.fmt.bufPrint(&tmp_buffer, "{s}\n{s}\n{s}\nhost:{s}\nx-amz-content-sha256:{s}\nx-amz-date:{s}\n\n{s}\n{s}", .{ method_name, normalizedPath, if (search_params) |p| p[1..] else "", host, aws_content_hash, amz_date, signed_headers, aws_content_hash }); | ||
| if (session_token) |token| { | ||
| break :brk_canonical try std.fmt.bufPrint(&tmp_buffer, "{s}\n{s}\n{s}\nhost:{s}\nx-amz-content-sha256:{s}\nx-amz-date:{s}\nx-amz-security-token:{s}\n\n{s}\n{s}", .{ method_name, normalizedPath, if (search_params) |p| p[1..] else "", host, aws_content_hash, amz_date, token, signed_headers, aws_content_hash }); |
There was a problem hiding this comment.
this conditions are getting too big we should break to a slow path when we actualy sort in the right order instead of if/else stuff
There was a problem hiding this comment.
Indeed this is unmaintainable and error prone
I wrote an implementation of the entire SigV4 with dynamic values, need deep testing before merging here
| import { describe, it, expect } from "bun:test"; | ||
| import { randomUUIDv7, S3Client, S3ListObjectsResponse, S3Options } from "bun"; | ||
|
|
||
| const options: S3Options = { |
There was a problem hiding this comment.
we should add tests using the CI credentials take a look on the s3.tests.ts
| @@ -3596,6 +3596,58 @@ pub const Blob = struct { | |||
|
|
|||
| return value; | |||
| } | |||
|
|
|||
| pub fn listObjects(this: *@This(), store: *Store, globalThis: *JSC.JSGlobalObject, listOptions: JSValue, extra_options: ?JSValue) bun.JSError!JSValue { | |||
There was a problem hiding this comment.
this should be in S3File.zig not in blob.zig
What does this PR do?
This PR adds support for S3 ListObjectsV2 action.
Same input/output as
@aws-sdk/s3-client.Closes #16495
May be helpful for #8947
How did you verify your code works?
I wrote automated tests
bun-debug test test/js/bun/s3/s3-list-objects.test.ts)If Zig files changed:
bun-debug test test/js/bun/s3/s3-list-objects.test.ts)If new methods, getters, or setters were added to a publicly exposed class: