Fix key encoding and virtual-prefix pagination in file listings

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "x2s3"
-version = "1.5.1"
+version = "1.5.2"
 description = "RESTful web service which makes any storage system X available as an S3-compatible REST API"
 readme = "README.md"
 license = { file = "LICENSE" }

tests/test_file.py

@@ -314,6 +314,26 @@ def test_virtual_prefix_list_within_prefix(app):
             assert root.find('KeyCount').text == '1'
 
 
+def test_virtual_prefix_within_prefix_pagination(app):
+    # The synthetic entry has to obey max-keys and continuation tokens too,
+    # the same way a listing off the filesystem does
+    with TestClient(app) as client:
+        for query in ["prefix=abc&delimiter=/&max-keys=0",
+                      "prefix=abc123/&delimiter=/&max-keys=0",
+                      "prefix=abc&delimiter=/&continuation-token=stale",
+                      "prefix=abc123/&delimiter=/&continuation-token=stale"]:
+            response = client.get(f"/mounted-files?list-type=2&{query}")
+            assert response.status_code == 200
+            root = parse_xml(response.text)
+            assert root.findall('CommonPrefixes') == [], query
+            assert root.find('KeyCount').text == '0', query
+            assert root.find('IsTruncated').text == 'false', query
+
+        # Same request without either returns the entry
+        root = parse_xml(client.get("/mounted-files?list-type=2&prefix=abc&delimiter=/").text)
+        assert [cp.find('Prefix').text for cp in root.findall('CommonPrefixes')] == ['abc123/']
+
+
 def test_virtual_prefix_list_no_match(app):
     # Anything outside the virtual prefix matches nothing
     with TestClient(app) as client:
@@ -384,3 +404,31 @@ def test_list_objects_encoding_type_url(app):
             assert root.find('EncodingType').text == 'url'
             assert f"{prefix}test_file.py" in [c.find('Key').text for c in root.findall('Contents')]
             assert f"{prefix}java/" in [cp.find('Prefix').text for cp in root.findall('CommonPrefixes')]
+
+
+@pytest.fixture
+def spaced_app(tmp_path):
+    """An app over a tree whose names need encoding."""
+    (tmp_path / "sub dir").mkdir()
+    (tmp_path / "my file.txt").write_text("spaced")
+    settings = Settings()
+    settings.base_url = HttpUrl('http://testserver')
+    settings.targets = [
+        Target(name='spaced-files', client='file', options={'path': str(tmp_path)})
+    ]
+    return create_app(settings)
+
+
+def test_list_objects_encodes_spaces(spaced_app):
+    # A space is %20, not '+': a client that reads a key out of a listing and
+    # asks for it back has to reach the same file
+    with TestClient(spaced_app) as client:
+        response = client.get("/spaced-files?list-type=2&delimiter=/&encoding-type=url")
+        assert response.status_code == 200
+        root = parse_xml(response.text)
+        assert [c.find('Key').text for c in root.findall('Contents')] == ['my%20file.txt']
+        assert [cp.find('Prefix').text for cp in root.findall('CommonPrefixes')] == ['sub%20dir/']
+
+        response = client.get("/spaced-files/my%20file.txt")
+        assert response.status_code == 200
+        assert response.text == "spaced"

x2s3/client_file.py

@@ -370,7 +370,13 @@ async def list_objects_v2(self,
 
             real_prefix, virtual_common = self._map_prefix(prefix, delimiter)
             if virtual_common is not None:
-                common_prefixes = [virtual_common]
+                # One virtual path segment is the whole listing, so there is
+                # nothing to page through: max-keys=0 asks for nothing, and a
+                # continuation token can only be stale here, since this branch
+                # never reports one and is never truncated. walk_path does the
+                # same with a token it cannot match.
+                if max_keys != 0 and not continuation_token:
+                    common_prefixes = [virtual_common]
             elif real_prefix is not None:
 
                 # ensure the prefix ends with a slash

x2s3/utils.py

@@ -71,8 +71,11 @@ def parse_xml(xml):
 
 def url_encode(s):
     if not s: return None
-    # AWS does something slightly strange here, maybe like this?
-    return urllib.parse.quote(s, safe='/ ').replace(' ','+')
+    # EncodingType=url means percent-encoding, so a space is %20 and not '+':
+    # that is what clients undo, whether they unquote (boto3) or call
+    # decodeURIComponent (Neuroglancer). '/' is left alone so that keys keep
+    # their structure.
+    return urllib.parse.quote(s, safe='/')
 
 
 S3_XMLNS = "http://s3.amazonaws.com/doc/2006-03-01/"