diff --git a/lua/compute_aws_s3_signature.lua b/lua/compute_aws_s3_signature.lua index 9d63515..d4603e3 100644 --- a/lua/compute_aws_s3_signature.lua +++ b/lua/compute_aws_s3_signature.lua @@ -373,14 +373,19 @@ elseif signature_mode == "PRESIGN_PART" then end local expires = ngx.time() + 3600 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 + -- ngx.var.arg_* may return a raw (already percent-encoded) or decoded value depending on the nginx + -- version. Normalize by unescaping first, then re-escaping to avoid double-encoding (%2B → %252B). + local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id)) -- subresources must appear in canonical resource (alphabetical: partNumber < uploadId) local canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. ngx.var.encoded_key .. - "?partNumber=" .. part_number .. "&uploadId=" .. upload_id + "?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id local string_to_sign = "PUT\n\n\n" .. expires .. "\n" .. canonicalized_resource local aws_signature = ngx.encode_base64(ngx.hmac_sha1(aws_secret_key, string_to_sign)) local presigned_url = ngx.var.redirect_endpoint .. "/" .. ngx.var.aws_tgt_bucket .. "/" .. ngx.var.encoded_key .. "?partNumber=" .. part_number .. - "&uploadId=" .. ngx.escape_uri(upload_id) .. + "&uploadId=" .. escaped_upload_id .. "&AWSAccessKeyId=" .. ngx.escape_uri(ngx.var.aws_access_key) .. "&Expires=" .. expires .. "&Signature=" .. ngx.escape_uri(aws_signature) diff --git a/tests/end2end/test_presign_upload.py b/tests/end2end/test_presign_upload.py index 16d437d..242c066 100644 --- a/tests/end2end/test_presign_upload.py +++ b/tests/end2end/test_presign_upload.py @@ -92,6 +92,26 @@ def test_presign_upload_part_returns_url(session, artifacts_url): ) +def test_presign_upload_part_encodes_special_chars_in_upload_id(session, artifacts_url): + """uploadId containing +, / and = (GCS-style base64) is percent-encoded in the presigned URL.""" + # Use a synthetic uploadId with base64 special characters. The PRESIGN_PART + # endpoint does not validate that the uploadId corresponds to a real upload, + # so we can inject one directly to verify the encoding behaviour without + # needing a backend that generates such IDs. + special_upload_id = 'abc+def/ghi=jkl' + resp = session.get( + f'{artifacts_url}/presign-upload-part/{STAGING_BUILD}/presign/encoding-test.bin', + params={'partNumber': 1, 'uploadId': special_upload_id}, + ) + assert resp.status_code == 200, f'{resp.status_code} {resp.text}' + url = resp.text.strip() + # The uploadId must appear percent-encoded in the presigned URL so that GCS + # can reconstruct the canonical resource from the URL literally (GCS V2 spec). + assert 'uploadId=abc%2Bdef%2Fghi%3Djkl' in url, ( + f'Expected uploadId to be percent-encoded in presigned URL, got: {url!r}' + ) + + def test_presign_multipart_full_round_trip(session, artifacts_url): """Initiate via nginx, upload parts directly to S3, complete via nginx.""" build = STAGING_BUILD