Parse file://a/b/ as a relative path, like fsspec

Author: Tomaz-Vieira

changes/4533.bugfix.md

@@ -0,0 +1,17 @@
+Mimic fsspec behavior when parsing file:// URLs
+
+File URLs like
+
+`file://a`
+
+were being parsed in `parse_store_url` with a path like
+
+`ParsedStoreUrl(..., path="")`
+
+instead of
+
+`ParsedStoreUrl(..., path="./a")`
+
+which is the behavior in `fsspec`. This also makes resolves the parsed
+paths to their absolute representation immediately, to prevent eventual
+changes in CWD from affecting the resolution of the path later.

src/zarr/storage/_utils.py

@@ -90,16 +90,24 @@ 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
-
-    # 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:
-        path = parsed.path.lstrip("/")
+    # Since we're allowing windows paths to carry URL separator characters,
+    # we'll do the same for posix paths as well
+    if parsed.scheme == "":
+        abs_path = Path(url).absolute()
+        return ParsedStoreUrl(scheme="", name=None, path=str(abs_path), raw=url)
+
+    if parsed.scheme == "file":
+        if parsed.query:
+            raise ValueError(f"Query is not supported in file:// URLs: {parsed.query}")
+        if parsed.fragment:
+            raise ValueError(f"Fragments are not supported in file:// URLs: {parsed.fragment}")
+        name = None
+        # mimic fsspec and interpret file://a/b as ./a/b
+        path = str(Path(parsed.netloc + parsed.path).absolute())
     else:
-        path = parsed.path
+        # netloc is the "host" part (store name for memory://, bucket for s3://, etc.)
+        name = parsed.netloc or None
+        path = parsed.path.lstrip("/")
 
     return ParsedStoreUrl(scheme=parsed.scheme, name=name, path=path, raw=url)
 

tests/test_store/test_utils.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import sys
+from typing import TYPE_CHECKING
 from unittest.mock import patch
 
 import pytest
@@ -9,6 +10,9 @@
 from zarr.core.buffer.core import default_buffer_prototype
 from zarr.storage._utils import ParsedStoreUrl, _normalize_byte_range_index, parse_store_url
 
+if TYPE_CHECKING:
+    from pathlib import Path
+
 
 class TestParseStoreUrl:
     """Tests for parse_store_url."""
@@ -39,18 +43,60 @@ 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"
+    @pytest.mark.parametrize(
+        ("url", "expected_path"),
+        [
+            ("file:///tmp/test", "/tmp/test"),
+            ("file:///a/b", "/a/b"),
+            ("file:///a/b/", "/a/b"),
+            ("file:///", "/"),
+        ],
+    )
+    def test_absolute_file_url(self, url: str, expected_path: str) -> None:
+        """file:///a/b (empty netloc) is an absolute path."""
+        result = parse_store_url(url)
+        assert result == ParsedStoreUrl(scheme="file", name=None, path=expected_path, raw=url)
+
+    @pytest.mark.parametrize(
+        ("url", "relative_path"),
+        [
+            ("file://a/b", "a/b"),
+            ("file://a/b/", "a/b"),
+            ("file://a", "a"),
+            ("file://./a/b", "a/b"),
+            ("file://", ""),
+        ],
+    )
+    def test_relative_file_url(
+        self, url: str, relative_path: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """file://a/b is a path relative to the cwd, not a host named "a"."""
+        monkeypatch.chdir(tmp_path)
+        result = parse_store_url(url)
+        assert result == ParsedStoreUrl(
+            scheme="file", name=None, path=str(tmp_path / relative_path), raw=url
+        )
+
+    @pytest.mark.parametrize(
+        "url",
+        [
+            "file:///a/b?query=1",
+            "file:///a/b#fragment",
+        ],
+    )
+    def test_file_url_rejects_params_query_fragment(self, url: str) -> None:
+        with pytest.raises(ValueError, match="not supported in file:// URLs"):
+            parse_store_url(url)
 
     def test_local_absolute_path(self) -> None:
         result = parse_store_url("/local/path")
         assert result == ParsedStoreUrl(scheme="", name=None, path="/local/path", raw="/local/path")
 
-    def test_local_relative_path(self) -> None:
+    def test_local_relative_path(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+        monkeypatch.chdir(tmp_path)
         result = parse_store_url("relative/path")
         assert result == ParsedStoreUrl(
-            scheme="", name=None, path="relative/path", raw="relative/path"
+            scheme="", name=None, path=str(tmp_path / "relative/path"), raw="relative/path"
         )
 
     @pytest.mark.parametrize(