ReviewOS

home-lang/home

Native S3: replace remaining facade stubs and complete proxy resolution

#597
Open chris opened this 9 days ago ·
9 days ago

Scope

Replace the remaining Phase-12 S3 facade placeholders with the real runtime implementations and validate the corresponding untouched Bun corpus.

Known gaps

  • S3.download delegates to runtime/webcore/s3/client.zig instead of returning without invoking its callback.
  • S3.downloadSlice delegates to the real ranged-download implementation.
  • S3.delete delegates to the real delete implementation.
  • S3.listObjects and getListObjectsOptionsFromJS delegate to the real list parser/client.
  • throwSignError and getJSSignError preserve Bun structured signing errors instead of returning generic/empty values.
  • Resolve HTTP(S)_PROXY and NO_PROXY from each signed URL for simple requests, list operations, download streams, and multipart requests; preserve true explicit proxy overrides.
  • Activate the applicable deterministic S3 corpus files through the production VM with no runtime source rewrites, extra skips, or weakened assertions.
  • Verify supported macOS, Linux, and Windows behavior.

Context

Commit f813c1d2b7e2d6fa0105a707726739f5ecd35b70 restored the S3 stat path and both central S3 task-dispatch bridges while completing the exact native HTTP proxy corpus on macOS. Commit 7542a8e1834d2e1c047a3244949094f5eb9ddf3b restored the remaining S3 facade delegates and structured errors. Commit 0e0af17efbf381b3f8ad51c701fcb70d94b44479 moved proxy resolution to each signed request URL across simple, list, streaming-download, and multipart paths. Commit 5c2ac45420be0462c315e6c31bfe6ba7bbd8817e completed request-body ownership, early-response HTTP/1.1 reuse safety, bounded malformed ListObjects parsing, and native production routing for eight deterministic S3 corpus files.

The credential-dependent umbrella and leak suites remain outside the deterministic native routing allowlist until their external S3/R2 environment can run the assertions rather than only register upstream skips.

Parent parity tracker: #66 Suppression ledger: #202 TLS proxy batch: #596

9 days ago

Facade milestone pushed in 7542a8e1834d2e1c047a3244949094f5eb9ddf3b (fix(runtime): restore S3 client facade).

Completed in this batch:

  • S3.download and S3.downloadSlice now invoke the real client and settle their callbacks.
  • S3.delete now invokes the real client.
  • S3.getListObjectsOptionsFromJS and S3.listObjects now use the actual JS option parser, SigV4 request builder, XML parser, and callback path.
  • throwSignError and getJSSignError now use error_jsc.zig; fixed the dormant incorrect aliases in client.zig.

Production-only local S3 fixture: 5 passed, 0 failed, 11 assertions, covering full/ranged GET, DELETE, list URL encoding/XML parsing, callback settlement, presign errors, and fetch errors. Stable home_rt: 1,816 passed, 19 skipped, 0 failed. Debug build and formatting/diff checks are green.

Remaining here: finish signed-URL HTTP(S)_PROXY/NO_PROXY resolution across non-stat simple requests, list/download streams, and multipart; then activate suitable untouched S3 corpus coverage and verify supported platforms.

9 days ago

Pushed 0e0af17efbf381b3f8ad51c701fcb70d94b44479 (fix(runtime): resolve S3 proxies per request).

This completes the proxy-resolution implementation item:

  • removed premature scheme-less proxy lookups from every non-explicit S3 call site;
  • simple GET/range/PUT/DELETE, list, streaming download, and multipart requests now resolve HTTP_PROXY versus HTTPS_PROXY only after the signed URL exists;
  • NO_PROXY now matches the signed endpoint hostname/host, including exact host:port;
  • the explicit proxy supplied by fetch("s3://…", { proxy }) remains explicit and is still filtered through NO_PROXY;
  • every task owns the resolved proxy bytes for its full HTTP-thread lifetime.

Validation on macOS with the clean production JSC binary:

  • focused S3 proxy diagnostic: 6 passed, 0 failed, 21 assertions (simple read/range/write/delete, list, streamed download, multipart writer, live proxy replacement/ownership, exact host:port NO_PROXY, explicit fetch proxy, and HTTPS selecting HTTPS_PROXY rather than HTTP_PROXY);
  • home_rt: 1,816 passed, 19 skipped, 0 failed;
  • production debug build, targeted Zig AST/format checks, and git diff --check passed.

The diagnostic also directly confirmed that native fetch and S3 observe the same runtime proxy-env map. The remaining issue scope is corpus activation and Linux/Windows verification.

9 days ago

Pushed 5c2ac45420be0462c315e6c31bfe6ba7bbd8817e (fix(runtime): complete native S3 corpus batch).

This closes the deterministic production-corpus activation item and ports three missing pieces from pinned Bun's active implementation:

  • every simple S3 request owns its upload body for the full asynchronous HTTP-thread lifetime;
  • direct HTTP/1.1 sockets are not pooled after an early response while byte-buffer request data remains unsent, preventing subsequent multipart requests from interleaving with the previous body;
  • the ListObjects XML parser terminates on recognized unclosed tags instead of rescanning the remaining input quadratically.

Eight S3 corpus files now run unchanged through the normal native Home corpus path: fd validation, insecure HTTP endpoints, list-option/key encoding, ListObjects parsing, queue-size validation, requester-pays, storage class, and stream-cancellation GC.

Exact parity against pinned Bun 4982b91e3702094330f3be3883354c52b8c01323: 66 passed, 3 upstream credential-gated skips, 0 failed on both runtimes. The 100 MiB queueSize=10 multipart case is green, and the 5 MB repeated-unclosed-Key regression fell from an unbounded quadratic run to about 129 ms in Home. Stable home_rt: 1,816 passed, 19 skipped, 0 failed. Production build, format checks, diff checks, and the focused routing unit are green.

Remaining scope here is supported-platform verification. The credential-dependent umbrella/leak files are deliberately not claimed as deterministic coverage when this environment can only register their upstream skips.

Sign in to comment on this issue.