Fix sftp join issue for non-root prefixed paths (#294)

* tests: add test reproducing join issue for non-root prefixed paths

* tests: improve ssh container fixture robustness

* tests: add additional test cases for constructing ssh paths

* upath._flavour: fix ssh path parsing (enforce absolute paths)

* typing: fix error in kwargs

Author: ap--

upath/_flavour.py

@@ -78,6 +78,7 @@ class ProtocolConfig(TypedDict):
     netloc_is_anchor: set[str]
     supports_empty_parts: set[str]
     meaningful_trailing_slash: set[str]
+    root_marker_override: dict[str, str]
 
 
 class WrappedFileSystemFlavour:  # (pathlib_abc.FlavourBase)
@@ -109,8 +110,6 @@ class WrappedFileSystemFlavour:  # (pathlib_abc.FlavourBase)
             "https",
             "s3",
             "s3a",
-            "sftp",
-            "ssh",
             "smb",
             "gs",
             "gcs",
@@ -135,6 +134,10 @@ class WrappedFileSystemFlavour:  # (pathlib_abc.FlavourBase)
             "http",
             "https",
         },
+        "root_marker_override": {
+            "ssh": "/",
+            "sftp": "/",
+        },
     }
 
     def __init__(
@@ -144,6 +147,7 @@ def __init__(
         netloc_is_anchor: bool = False,
         supports_empty_parts: bool = False,
         meaningful_trailing_slash: bool = False,
+        root_marker_override: str | None = None,
     ) -> None:
         """initialize the flavour with the given fsspec"""
         self._spec = spec
@@ -163,6 +167,12 @@ def __init__(
         #   - UPath._parse_path
         self.has_meaningful_trailing_slash = bool(meaningful_trailing_slash)
 
+        # some filesystems require UPath to enforce a specific root marker
+        if root_marker_override is None:
+            self.root_marker_override = None
+        else:
+            self.root_marker_override = str(root_marker_override)
+
     @classmethod
     @lru_cache(maxsize=None)
     def from_protocol(
@@ -172,10 +182,11 @@ def from_protocol(
         """return the fsspec flavour for the given protocol"""
 
         _c = cls.protocol_config
-        config = {
+        config: dict[str, Any] = {
             "netloc_is_anchor": protocol in _c["netloc_is_anchor"],
             "supports_empty_parts": protocol in _c["supports_empty_parts"],
             "meaningful_trailing_slash": protocol in _c["meaningful_trailing_slash"],
+            "root_marker_override": _c["root_marker_override"].get(protocol),
         }
 
         # first try to get an already imported fsspec filesystem class
@@ -217,7 +228,10 @@ def protocol(self) -> tuple[str, ...]:
 
     @property
     def root_marker(self) -> str:
-        return self._spec.root_marker
+        if self.root_marker_override is not None:
+            return self.root_marker_override
+        else:
+            return self._spec.root_marker
 
     @property
     def local_file(self) -> bool:
@@ -278,7 +292,7 @@ def join(self, path: PathOrStr, *paths: PathOrStr) -> str:
                 drv, p0 = self.splitdrive(path)
             p0 = p0 or self.sep
         else:
-            p0 = str(self.strip_protocol(path))
+            p0 = str(self.strip_protocol(path)) or self.root_marker
             pN = list(map(self.stringify_path, paths))
             drv = ""
         if self.supports_empty_parts:

upath/tests/conftest.py

@@ -486,7 +486,6 @@ def ssh_container():
     )
     try:
         subprocess.run(shlex.split(cmd))
-        time.sleep(1)
         yield {
             "host": "localhost",
             "port": 2222,
@@ -499,7 +498,7 @@ def ssh_container():
 
 @pytest.fixture
 def ssh_fixture(ssh_container, local_testdir, monkeypatch):
-    pytest.importorskip("paramiko", reason="sftp tests require paramiko")
+    paramiko = pytest.importorskip("paramiko", reason="sftp tests require paramiko")
 
     cls = fsspec.get_filesystem_class("ssh")
     if cls.put != fsspec.AbstractFileSystem.put:
@@ -509,13 +508,28 @@ def ssh_fixture(ssh_container, local_testdir, monkeypatch):
 
         monkeypatch.setattr(_DEFAULT_CALLBACK, "relative_update", lambda *args: None)
 
-    fs = fsspec.filesystem(
-        "ssh",
-        host=ssh_container["host"],
-        port=ssh_container["port"],
-        username=ssh_container["username"],
-        password=ssh_container["password"],
-    )
+    for _ in range(100):
+        try:
+            fs = fsspec.filesystem(
+                "ssh",
+                host=ssh_container["host"],
+                port=ssh_container["port"],
+                username=ssh_container["username"],
+                password=ssh_container["password"],
+                timeout=10.0,
+                banner_timeout=30.0,
+                skip_instance_cache=True,
+            )
+        except (
+            paramiko.ssh_exception.NoValidConnectionsError,
+            paramiko.ssh_exception.SSHException,
+        ):
+            time.sleep(0.1)
+            continue
+        break
+    else:
+        raise RuntimeError("issue with openssh-container startup")
+
     fs.put(local_testdir, "/app/testdir", recursive=True)
     try:
         yield "ssh://{username}:{password}@{host}:{port}/app/testdir/".format(

upath/tests/implementations/test_sftp.py

@@ -38,3 +38,24 @@ def test_mkdir_parents_true_exists_ok_false(self):
     @_xfail_old_fsspec
     def test_mkdir_parents_true_exists_ok_true(self):
         super().test_mkdir_parents_true_exists_ok_true()
+
+
+@pytest.mark.parametrize(
+    "args,parts",
+    [
+        (("sftp://user@host",), ("/",)),
+        (("sftp://user@host/",), ("/",)),
+        (("sftp://user@host", ""), ("/",)),
+        (("sftp://user@host/", ""), ("/",)),
+        (("sftp://user@host", "/"), ("/",)),
+        (("sftp://user@host/", "/"), ("/",)),
+        (("sftp://user@host/abc",), ("/", "abc")),
+        (("sftp://user@host", "abc"), ("/", "abc")),
+        (("sftp://user@host", "/abc"), ("/", "abc")),
+        (("sftp://user@host/", "/abc"), ("/", "abc")),
+    ],
+)
+def test_join_produces_correct_parts(args, parts):
+    pth = UPath(*args)
+    assert pth.storage_options == {"host": "host", "username": "user"}
+    assert pth.parts == parts