From d4764cf5b93d5db9f1094488a32eb392c5fbdec0 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 16 Oct 2026 19:22:05 +0800 Subject: [PATCH 1/2] fix(fsspec): handle Windows drive paths with the file protocol On Windows, paths like `D:/data/file.txt` are parsed by `urlparse` with `D:` treated as a URL scheme, so the drive is dropped from the bucket and later re-resolved against the current drive, producing duplicated-drive paths like `D:/D:/data/file.txt` (os error 123). This broke every `file`-protocol operation on Windows, including relative paths, which are absolutized to drive paths (#763, #555). - `_split_path`: detect single-letter drive schemes on Windows and return the drive (with colon, e.g. `D:`) as the bucket, preserving the original drive instead of re-resolving against the cwd drive. - `_construct_store`: for the `file` protocol with a drive bucket, construct a `LocalStore` rooted at that drive directly instead of going through `from_url`, which cannot express drive roots. - `_info`: on Windows, `head` on a directory raises `GenericError` rather than `FileNotFoundError`, so also catch `GenericError` and fall through to the directory-listing fallback for local directories. - `_get_file`: a Windows drive-letter local path was treated as a URL scheme and silently skipped; only treat genuine schemes as remote sources. --- obstore/python/obstore/fsspec.py | 49 +++++++++++++- tests/test_fsspec.py | 107 +++++++++++++++++++++++++++---- 2 files changed, 140 insertions(+), 16 deletions(-) diff --git a/obstore/python/obstore/fsspec.py b/obstore/python/obstore/fsspec.py index 5587b1ec..15fe32e0 100644 --- a/obstore/python/obstore/fsspec.py +++ b/obstore/python/obstore/fsspec.py @@ -33,6 +33,7 @@ from __future__ import annotations import asyncio +import os import warnings from collections import defaultdict from functools import cached_property, lru_cache @@ -46,7 +47,8 @@ from fsspec.implementations.local import make_path_posix from obstore import open_reader, open_writer -from obstore.store import from_url +from obstore.exceptions import GenericError +from obstore.store import LocalStore, from_url if TYPE_CHECKING: from collections.abc import Coroutine, Iterable @@ -108,6 +110,16 @@ """A type hint for all supported protocols.""" +def _is_windows_drive_path(path: str) -> bool: + """Whether the input is a Windows drive path such as `D:/dir/file`. + + `urlparse` parses the drive letter of such a path as the URL scheme, so + drive paths must be detected before any URL handling. + """ + parsed = urlparse(path) + return os.name == "nt" and len(parsed.scheme) == 1 and parsed.scheme.isalpha() + + class FsspecStore(fsspec.asyn.AsyncFileSystem): """An fsspec implementation based on a obstore Store. @@ -295,11 +307,29 @@ def _split_path(self, path: str) -> tuple[str, str]: # If the protocol doesn't require buckets, return empty bucket and full path if self.protocol in protocol_without_bucket: + if self.protocol == "file" and _is_windows_drive_path(path): + # A Windows drive path such as `D:\dir\file` or `D:/dir/file` is + # not a URL: `urlparse` would parse the drive letter as the URL + # scheme. Resolve the whole path and return the addressed drive + # as the bucket so that `_construct_store` can root a local + # store on that drive. + drive, _, rest = str(make_path_posix(path)).partition(":") + return (f"{drive}:", rest.lstrip("/")) parsed_path = ( f"{parsed.netloc}/{parsed.path.lstrip('/')}" if parsed.scheme else path ) if self.protocol == "file": parsed_path = str(make_path_posix(parsed_path)) + if os.name == "nt": + # On Windows, `make_path_posix` resolves any path to a + # drive-rooted path such as `D:/dir/file`. As the store for + # an empty bucket is rooted at the current drive, joining a + # drive-rooted path onto it would duplicate the drive; + # instead return the resolved drive as the bucket, as for + # drive paths above. + drive, _, rest = parsed_path.partition(":") + if len(drive) == 1 and drive.isalpha(): + return (f"{drive}:", rest.lstrip("/")) return ( "", parsed_path, @@ -330,6 +360,11 @@ def _split_path(self, path: str) -> tuple[str, str]: def _construct_store(self, bucket: str) -> ObjectStore: protocol = self.protocol if isinstance(self.protocol, str) else self.protocol[0] + if protocol == "file" and bucket: + # A `file://` URL cannot express a Windows drive root: the drive + # would be parsed as the URL authority, not the path. Root the + # local store on the drive directly. + return LocalStore(f"{bucket}/", **self._config_kwargs) return from_url( url=f"{protocol}://{bucket}", config=self._config, # type: ignore (type narrowing) @@ -485,7 +520,10 @@ async def _put_file( async def _get_file(self, rpath: str, lpath: str, **_kwargs: Any) -> None: res = urlparse(lpath) - if res.scheme or await self._local_store._isdir(lpath): # noqa: SLF001 + if ( + (res.scheme and not _is_windows_drive_path(lpath)) + or await self._local_store._isdir(lpath) # noqa: SLF001 + ): # lpath need to be local file and cannot contain scheme return @@ -520,6 +558,13 @@ async def _info(self, path: str, **_kwargs: Any) -> dict[str, Any]: } except FileNotFoundError: pass + except GenericError: + # On Windows, opening a directory to stat it fails with access + # denied before the upstream is-directory check can report it as + # not found, as `head` does on POSIX. Fall through to the + # directory fallback below only for directories. + if self.protocol != "file" or not Path(path).is_dir(): # noqa: ASYNC240 + raise # Ref: https://github.com/fsspec/s3fs/blob/01b9c4b838b81375093ae1d78562edf6bdc616ea/s3fs/core.py#L1471-L1492 # We check to see if the path is a directory by attempting to list its diff --git a/tests/test_fsspec.py b/tests/test_fsspec.py index 8bf490ef..a38674ac 100644 --- a/tests/test_fsspec.py +++ b/tests/test_fsspec.py @@ -363,24 +363,48 @@ def test_split_path(fs: FsspecStore): # in url format, without bucket file_fs = FsspecStore("file") - assert file_fs._split_path("file:///mybucket/path/to/file") == ( - "", - "/mybucket/path/to/file", - ) - assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/") + drive, cwd_rel = os.path.splitdrive(Path.cwd().as_posix()) + cwd_rel = cwd_rel.lstrip("/") + if os.name == "nt": + # On Windows, every path resolves onto a drive and the addressed drive + # is returned as the bucket so that `_construct_store` can root the + # local store on that drive. + for input_path in ("file:///mybucket/path/to/file", "/mybucket/path/to/file"): + assert file_fs._split_path(input_path) == ( + drive, + f"{cwd_rel}/mybucket/path/to/file", + ) + # An explicitly addressed drive is preserved. + assert file_fs._split_path(f"{drive}/data-bucket/") == ( + drive, + "data-bucket/", + ) + else: + assert file_fs._split_path("file:///mybucket/path/to/file") == ( + "", + "/mybucket/path/to/file", + ) + assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/") - # path format, without bucket - assert file_fs._split_path("/mybucket/path/to/file") == ( - "", - "/mybucket/path/to/file", - ) - assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/") + # path format, without bucket + assert file_fs._split_path("/mybucket/path/to/file") == ( + "", + "/mybucket/path/to/file", + ) + assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/") # relative path, without bucket -> resolved to absolute for the "file" protocol rel_bucket, rel_path = file_fs._split_path("relative/path/to/file") - assert rel_bucket == "" - assert Path(rel_path).is_absolute() - assert rel_path.endswith("relative/path/to/file") + if os.name == "nt": + # The resolved drive is returned as the bucket; joining it back onto + # the path yields an absolute path. + assert rel_bucket == drive + assert rel_path == f"{cwd_rel}/relative/path/to/file" + assert Path(f"{drive}/{rel_path}").is_absolute() + else: + assert rel_bucket == "" + assert Path(rel_path).is_absolute() + assert rel_path.endswith("relative/path/to/file") # memory protocol keeps relative paths untouched (paths are virtual) memory_fs = FsspecStore("memory") # type: ignore (no __init__ overload for memory) @@ -390,6 +414,61 @@ def test_split_path(fs: FsspecStore): ) +@pytest.mark.skipif(os.name != "nt", reason="Windows drive path handling") +def test_file_protocol_windows_drive_paths(): + """The fsspec ``file`` protocol supports Windows drive paths. + + On Windows, every path addresses a drive; the drive is returned as the + bucket so that `_construct_store` can root the local store there, and + directories resolve through the listing fallback, matching the POSIX + behavior of `head` on a directory. + + Ref: https://github.com/developmentseed/obstore/issues/763 + Ref: https://github.com/developmentseed/obstore/issues/555 + """ + register("file") + fs = fsspec.filesystem("file") + + with TemporaryDirectory() as tmp: + posix_dir = Path(tmp).as_posix() + file_path = f"{posix_dir}/file.txt" + Path(tmp, "file.txt").write_text("data") + subdir = Path(tmp, "sub") + subdir.mkdir() + Path(tmp, "sub", "inner.txt").write_text("inner") + + assert fs.exists(file_path) + assert not fs.exists(f"{posix_dir}/missing.txt") + assert fs.info(file_path)["size"] == 4 + assert fs.cat_file(file_path) == b"data" + with fs.open(file_path) as f: + assert f.read() == b"data" + + assert fs.exists(f"{posix_dir}/sub") + assert fs.isdir(f"{posix_dir}/sub") + assert not fs.isdir(file_path) + + names = fs.ls(posix_dir, detail=False) + assert file_path in names + assert f"{posix_dir}/sub" in names + assert fs.ls(f"{posix_dir}/sub", detail=False) == [ + f"{posix_dir}/sub/inner.txt", + ] + + fs.pipe_file(f"{posix_dir}/piped.txt", b"piped") + assert fs.cat_file(f"{posix_dir}/piped.txt") == b"piped" + with fs.open(f"{posix_dir}/written.txt", mode="wb") as f: + f.write(b"written") + assert fs.cat_file(f"{posix_dir}/written.txt") == b"written" + + local_out = Path(tmp, "out.txt") + fs.get(file_path, str(local_out)) + assert local_out.read_text() == "data" + + fs.rm_file(f"{posix_dir}/piped.txt") + assert not fs.exists(f"{posix_dir}/piped.txt") + + def test_list( minio_bucket: tuple[S3Config, ClientConfig], ): From 3ee20b8b35377c2efc4ed1f58cfb5a706c412583 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 16 Oct 2026 23:49:46 +0800 Subject: [PATCH 2/2] test(fsspec): simplify Windows path assertions --- tests/test_fsspec.py | 34 ++++------------------------------ 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/tests/test_fsspec.py b/tests/test_fsspec.py index a38674ac..ad5889de 100644 --- a/tests/test_fsspec.py +++ b/tests/test_fsspec.py @@ -361,47 +361,21 @@ def test_split_path(fs: FsspecStore): with pytest.raises(ValueError, match=r"Expected protocol to be s3. Got gs"): fs._split_path("gs://data-bucket/") - # in url format, without bucket + # Paths without a bucket keep their existing POSIX behavior. Windows + # drive splitting and file operations are covered by the focused test below. file_fs = FsspecStore("file") - drive, cwd_rel = os.path.splitdrive(Path.cwd().as_posix()) - cwd_rel = cwd_rel.lstrip("/") - if os.name == "nt": - # On Windows, every path resolves onto a drive and the addressed drive - # is returned as the bucket so that `_construct_store` can root the - # local store on that drive. - for input_path in ("file:///mybucket/path/to/file", "/mybucket/path/to/file"): - assert file_fs._split_path(input_path) == ( - drive, - f"{cwd_rel}/mybucket/path/to/file", - ) - # An explicitly addressed drive is preserved. - assert file_fs._split_path(f"{drive}/data-bucket/") == ( - drive, - "data-bucket/", - ) - else: + if os.name != "nt": assert file_fs._split_path("file:///mybucket/path/to/file") == ( "", "/mybucket/path/to/file", ) assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/") - - # path format, without bucket assert file_fs._split_path("/mybucket/path/to/file") == ( "", "/mybucket/path/to/file", ) assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/") - - # relative path, without bucket -> resolved to absolute for the "file" protocol - rel_bucket, rel_path = file_fs._split_path("relative/path/to/file") - if os.name == "nt": - # The resolved drive is returned as the bucket; joining it back onto - # the path yields an absolute path. - assert rel_bucket == drive - assert rel_path == f"{cwd_rel}/relative/path/to/file" - assert Path(f"{drive}/{rel_path}").is_absolute() - else: + rel_bucket, rel_path = file_fs._split_path("relative/path/to/file") assert rel_bucket == "" assert Path(rel_path).is_absolute() assert rel_path.endswith("relative/path/to/file")