Merge pull request #219 from scality/fix/gcs-presign-colon-encoding

Fix ENDPOINT_URL not exposed to Lua workers in nginx

Author: matthiasL-scality

conf/nginx.conf.template

@@ -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;
 

lua/compute_aws_s3_signature.lua

@@ -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

tests/end2end/test_presign_upload.py

@@ -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):
     """Initiate via nginx, upload parts directly to S3, complete via nginx."""