Restore Content-Type emission for S3 SigV4 requests under EMSCRIPTEN#352
Open
konstantinoscs wants to merge 1 commit into
Open
Restore Content-Type emission for S3 SigV4 requests under EMSCRIPTEN#352konstantinoscs wants to merge 1 commit into
konstantinoscs wants to merge 1 commit into
Conversation
On Emscripten, S3 PUT/POST sign `content-type: application/octet-stream` into the SigV4 canonical request and SignedHeaders, but DuckDB-Wasm's XHR transport does not send a Content-Type for a Uint8Array body. S3 recomputes a different signature and rejects with `403 SignatureDoesNotMatch`. Reads (GET/HEAD, no body) are unaffected. Native curl/httplib re-add the default Content-Type, so signed == sent there. Emit the signed Content-Type into the header map under EMSCRIPTEN so the wire value matches what was signed -- restoring the behavior of duckdb#105, which was regressed by the `!= "application/octet-stream"` guard introduced in duckdb#234. Native behavior is unchanged.
konstantinoscs
force-pushed
the
fix-wasm-s3-put
branch
from
49102d0 to
037aeb0
Compare
Author
|
@carlopi I see that historically you are active on this area. Can you take a look? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #351.
Summary
Restore
Content-Typeheader emission for S3 SigV4 requests underEMSCRIPTEN.CreateS3Headersignscontent-typewhen a content type is provided, including theapplication/octet-streamused by S3PUT. After #234, that value is intentionally not emitted into the header map so native curl can keep its existing default/override behavior. In DuckDB-Wasm, however, the header map is whatHTTPWasmClientforwards toXMLHttpRequest, so the browser request can be missing a header that was included inSignedHeaders, producing AWS S3403 SignatureDoesNotMatch. Only writes are affected —GET/HEADpass an empty content type, so it is neither signed nor sent, which is why reads work and writes fail.This change keeps native behavior unchanged and restores the previous WASM-specific emission from #105.
Testing
#elsebranch is byte-identical to the current code, so the existing MinIO S3 suite is unaffected (regression guard).httpfsforwasm_ehagainst DuckDBv1.5.4(emsdk3.1.71) and loaded it into a DuckDB-Wasm app viaINSTALL httpfs FROM '<local-repo>'withSET allow_unsigned_extensions=true. The fix lives ins3fs.cpp, so it applies identically to themvp/threadsvariants.INSERTfailed with403 SignatureDoesNotMatchon the data-filePUT.INSERTsucceeded against real S3 — thePUTreturned200. In DevTools the request now carriesContent-Type: application/octet-stream, matching thecontent-typealready listed inAuthorization'sSignedHeaders=...(i.e. signed == sent).PUTpath:COPY (SELECT 1) TO 's3://<bucket>/<prefix>/probe.parquet' (FORMAT parquet).From a web app I'm building that uses duckdb WASM to attach to a Polaris backed Catalog:
Notes
The
duckdb-httpfsnative MinIO CI is not expected to exercise this branch because the behavior is guarded byEMSCRIPTEN. There is also currently no CI harness that runs SQL through the WASM HTTP transport, so this path was validated manually in a browser against real S3 (above) rather than by an automated test.