Skip to content

fix: Fixes parsing of relative file:// URLs #4533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tomaz-Vieira wants to merge 2 commits into
base: main
Choose a base branch
from
+32 −11
Open

fix: Fixes parsing of relative file:// URLs #4533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9e3641
Fixes parsing of relative file:// URLs
Tomaz-Vieira Oct 20, 2026
4092d9b
Add file to changes/
Tomaz-Vieira Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changes/4533.bugfix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Properly parses relative `file://` URLs

URLs with a relative file path like

`file://a/b`

were being parsed in `parse_store_url` into something like

`ParsedStoreUrl(..., path="b")`

because `a` was being seen as a hostname.
16 changes: 8 additions & 8 deletions src/zarr/storage/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,16 @@ def parse_store_url(url: str) -> ParsedStoreUrl:

parsed = urlparse(url)

# netloc is the "host" part (store name for memory://, bucket for s3://, etc.)
name = parsed.netloc or None
if parsed.scheme in ("file", ""):
name = None
path = url.removeprefix("file://")

@d-v-b d-v-b Oct 20, 2026

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.

dont do string manipulation. we just parsed the url. used the value we got from parsing it

@Tomaz-Vieira Tomaz-Vieira Oct 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't like it either, but re-joining pieces of the parsed URL is destructive:

>>> urlparse("file://a/b")
ParseResult(scheme='file', netloc='a', path='/b', params='', query='', fragment='')
>>> urlparse("file://a/b?")
ParseResult(scheme='file', netloc='a', path='/b', params='', query='', fragment='')

@d-v-b d-v-b Oct 20, 2026

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.

fwiw "file://a/b" is not a valid file uri. maybe thats part of the issue here

@d-v-b d-v-b Oct 20, 2026

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.

unless your hostname is "a"

@d-v-b d-v-b Oct 20, 2026

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.

i did a bit of reading and it seems like fsspec was naughty and treats file://a/b as a relative path starting with a, so we are probably on the hook for continuing to support that, even though its clearly wrong.

try this:

parsed = urlparse(url)

if parsed.scheme == "file":
    # RFC 8089: the host is "localhost" or empty. Anything else is a user
    # writing a relative path, so fold it back into the path.
    host = "" if parsed.netloc == "localhost" else parsed.netloc
    path = host + parsed.path
    if path == "":
        raise ValueError(f"The URL {url!r} has no path. ...")
    name = None
elif parsed.scheme:
    name = parsed.netloc or None
    path = parsed.path.lstrip("/")
else:
    name = None
    path = parsed.path

then, in LocalStore, make the root absolute. We need to check if anyone is affected downstream of that change.

else:
# netloc is the "host" part (store name for memory://, bucket for s3://, etc.)
name = parsed.netloc or None

# For URLs with a scheme and netloc (like memory://store/path or s3://bucket/key),
# strip the leading slash from the path component.
# For local paths (no scheme), preserve the path as-is.
if parsed.scheme and parsed.netloc:
# For URLs with a non-file scheme and netloc (like memory://store/path or s3://bucket/key),
# strip the leading slash from the path component.
path = parsed.path.lstrip("/")
else:
path = parsed.path

return ParsedStoreUrl(scheme=parsed.scheme, name=name, path=path, raw=url)

Expand Down
16 changes: 13 additions & 3 deletions tests/test_store/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,19 @@ def test_s3_url(self) -> None:
scheme="s3", name="bucket", path="key", raw="s3://bucket/key"
)

def test_file_url(self) -> None:
result = parse_store_url("file:///tmp/test")
assert result.scheme == "file"
def test_absolute_file_url(self) -> None:
url = "file:///something/absolute"
result = parse_store_url(url)
assert result == ParsedStoreUrl(
scheme="file", name=None, path="/something/absolute", raw=url
)

def test_relative_file_url(self) -> None:
url = "file://something/relative"
result = parse_store_url(url)
assert result == ParsedStoreUrl(
scheme="file", name=None, path="something/relative", raw=url
)

def test_local_absolute_path(self) -> None:
result = parse_store_url("/local/path")
Expand Down
Loading