fix: URL-encode uploadId in canonical resource for GCS V2 presigned part URLs#217
Merged
Merged
Conversation
…art URLs GCS V2 signing requires the canonical resource to include URL-encoded values as they appear in the URL (unlike AWS V2 which uses raw/decoded values). Without this, uploadIds containing characters like +, / or = produce a signature mismatch (403 SignatureDoesNotMatch) when GCS verifies the request. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| local aws_secret_key = os.getenv('AWS_SECRET_ACCESS_KEY') | ||
| -- GCS V2 requires URL-encoded values in the canonical resource (unlike AWS V2 which uses raw values). | ||
| -- See: https://cloud.google.com/storage/docs/access-control/signed-urls-v2 | ||
| local escaped_upload_id = ngx.escape_uri(upload_id) |
There was a problem hiding this comment.
Potential double-encoding risk: ngx.var.arg_uploadId returns the raw query-string value without percent-decoding (standard nginx $arg_* behavior). If the client percent-encodes the upload ID (e.g. Python requests with params= turns + into %2B), then ngx.escape_uri() double-encodes it (%2B → %252B). The signature will be internally consistent (canonical resource matches URL), but GCS will URL-decode the presigned URL once and get %2B instead of +, failing to match the actual multipart upload.
Normalizing first avoids this regardless of how the client encodes:
```suggestion
local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id))
Review by Claude Code |
ngx.var.arg_* may return a raw (already percent-encoded) or decoded value depending on the nginx version. Without normalization, calling ngx.escape_uri() on an already-encoded value produces double-encoding (%2B → %252B), which GCS would decode one level and fail to match the multipart upload ID. Use ngx.escape_uri(ngx.unescape_uri(upload_id)) to normalize to a consistent single-encoded form regardless of how nginx provides the arg value. Also adds a test that directly injects a base64-style uploadId (containing +, / and =) to verify the percent-encoding in the returned presigned URL, covering the GCS ID format that Cloudserver does not naturally produce. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
LGTM |
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.
Summary
+,/or=(common in GCS base64-encoded IDs) produce aSignatureDoesNotMatch(403) when GCS verifies the presigned part PUTngx.escape_uri(upload_id)in the canonical resource forPRESIGN_PARTmode, consistent with the value already used in the presigned URL itselfReference: GCS V2 Signing Process — "copy the HTTP request path literally: that is, you should include all URL encoding (percent signs) in the string that you create."
Root cause
Observed in production on
scality/scalityosCI — multipart uploads of large ISO files (50 parts) failed with:Test plan
test_presign_upload.py— all 7 tests pass (includingtest_presign_multipart_full_round_trip)test_multipart_upload.py— all 5 tests passscalityosCI job🤖 Generated with Claude Code