fix: route V1 ListObjects to the list handler instead of raw-forwarding

s3proxy/routing/dispatcher.py

@@ -19,6 +19,23 @@
 QUERY_DELETE = "delete"
 QUERY_TAGGING = "tagging"
 
+# Query keys that identify a ListObjects request (V1 or V2) rather than a bucket
+# sub-resource (acl, cors, versioning, ...). A GET whose query is only these is a
+# listing and must go through the list handler, not raw forward_request.
+LIST_QUERY_KEYS = frozenset(
+    {
+        "list-type",
+        "prefix",
+        "delimiter",
+        "marker",
+        "max-keys",
+        "encoding-type",
+        "fetch-owner",
+        "start-after",
+        "continuation-token",
+    }
+)
+
 # Header constants
 HEADER_COPY_SOURCE = "x-amz-copy-source"
 
@@ -138,6 +155,14 @@ async def _dispatch_bucket(
 
         skip_queries = (QUERY_LIST_TYPE, QUERY_DELETE, QUERY_UPLOADS, QUERY_LOCATION)
         if query and not any(q in query for q in skip_queries):
+            # A GET whose query is only listing params is ListObjects V1 (it lacks
+            # list-type=2), not a bucket sub-resource. Fall through to the list
+            # handler, which serves it via the backend's V2 API — raw-forwarding it
+            # sends V1 to the backend, which Hetzner & co. reject with HTTP 400.
+            if method == METHOD_GET and set(parse_qs(query, keep_blank_values=True)) <= (
+                LIST_QUERY_KEYS
+            ):
+                return None
             return await self.handler.forward_request(request, creds)
 
         if not query:

tests/unit/test_dispatch_v1_list.py

@@ -0,0 +1,80 @@
+"""Self-check: a V1 ListObjects GET routes to the list handler, not forward_request.
+
+V1 ListObjects is `GET /bucket?prefix&delimiter&max-keys&encoding-type` with no
+`list-type=2`. _dispatch_bucket used to raw-forward any bucket GET whose query
+lacked list-type/delete/uploads/location, so V1 lists were sent verbatim to the
+backend — which Hetzner rejects with HTTP 400. They must instead reach
+handle_list_objects_v1 (which serves them via the backend's V2 API).
+"""
+
+import asyncio
+
+from s3proxy.routing.dispatcher import RequestDispatcher
+
+
+class _URL:
+    def __init__(self, path, query):
+        self.path = path
+        self.query = query
+
+
+class _Req:
+    def __init__(self, method, path, query):
+        self.method = method
+        self.url = _URL(path, query)
+        self.headers = {}
+
+
+def _record(name):
+    async def f(self, request, creds):
+        self.called = name
+        return name
+
+    return f
+
+
+class _RecordingHandler:
+    def __init__(self):
+        self.called = None
+
+    handle_list_objects_v1 = _record("v1")
+    handle_list_objects = _record("v2")
+    forward_request = _record("forward")
+    handle_get_bucket_location = _record("location")
+    handle_list_buckets = _record("list_buckets")
+
+
+def _route(method, path, query):
+    handler = _RecordingHandler()
+    asyncio.run(RequestDispatcher(handler).dispatch(_Req(method, path, query), creds=None))
+    return handler.called
+
+
+def test_v1_list_with_params_routes_to_v1_handler():
+    # exactly what scylla-manager's rclone / barman-cloud-backup-delete send
+    assert (
+        _route("GET", "/bkt", "prefix=x%2F&delimiter=%2F&max-keys=1000&encoding-type=url") == "v1"
+    )
+    assert _route("GET", "/bkt", "prefix=x&marker=y") == "v1"
+
+
+def test_bare_bucket_get_is_v1_list():
+    assert _route("GET", "/bkt", "") == "v1"
+
+
+def test_v2_list_still_routes_to_v2():
+    assert _route("GET", "/bkt", "list-type=2&prefix=x") == "v2"
+
+
+def test_subresource_get_still_forwards():
+    assert _route("GET", "/bkt", "acl") == "forward"
+    assert _route("GET", "/bkt", "versioning") == "forward"
+    assert _route("GET", "/bkt", "location") == "location"
+
+
+if __name__ == "__main__":
+    test_v1_list_with_params_routes_to_v1_handler()
+    test_bare_bucket_get_is_v1_list()
+    test_v2_list_still_routes_to_v2()
+    test_subresource_get_still_forwards()
+    print("ok")