[robobun]

Stacked on #44287. Do not merge before #43140.

Problem

• S3File.writer() never sends x-amz-acl and, without arguments, no x-amz-storage-class. write() sends both. The object gets the default ACL and class, or a bucket policy that requires the ACL refuses the upload.
• writable_stream (src/runtime/webcore/s3/client.rs:410) has no acl parameter. get_writer without an options object (src/runtime/webcore/Blob.rs:1651) passes None for the storage class.

Fix

• writable_stream takes acl and gives it to the MultiPartUpload, as upload_stream does.
• get_writer passes the acl and storage_class of the store, or the parsed acl when there is an options object.
• Correct because MultiPartUpload already sends both fields on PutObject and on CreateMultipartUpload.
• Verified: test/js/bun/s3/s3-requester-pays.test.ts (35 new cases, 31 fail on the base). Self-reviewed: 29 concerns raised, 28 addressed (Notes).

Background

• An S3 file keeps acl and storage_class on its store (store::S3, src/jsc/webcore_types.rs:828).
• writer() reports an upload failure through a pending flush() or end(). A failure with no pending promise is lost (S3File.writer() drops an upload failure that arrives with no pending flush() or end() promise: end() resolves 0 #43133). s3: report a writer() upload failure that no pending promise could carry #43140 reports it.
• Considered one PR with s3: honor partSize, queueSize, retry, storageClass and acl in writer() #44287. This half adds headers that a store can refuse, so it waits for s3: report a writer() upload failure that no pending promise could carry #43140.

Downsides

• A writer() with an ACL or storage class that the store refuses now fails, as write() does (for example 400 AccessControlListNotSupported). Before, the object was stored without them.
• Until s3: report a writer() upload failure that no pending promise could carry #43140 merges, a refusal that arrives between writes of a multipart body is lost: end() resolves 0, nothing is stored. Main stores the object.
• Callers who set no option: 0 new allocations or calls, +7 instructions per writer() (996, base 989), .text +0 B.

Notes

Why this is a draft. With this change a store can refuse a writer() upload that it accepted before, because the upload now carries the header. writer() loses a failure that arrives when no flush() or end() is pending (#43133). The loop in docs/guides/write-file/stream.mdx (for await (const chunk of stream) writer.write(chunk), then end()) is that shape. #43140 reports the failure. It does not compile on main today: it calls NetworkSink::path(), which #41688 removed. When #43140 is on main I add the case below as a test and mark this PR ready.

A bucket that has ACLs disabled (BucketOwnerEnforced in test/packages/s3-server), a writer with acl: "public-read" and retry: 0:

Case	base	PR
1 byte	end() resolves 1, stored, no header sent	end() rejects AccessControlListNotSupported, nothing stored, 1 request
5 MiB + 1, end() at once	resolves 5242881, stored	rejects AccessControlListNotSupported, nothing stored, 1 request
5 MiB + 1, await flush()	resolves, stored	flush() rejects AccessControlListNotSupported, nothing stored
5 MiB + 1, end() after the answer to CreateMultipartUpload	resolves 5242881, stored	resolves 0, nothing stored, 1 request
acl: "private" or "bucket-owner-full-control", 1 byte	stored, no header sent	stored, header sent
write(data) with the same options	rejects AccessControlListNotSupported	the same

The fourth row is the one that #43140 changes. The same loss occurs on main with no ACL: a writer with a wrong secretAccessKey and 5 MiB + 1 resolves 0 and stores nothing. A bucket with BlockPublicAcls answers 403 AccessDenied in place of the 400.

A bucket policy that requires the ACL (Deny s3:PutObject unless s3:x-amz-acl is bucket-owner-full-control), 1 byte:

Case	base	PR
file(key, { acl }).writer() and writer({ acl })	end() rejects AccessDenied after 4 PutObject	stored, 1 PutObject
file(key, { acl }).write(data)	stored	stored

Measurements. Release builds from one toolchain: #44287 (the base of this PR) and this branch. The counts are from gdb, inside the dynamic extent of get_writer, on the thread that entered it.

What	base	PR
writer() without options: allocations / frees / parser calls / credential copies (mode, 191 of 200 calls)	4 / 0 / 0 / 0	4 / 0 / 0 / 0
writer({}) and writer({ acl }): allocations / frees / parser calls / clone / dupe (mode, 191 of 200 calls)	35 / 25 / 1 / 1 / 1	35 / 25 / 1 / 1 / 1
Property reads on the options object, writer(bag) / writer()	20 / 0	20 / 0
Instructions per writer() without options (gdb stepi, 10th call, JIT off, 12 runs)	989 (6 runs), 1001 (6 runs)	996 (8 runs), 1008 (4 runs)
.text of bun (llvm-size -A)	58143477 B	58143477 B
get_writer / writable_stream (nm -S)	4044 B / 1046 B	4085 B / 1049 B
Requests of a writer with no option, 1 byte and 5 MiB + 1		0 lines differ

The writer(bag) row is from the merge base of #44287, which has the same code for that branch. valgrind and bloaty are not installed in the container, and apt reaches no mirror. The instruction count is from gdb for that reason.

The test.

• The header cases run for 5 ways to make the file and 4 forms of the call, and for writer(options). Each case compares x-amz-acl, x-amz-storage-class and x-amz-request-payer. The test server refuses a request with an x-amz-* header that is not signed, so the cases do not compare SignedHeaders.
• The flush() case calls end() after flush(). Without it nothing holds the writer, the garbage collector takes it, and the upload fails with UnknownError.
• Release build of the base: 39 pass, 31 fail. Release and debug build of this branch: 70 pass. Debug build with BUN_JSC_validateExceptionChecks=1, BUN_DESTRUCT_VM_ON_EXIT=1 and LeakSanitizer: 70 pass, 0 leak records.
• Also on the debug build: s3.test.ts (307 pass, 270 skipped without R2 credentials), the other files of test/js/bun/s3/, test/regression/issue/s3-header-injection.test.ts. I ran them with --timeout 120000 because of the load of the host.

Self-review. Rejected: expected-failure rows for the sibling sites (code this PR does not change). Not carried from the review: a list of S3-compatible services that refuse one of the two headers. I had no way to check it from this environment.

Not in this PR. #44289 has the first two, with output.

• contentDisposition and contentEncoding given to file() or to the client reach no upload, from write() or from writer(). The store has no field for them.
• presign() without options does not use the ACL and the storage class of the file. A comment at S3File.rs:563 says that this is deliberate.
• writer(bag) reads type, contentDisposition and contentEncoding two times. To remove the first read changes the text of an error.
• options, acl, storage_class and request_payer go to upload, writable_stream and upload_stream as separate positional arguments. One struct for them is a possible follow-up.

Open PRs near these lines. #40252 moves the body of writable_stream into new_upload(..., acl, ...) and passes None from the writer. After this PR that call passes acl. #43045 changes the signed headers, which these tests do not compare.

Earlier work. There is no user report for writer(). #33502 made this change inside a larger one. It went stale and got no review.

Question. get_writer keeps its two calls of writable_stream. One call behind the test for the options object is possible at the same cost per call. I kept the two calls because the diff is 7 lines. Say so if you prefer the one call.

[robobun]

Status: draft. It waits for #43140.

How I reproduced it: client.file(key, { acl: "public-read", storageClass: "STANDARD_IA" }).writer() with one byte written, against the S3 server of test/packages/s3-server. On #44287 the PutObject request has no x-amz-acl and no x-amz-storage-class. This branch sends both. bun bd test test/js/bun/s3/s3-requester-pays.test.ts fails 31 cases on #44287 and passes on this branch.

[robobun]

Closing: #44287 now has this change. It is one pull request for the writer options and for write() from a file.