feat(presign): support bare query params in canonicalization

Author: lcian

clients/python/src/objectstore_client/presign.py

@@ -90,7 +90,7 @@ def _canonical_presigned_request(path: str, query: str) -> str:
     - Method is always ``GET`` (HEAD maps to GET).
     - Path uses the encoded URI path as received by the service.
     - Percent-encoded octets are normalized to uppercase hex digits.
-    - Query params use the encoded key/value pairs from the URI, excluding
+    - Query params use the encoded keys and optional values from the URI, excluding
       ``X-Os-Signature``, sorted by encoded key.
     """
     canonical_path = _normalize_percent_encoding(path)
@@ -99,17 +99,20 @@ def _canonical_presigned_request(path: str, query: str) -> str:
     for pair in query.split("&"):
         if not pair:
             continue
-        if "=" not in pair:
-            continue
-        k, _, v = pair.partition("=")
+        if "=" in pair:
+            k, _, v = pair.partition("=")
+            value = _normalize_percent_encoding(v)
+        else:
+            k = pair
+            value = None
         k = _normalize_percent_encoding(k)
         if k == PARAM_SIGNATURE:
             continue
-        params.append((k, _normalize_percent_encoding(v)))
+        params.append((k, value))
 
     params.sort(key=lambda x: (x[0], x[1]))
 
-    query_str = "&".join(f"{k}={v}" for k, v in params)
+    query_str = "&".join(k if v is None else f"{k}={v}" for k, v in params)
     return f"GET\n{canonical_path}\n{query_str}"
 
 

clients/python/tests/test_presign.py

@@ -75,6 +75,13 @@ def test_reordered_query_params(self) -> None:
         )
         assert c1 == c2
 
+    def test_bare_query_params(self) -> None:
+        canonical = _canonical_presigned_request(
+            "/path",
+            "y&X-Os-KeyId=test&x=1&X-Os-Expires=1000&z",
+        )
+        assert canonical == ("GET\n/path\nX-Os-Expires=1000&X-Os-KeyId=test&x=1&y&z")
+
 
 class TestPresignUrl:
     def test_produces_valid_format(self) -> None:

objectstore-shared/src/presign.rs

@@ -15,30 +15,36 @@ pub const PARAM_SIGNATURE: &str = "X-Os-Signature";
 /// - Method is always `GET` (HEAD maps to GET).
 /// - Path uses the encoded URI path as received by the service.
 /// - Percent-encoded octets are normalized to uppercase hex digits.
-/// - Query params use the encoded key/value pairs from the URI, excluding
-///   `X-Os-Signature`, sorted by encoded key.
+/// - Query params use the encoded keys and optional values from the URI,
+///   excluding `X-Os-Signature`, sorted by encoded key.
 pub fn canonical_presigned_request(path: &str, query: Option<&str>) -> String {
     let canonical_path = normalize_percent_encoding(path);
 
-    let mut params: Vec<(String, String)> = query
+    let mut params: Vec<(String, Option<String>)> = query
         .unwrap_or("")
         .split('&')
         .filter(|s| !s.is_empty())
         .filter_map(|pair| {
-            let (k, v) = pair.split_once('=')?;
+            let (k, v) = match pair.split_once('=') {
+                Some((k, v)) => (k, Some(normalize_percent_encoding(v))),
+                None => (pair, None),
+            };
             let canonical_key = normalize_percent_encoding(k);
             if canonical_key == PARAM_SIGNATURE {
                 return None;
             }
-            Some((canonical_key, normalize_percent_encoding(v)))
+            Some((canonical_key, v))
         })
         .collect();
 
     params.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| a.1.cmp(&b.1)));
 
     let query_str = params
         .iter()
-        .map(|(k, v)| format!("{k}={v}"))
+        .map(|(k, v)| match v {
+            Some(v) => format!("{k}={v}"),
+            None => k.clone(),
+        })
         .collect::<Vec<_>>()
         .join("&");
 
@@ -114,4 +120,14 @@ mod tests {
         let c2 = canonical_presigned_request("/path", Some("X-Os-Expires=1000&X-Os-KeyId=test"));
         assert_eq!(c1, c2);
     }
+
+    #[test]
+    fn test_canonical_form_bare_query_params() {
+        let canonical =
+            canonical_presigned_request("/path", Some("y&X-Os-KeyId=test&x=1&X-Os-Expires=1000&z"));
+        assert_eq!(
+            canonical,
+            "GET\n/path\nX-Os-Expires=1000&X-Os-KeyId=test&x=1&y&z"
+        );
+    }
 }