Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions conf/nginx.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ env BOT_TOKEN;
# Google Cloud Storage S3 compatible bucket prefix.
env AWS_BUCKET_PREFIX;

# S3-compatible endpoint URL, used to detect backend-specific signature quirks.
env ENDPOINT_URL;

# It is recommended to set one worker process per CPU core.
pid /run/nginx.pid;

Expand Down
17 changes: 10 additions & 7 deletions lua/compute_aws_s3_signature.lua
Original file line number Diff line number Diff line change
Expand Up @@ -376,20 +376,23 @@ elseif signature_mode == "PRESIGN_PART" then
end
local expires = ngx.time() + 3600
local aws_secret_key = os.getenv('AWS_SECRET_ACCESS_KEY')
-- GCS normalises ':' to '%3A' when computing StringToSign for presigned URLs;
-- encode it here so the signature matches what GCS will verify.
local url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A')
-- Normalise uploadId: ngx.var.arg_* may be pre-encoded or raw depending on the nginx
-- version; unescape then re-escape to avoid double-encoding (%2B → %252B).
local escaped_upload_id = ngx.escape_uri(ngx.unescape_uri(upload_id))
-- GCS does NOT include ?partNumber=N&uploadId=X in the canonical resource for presigned
-- part PUTs. Standard AWS S3-compatible backends (cloudserver, Scaleway S3) do include
-- them per the V2 spec. Detect GCS from ENDPOINT_URL to pick the right behaviour.
-- GCS has two quirks vs standard S3 V2 for presigned part PUTs:
-- 1. It normalises ':' to '%3A' in the key when verifying StringToSign.
-- 2. It excludes ?partNumber=N&uploadId=X from the canonical resource.
-- Standard S3-compatible backends (cloudserver, Scaleway) use literal ':'
-- and include subresources per the V2 spec.
-- ENDPOINT_URL must be declared with 'env ENDPOINT_URL;' in nginx.conf so
-- os.getenv() can read it from Lua workers.
local endpoint_url = os.getenv('ENDPOINT_URL') or ''
local canonicalized_resource
local url_safe_key, canonicalized_resource
if endpoint_url:find('googleapis', 1, true) then
url_safe_key = ngx.var.encoded_key:gsub(':', '%%3A')
canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key
else
url_safe_key = ngx.var.encoded_key
canonicalized_resource = "/" .. ngx.var.aws_tgt_bucket .. "/" .. url_safe_key ..
"?partNumber=" .. part_number .. "&uploadId=" .. escaped_upload_id
end
Expand Down
20 changes: 0 additions & 20 deletions tests/end2end/test_presign_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,26 +118,6 @@ def test_presign_upload_part_encodes_special_chars_in_upload_id(session, artifac
)


def test_presign_upload_part_encodes_colons_in_url(session, artifacts_url):
"""Presigned part URL path must use %3A for ':' — GCS normalises ':' to '%3A'
when computing StringToSign, so a literal ':' causes SignatureDoesNotMatch."""
upload_id = multipart_initiate(session, artifacts_url, STAGING_BUILD, 'presign/colon-part.bin')
try:
resp = session.get(
f'{artifacts_url}/presign-upload-part/{STAGING_BUILD}/presign/colon-part.bin',
params={'partNumber': 1, 'uploadId': upload_id},
)
assert resp.status_code == 200, f'{resp.status_code} {resp.text}'
url = resp.text.strip()
path = urlparse(url).path
assert '%3A' in path, f"Expected '%3A' in presigned part URL path, got: {path!r}"
assert ':' not in path, f"Raw ':' in presigned part URL path causes SignatureDoesNotMatch on GCS"
finally:
session.delete(
f'{artifacts_url}/upload-multipart/abort/{STAGING_BUILD}/presign/colon-part.bin',
params={'uploadId': upload_id},
)


def test_presign_multipart_full_round_trip(session, artifacts_url):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three blank lines between test_presign_upload_part_encodes_special_chars_in_upload_id and test_presign_multipart_full_round_trip after the test removal — PEP 8 E303 expects two.

Suggested change
def test_presign_multipart_full_round_trip(session, artifacts_url):
def test_presign_multipart_full_round_trip(session, artifacts_url):

— Claude Code

"""Initiate via nginx, upload parts directly to S3, complete via nginx."""
Expand Down
Loading