Skip to content

Fix listing bounds in S3 and GS stores - #48

Merged
sduchesneau merged 1 commit into
developfrom
fix/listing-bounds
Aug 25, 2026
Merged

sduchesneau merged 1 commit into
developfrom
fix/listing-bounds

Conversation

@sduchesneau

Copy link
Copy Markdown
Contributor

Bugs

Both were caught by new shared storetests cases, verified by reverting the source and watching each one fail on exactly the store that had it (MinIO / fake-gcs).

  • S3 WalkFromTo compared its exclusive end point against a prefix-stripped name while the walked names are store-relative. With a non-empty prefix the bound never matched, so the walk yielded keys past it and paged to the end of the prefix.
  • GS WalkFromTo built the listing's end offset with filepath.Join, which ate the trailing /. Walking up to alpha/ dropped the object named alpha, which sorts before the bound.

Same results, less work

  • ListFoldersFromTo on S3 and Azure returns at the first folder >= exclusiveTo instead of listing the rest of the prefix to discard it. Azure has no server-side bound, so this is its only lever.
  • S3 ListFoldersFromTo pushes its lower bound down whole, minus the trailing / that makes StartAfter exclusive. It used to drop the bound's last character, and push nothing at all when a single character was left.
  • S3 WalkFromTo starts at the key right before the starting point (helloworld.html → helloworld.htmk) instead of the stem (helloworld.htm), which had the service send every key in between for the walk to filter out. On fixed-width keys such as block numbers that window held every key sharing the stem; it now holds none. New keyBefore() decrements the last rune, so the marker stays valid UTF-8, and falls back to dropping it (control characters, invalid bytes, the surrogate boundary).

Tests

  • storetests: TestWalkFromTo_WithPrefix, TestWalkFromTo_FolderEndPoint, TestListFoldersFromTo_SingleCharacterBounds — the suite only ever walked ranges with an empty prefix and never used a slash-terminated end point, which is how both bugs got in.
  • s3store_mock_test.go: a recording RoundTripper serving canned pages, asserting the start-after/prefix/delimiter each bound shape produces and proving early termination by round-trip count — a real backend can't show you that. Plus a keyBefore table with a sorts-before invariant.
  • gsstore_test.go: TestGSStore_calculateEndOffset.

Green against MinIO, Ceph, fake-gcs and Azurite (docker compose up -d), plus local/memory/mock.

Unrelated, pre-existing: azure_test.go TestAzureStoreWriteObject gates on AZURE_STORAGE_KEY but hardcodes account streamingfasttest01/container myblobs, so pointing it at Azurite 404s. Left alone.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ESMxv9zsuiAXikNs1wwYup

WalkFromTo compared its exclusive end point against a prefix-stripped
name on S3, and GS lost the trailing slash of that bound, so a folder
end point skipped the object named after it. Ranged folder listings now
stop at the upper bound instead of listing the rest of the prefix.

@sduchesneau sduchesneau left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (claude/gpt sol wrote that)

@UlysseCorbeil UlysseCorbeil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing jumps out, lgtm

@sduchesneau
sduchesneau merged commit 56e8748 into develop Aug 25, 2026
3 checks passed
@sduchesneau
sduchesneau deleted the fix/listing-bounds branch August 25, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants