feat: presigned URLs with extra query params and signed headers (SignedUrlOptions)#771
Open
zfarrell wants to merge 6 commits into
Open
feat: presigned URLs with extra query params and signed headers (SignedUrlOptions)#771zfarrell wants to merge 6 commits into
zfarrell wants to merge 6 commits into
Conversation
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
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.
Which issue does this PR close?
Closes #271
Closes #270
Also related to #620 (versionId) and #318.
Rationale for this change
Right now
Signer::signed_urlcan only presign a bare URL. There's no way to add extra query parameters or to sign request headers, and S3 needs both.The big one is multipart uploads. To presign an
UploadPartrequest you have to includepartNumberanduploadIdas query parameters, and they have to be part of the signature. There's no way to do that today, so you can't hand out presigned URLs for the parts of a multipart upload.Signing headers matters for the same reason. If you want S3 to enforce a checksum, or pin the
Content-Type, or require SSE, those headers have to be baked into the signature when you presign. Otherwise the client can send whatever it wants.What changes are included in this PR?
SignedUrlOptionsand a newSigner::signed_url_optsmethod that takes extra query params and signed(name, value)header pairs along with the usual method/path/expiry.signed_url_optshas a default impl, and the existingsigned_urlnow just calls it with no options. So nothing changes for existing callers and Azure doesn't have to implement anything.+instead of%20, which doesn't match the canonical query, and the canonical query now sorts by the encoded(key, value)pair the way the spec wants.Are there any user-facing changes?
Yes, but only additions.
SignedUrlOptionsandsigned_url_optsare new.signed_urlbehaves exactly as before. No breaking changes — the trait method is defaulted, and the internal signing methods that got reworked were allpub(crate).This was tested end-to-end against real S3 (and MinIO): a full multipart round-trip with a sub-5MiB final part, checksum enforcement (accepts a matching body, rejects a tampered one, rejects a missing one),
Content-Typebinding, tampering withpartNumber/uploadId/signed headers all getting a 403, percent-encoded keys, expiry, andIf-None-Matchconditional create.