fix: Fix handling prefix during signing

obstore/src/signer.rs

@@ -80,9 +80,9 @@ impl Signer for SignCapableStore {
         Self: 'async_trait,
     {
         match self {
-            Self::S3(inner) => inner.as_ref().inner().signed_url(method, path, expires_in),
-            Self::Gcs(inner) => inner.as_ref().inner().signed_url(method, path, expires_in),
-            Self::Azure(inner) => inner.as_ref().inner().signed_url(method, path, expires_in),
+            Self::S3(inner) => inner.signed_url(method, path, expires_in),
+            Self::Gcs(inner) => inner.signed_url(method, path, expires_in),
+            Self::Azure(inner) => inner.signed_url(method, path, expires_in),
         }
     }
 
@@ -98,18 +98,9 @@ impl Signer for SignCapableStore {
         Self: 'async_trait,
     {
         match self {
-            Self::S3(inner) => inner
-                .as_ref()
-                .inner()
-                .signed_urls(method, paths, expires_in),
-            Self::Gcs(inner) => inner
-                .as_ref()
-                .inner()
-                .signed_urls(method, paths, expires_in),
-            Self::Azure(inner) => inner
-                .as_ref()
-                .inner()
-                .signed_urls(method, paths, expires_in),
+            Self::S3(inner) => inner.signed_urls(method, paths, expires_in),
+            Self::Gcs(inner) => inner.signed_urls(method, paths, expires_in),
+            Self::Azure(inner) => inner.signed_urls(method, paths, expires_in),
         }
     }
 }

pyo3-object_store/src/prefix.rs

@@ -5,9 +5,15 @@
 
 use bytes::Bytes;
 use futures::{stream::BoxStream, StreamExt, TryStreamExt};
+use http::Method;
+use object_store::signer::Signer;
 use std::borrow::Cow;
+use std::future::Future;
 use std::ops::Range;
+use std::pin::Pin;
 use std::sync::OnceLock;
+use std::time::Duration;
+use url::Url;
 
 use object_store::{path::Path, CopyOptions};
 use object_store::{
@@ -213,3 +219,42 @@ impl<T: ObjectStore> ObjectStore for MaybePrefixedStore<T> {
             .boxed()
     }
 }
+
+impl<T: ObjectStore + Signer> Signer for MaybePrefixedStore<T> {
+    fn signed_url<'life0, 'life1, 'async_trait>(
+        &'life0 self,
+        method: Method,
+        path: &'life1 Path,
+        expires_in: Duration,
+    ) -> Pin<Box<dyn Future<Output = object_store::Result<Url>> + Send + 'async_trait>>
+    where
+        'life0: 'async_trait,
+        'life1: 'async_trait,
+        Self: 'async_trait,
+    {
+        let full = full_path(self.prefix.as_ref(), path).into_owned();
+        Box::pin(async move { self.inner.signed_url(method, &full, expires_in).await })
+    }
+
+    fn signed_urls<'life0, 'life1, 'async_trait>(
+        &'life0 self,
+        method: Method,
+        paths: &'life1 [Path],
+        expires_in: Duration,
+    ) -> Pin<Box<dyn Future<Output = Result<Vec<Url>>> + Send + 'async_trait>>
+    where
+        'life0: 'async_trait,
+        'life1: 'async_trait,
+        Self: 'async_trait,
+    {
+        let full_paths = paths
+            .iter()
+            .map(|path| full_path(self.prefix.as_ref(), path).into_owned())
+            .collect::<Vec<_>>();
+        Box::pin(async move {
+            self.inner
+                .signed_urls(method, &full_paths, expires_in)
+                .await
+        })
+    }
+}

tests/store/test_sign.py

@@ -0,0 +1,99 @@
+from __future__ import annotations
+
+from datetime import timedelta
+from typing import TYPE_CHECKING
+
+import requests
+
+import obstore
+from obstore.store import S3Store
+
+if TYPE_CHECKING:
+    from obstore.store import ClientConfig, S3Config
+
+
+def test_sign_includes_prefix_in_url() -> None:
+    """Signing must apply the store prefix to the path (regression for #682).
+
+    This needs no live server: object_store builds the SigV4 presigned URL
+    locally from the static credentials, so we can just inspect the path.
+    """
+    store = S3Store(
+        "my-bucket",
+        access_key_id="foo",
+        secret_access_key="bar",  # noqa: S106
+        region="us-east-1",
+        prefix="some/prefix",
+    )
+    url = str(obstore.sign(store, "GET", "filename.txt", timedelta(seconds=3600)))
+    assert "/my-bucket/some/prefix/filename.txt?" in url
+
+
+def test_sign_roundtrip_with_prefix(
+    minio_bucket: tuple[S3Config, ClientConfig],
+) -> None:
+    """End-to-end: a presigned GET for a prefixed store actually fetches the object.
+
+    Before the fix, put() wrote to ``some/prefix/filename.txt`` but sign() pointed
+    at ``filename.txt``, so this GET would 404.
+    """
+    s3_config, client_options = minio_bucket
+    store = S3Store(
+        config=s3_config,
+        client_options=client_options,
+        prefix="some/prefix",
+    )
+
+    store.put("filename.txt", b"hello world")
+
+    url = str(obstore.sign(store, "GET", "filename.txt", timedelta(seconds=3600)))
+    assert "/some/prefix/filename.txt?" in url
+
+    resp = requests.get(url, timeout=30)
+    assert resp.status_code == 200, resp.text
+    assert resp.content == b"hello world"
+
+
+def test_sign_batch_includes_prefix_in_urls() -> None:
+    """Signing a list of paths applies the prefix to each (regression for #682).
+
+    Exercises the batch ``signed_urls`` path rather than the single ``signed_url``.
+    """
+    store = S3Store(
+        "my-bucket",
+        access_key_id="foo",
+        secret_access_key="bar",  # noqa: S106
+        region="us-east-1",
+        prefix="some/prefix",
+    )
+    paths = ["one.txt", "nested/two.txt"]
+    urls = obstore.sign(store, "GET", paths, timedelta(seconds=3600))
+    assert isinstance(urls, list)
+    assert len(urls) == len(paths)
+    assert "/my-bucket/some/prefix/one.txt?" in str(urls[0])
+    assert "/my-bucket/some/prefix/nested/two.txt?" in str(urls[1])
+
+
+def test_sign_batch_roundtrip_with_prefix(
+    minio_bucket: tuple[S3Config, ClientConfig],
+) -> None:
+    """End-to-end batch: presigned GETs for a prefixed store fetch each object."""
+    s3_config, client_options = minio_bucket
+    store = S3Store(
+        config=s3_config,
+        client_options=client_options,
+        prefix="some/prefix",
+    )
+
+    store.put("one.txt", b"first")
+    store.put("nested/two.txt", b"second")
+
+    paths = ["one.txt", "nested/two.txt"]
+    urls = obstore.sign(store, "GET", paths, timedelta(seconds=3600))
+    assert "/some/prefix/one.txt?" in str(urls[0])
+    assert "/some/prefix/nested/two.txt?" in str(urls[1])
+
+    for url, expected in zip([str(u) for u in urls], [b"first", b"second"]):
+        resp = requests.get(url, timeout=30)
+        assert resp.status_code == 200, resp.text
+        assert resp.content == expected