fix(core): percent-encode object keys in raw-signed backend URLs (#105)

* fix(core): percent-encode object keys in raw-signed backend URLs

Multipart operations (CreateMultipartUpload, UploadPart, Complete, Abort)
build their backend URL by splicing the decoded object key into a string.
For key characters that url::Url leaves literal in paths but that are
outside the RFC 3986 unreserved set (`=`, `!`, `(`, `)`, `:`, `@`, ...),
the request went out — and was signed — with the literal byte, while
S3/MinIO reconstruct the SigV4 canonical URI by strict-encoding the
decoded path. The signatures never matched, so multipart uploads to
Hive-style partition keys (`country_iso=ETH/...`) failed with 403
SignatureDoesNotMatch at CreateMultipartUpload.

Encode the assembled prefix+key with the SigV4 strict set (unreserved
chars, `/` kept as separator) before splicing. The URL string is both the
signing input and the wire bytes, so the two stay byte-identical on any
backend. Presigned CRUD ops already encode this way via object_store's
STRICT_ENCODE_SET, which is why single PUT/GET on such keys worked.

Also syncs Cargo.lock with the 0.6.3 release version bump.

Reported downstream as source-cooperative/data.source.coop#180.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: state the encode-set rationale timelessly

Comments described the pre-fix state ('already work', 'pre-existing');
reword to describe the invariant instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: alukach

crates/core/src/backend/multipart.rs

@@ -13,6 +13,22 @@ use crate::types::{BucketConfig, S3Operation};
 use http::{HeaderMap, Method};
 use url::Url;
 
+/// SigV4 canonical path encoding: everything but RFC 3986 unreserved
+/// characters is percent-encoded; `/` stays raw as the segment separator.
+///
+/// AWS (and MinIO) reconstruct the canonical URI by strict-encoding the
+/// decoded request path, and `url::Url` leaves `=` and friends literal in
+/// paths — so the key must be encoded with this exact set before it is
+/// spliced into the URL, or the backend signature won't match
+/// (`SignatureDoesNotMatch`). This is the same strict set `object_store`
+/// applies when building presigned URLs for the CRUD operations.
+const S3_PATH_ENCODE_SET: &percent_encoding::AsciiSet = &percent_encoding::NON_ALPHANUMERIC
+    .remove(b'-')
+    .remove(b'.')
+    .remove(b'_')
+    .remove(b'~')
+    .remove(b'/');
+
 /// Build the backend URL for an S3 operation.
 ///
 /// Used for multipart operations that go through raw signed HTTP.
@@ -40,6 +56,7 @@ pub fn build_backend_url(
         key.push('/');
     }
     key.push_str(operation.key());
+    let key = percent_encoding::utf8_percent_encode(&key, S3_PATH_ENCODE_SET).to_string();
 
     let mut url = if bucket_is_empty {
         format!("{}/{}", base, key)
@@ -192,6 +209,67 @@ mod tests {
         );
     }
 
+    #[test]
+    fn key_special_chars_percent_encoded_in_backend_path() {
+        let config = test_bucket_config();
+        let op = S3Operation::CreateMultipartUpload {
+            bucket: "test".into(),
+            key: "by_country/country_iso=ETH/ETH file.pmtiles".into(),
+        };
+
+        let url = build_backend_url(&config, &op).unwrap();
+
+        // `=` and space are outside the AWS strict set and must be encoded;
+        // `/` stays raw as the segment separator.
+        assert_eq!(
+            url,
+            "https://s3.us-east-1.amazonaws.com/my-backend-bucket/by_country/country_iso%3DETH/ETH%20file.pmtiles?uploads"
+        );
+        // Url::parse must not re-encode the result: what sign_request signs
+        // (`url.path()`) is byte-identical to the wire path.
+        assert_eq!(
+            Url::parse(&url).unwrap().path(),
+            "/my-backend-bucket/by_country/country_iso%3DETH/ETH%20file.pmtiles"
+        );
+    }
+
+    #[test]
+    fn key_aws_special_chars_encoded_slash_preserved() {
+        let config = test_bucket_config();
+        let op = S3Operation::UploadPart {
+            bucket: "test".into(),
+            key: "p!('*'):@+,;$&/f.bin".into(),
+            upload_id: "uid".into(),
+            part_number: 1,
+        };
+
+        let url = build_backend_url(&config, &op).unwrap();
+
+        let path = url.split_once('?').unwrap().0;
+        assert_eq!(
+            path,
+            "https://s3.us-east-1.amazonaws.com/my-backend-bucket/p%21%28%27%2A%27%29%3A%40%2B%2C%3B%24%26/f.bin"
+        );
+    }
+
+    #[test]
+    fn key_with_literal_percent_triplet_is_double_encoded() {
+        // A key that literally contains `%3D` (already decoded upstream) must
+        // go out as `%253D`, not be mistaken for an encoded `=`.
+        let config = test_bucket_config();
+        let op = S3Operation::CreateMultipartUpload {
+            bucket: "test".into(),
+            key: "weird/%3D-literal.txt".into(),
+        };
+
+        let url = build_backend_url(&config, &op).unwrap();
+
+        assert_eq!(
+            url,
+            "https://s3.us-east-1.amazonaws.com/my-backend-bucket/weird/%253D-literal.txt?uploads"
+        );
+    }
+
     #[test]
     fn normal_upload_id_works() {
         let config = test_bucket_config();

tests/integration/test_integration.py

@@ -330,6 +330,47 @@ def test_multipart_roundtrip_high_level(self):
 
         client.delete_object(Bucket="private-uploads", Key=key)
 
+    @pytest.mark.parametrize(
+        "key_stem",
+        [
+            # Hive-style partition path — data.source.coop#180.
+            "by_country/country_iso=ETH/ETH",
+            # The other chars the url crate leaves literal in paths but AWS
+            # strict-encodes when reconstructing the canonical URI. Chars in
+            # object_store's PathPart INVALID set (`*`, `%`, `~`, `#`, ...)
+            # are excluded: the presigned CRUD path rewrites those in the
+            # logical key (`*` → `%2A`), so a byte-faithful multipart write
+            # can't be read back through it — a separate limitation of the
+            # presigned path.
+            "specials !('):@+,;$&/part=2",
+        ],
+    )
+    def test_multipart_special_char_key_roundtrip(self, key_stem):
+        """Multipart to a key with chars outside the RFC 3986 unreserved set.
+
+        The raw-signed backend URL must percent-encode these or the backend
+        (AWS/MinIO) re-encodes them server-side and rejects the signature with
+        SignatureDoesNotMatch at CreateMultipartUpload.
+        """
+        client = static_client()
+        key = f"{key_stem}-{uuid.uuid4()}.bin"
+        body = b"special-key-payload!" * (6 * MIB // 20 + 1)
+        body = body[: 6 * MIB]  # 6 MiB → two parts, forces multipart
+        config = TransferConfig(
+            multipart_threshold=5 * MIB,
+            multipart_chunksize=5 * MIB,
+            max_concurrency=1,
+            use_threads=False,
+        )
+
+        client.upload_fileobj(BytesIO(body), "private-uploads", key, Config=config)
+        # Reading back through the presigned GET path proves both paths agree
+        # on what the key is.
+        resp = client.get_object(Bucket="private-uploads", Key=key)
+        assert resp["Body"].read() == body
+
+        client.delete_object(Bucket="private-uploads", Key=key)
+
     def test_multipart_low_level_explicit(self):
         """Drive Create/UploadPart/Complete directly and verify the round-trip."""
         client = static_client()