From c0872d11b31859d92c713a9398f40740b2a1a661 Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Thu, 27 Aug 2026 22:44:48 -0400 Subject: [PATCH 1/2] fix: encode spaces in listings as %20, not '+' EncodingType=url means percent-encoding, but url_encode() replaced spaces with '+', so a file named "my file.txt" was listed as key "my+file.txt". Clients undo the encoding by unquoting (boto3) or with decodeURIComponent (Neuroglancer), and neither turns '+' back into a space, so a key taken from a listing named a file that does not exist and fetching it 404'd. Any name containing a space was affected: browsing a Fileglancer data link in Neuroglancer listed the entries but could not open them. quote(s, safe='/') also encodes a literal '+' as %2B, which tells the two apart. Added a listing test over a tree with spaced names that reads a reported key back through the app. Co-Authored-By: Claude Opus 5 (1M context) --- pyproject.toml | 2 +- tests/test_file.py | 28 ++++++++++++++++++++++++++++ x2s3/utils.py | 7 +++++-- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a9a5321..41e74ac 100644 --- a/pyproject.toml +++ b/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" } diff --git a/tests/test_file.py b/tests/test_file.py index 6f338c3..31b846a 100644 --- a/tests/test_file.py +++ b/tests/test_file.py @@ -384,3 +384,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" diff --git a/x2s3/utils.py b/x2s3/utils.py index 8e0960d..fd9d589 100644 --- a/x2s3/utils.py +++ b/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/" From 934ef7597462072b2bf1873162a7b9b9ef0dc8e3 Mon Sep 17 00:00:00 2001 From: Konrad Rokicki Date: Fri, 28 Aug 2026 08:41:03 -0400 Subject: [PATCH 2/2] fix: obey max-keys and continuation tokens inside a virtual prefix A listing whose prefix stopped inside the virtual prefix emitted its one synthetic CommonPrefixes entry unconditionally, so max-keys=0 answered with KeyCount=1, and a client that passed a continuation token was handed the same entry again. The filesystem branch of the same method honours both, so the two disagreed. That entry is the entire listing and is never truncated, so there is nothing to page: max-keys=0 now returns an empty listing, and a token can only be stale, which walk_path already treats as nothing to report. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_file.py | 20 ++++++++++++++++++++ x2s3/client_file.py | 8 +++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/test_file.py b/tests/test_file.py index 31b846a..f58ca3b 100644 --- a/tests/test_file.py +++ b/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: diff --git a/x2s3/client_file.py b/x2s3/client_file.py index 04f6d34..3b90e64 100644 --- a/x2s3/client_file.py +++ b/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