Skip to content

fix(storage): yield only direct children from FsspecStore.list_dir - #4402

Merged
d-v-b merged 5 commits into
zarr-developers:mainfrom
d-v-b:fix/fsspec-list-dir-http-children
Sep 24, 2026
Merged

d-v-b merged 5 commits into
zarr-developers:mainfrom
d-v-b:fix/fsspec-list-dir-http-children

Conversation

@d-v-b

@d-v-b d-v-b commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

AI PR that ensures that FspecStore.list_dir doesn't return garbage for http stores

🤖 AI text below 🤖

Closes #3575.

Problem

Over HTTP, fsspec builds a directory listing by scraping the links on an HTML index page. Some of those links aren't children of the directory, and FsspecStore.list_dir passed them through as keys:

  • A link to the site root (href="/") is treated by fsspec as relative and joined onto the directory URL, producing the directory itself. list_dir stripped that down to "", and Group.getitem("") resolves to the group itself, so the group listed itself as a member named "". This is the bug reported in Zarr over https returns an empty string in keys() #3575.
  • In-page anchors (href="#usage") became keys like #usage. Fetching metadata under such a key drops the fragment and returns the HTML page, so on the ECMWF dataset from the issue, whose page has since gained anchor links, keys() now raises JSONDecodeError.
  • Directory entries kept a trailing / (count/), contrary to the Store.list_dir contract.

Fix

list_dir now:

  • skips http(s):// entries that contain # or ?, since they name the index page rather than an object (other filesystems are unaffected, so a key containing # on S3 or local disk is still listed);
  • strips the trailing /;
  • drops names that are empty or still contain /;
  • yields each name once.

The fsspec behaviour that turns href="/" into a self-link is arguably an fsspec bug too, but zarr shouldn't depend on it being fixed.

Test

test_list_dir_http_yields_only_children serves an index page from a local http.server with links /, ../, #, #usage, ?sort=name, a/, a, b, /group/c/, and checks that list_dir("") returns exactly a, b, c. It fails on main with ['', '#', '#usage', 'a', 'a/', ...].

🤖 Generated with Claude Code

An HTTP listing is scraped from the links on an HTML index page. A link to
the site root ("/") becomes a self-link that list_dir reduced to "", so a
group listed itself as a member named "". In-page anchors ("#usage") became
keys whose metadata fetch returned the HTML page, and directories carried a
trailing "/".

list_dir now skips http(s) entries with a fragment or query, strips the
trailing "/", drops empty or nested names, and yields each name once.

Closes zarr-developers#3575

Assisted-by: ClaudeCode:claude-opus-5-5
Assisted-by: ClaudeCode:claude-opus-5-5
@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.36%. Comparing base (eb2ce59) to head (1074ff9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4402   +/-   ##
=======================================
  Coverage   94.35%   94.36%           
=======================================
  Files          93       93           
  Lines       13131    13142   +11     
=======================================
+ Hits        12390    12401   +11     
  Misses        741      741           
Files with missing lines Coverage Δ
src/zarr/storage/_fsspec.py 91.71% <100.00%> (+0.53%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The "#"/"?" check ran on the whole URL, so a store rooted at a URL
containing either character would list nothing. It now runs on the name
left after stripping the prefix. HTTP names are also normalized with
posixpath.normpath, so "./a" lists as "a" and "./" no longer lists as ".".

Assisted-by: ClaudeCode:claude-opus-5-5
@d-v-b
d-v-b marked this pull request as ready for review September 24, 2026 15:44
@d-v-b
d-v-b merged commit 34b7c3d into zarr-developers:main Sep 24, 2026
39 checks passed
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.

Zarr over https returns an empty string in keys()

1 participant