diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 2df3850fb..4b5ab1673 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -27,6 +27,13 @@ Fixes - End the transaction even when a commit or discard raises, so the filesystem is not left in transaction mode and deferred temporary files are cleaned up +- Percent-decode the username and password parsed from URLs in + ``infer_storage_options`` so that backends (ftp, sftp, smb, ...) receive + the real credentials rather than their URL-encoded form; a password whose + bare ``%`` produces bytes that are not valid UTF-8 (for example + ``pass%ab``) is left untouched so that pre-existing URLs with unescaped + ``%`` continue to work (#1871) + 2026.7.0 -------- diff --git a/fsspec/tests/test_utils.py b/fsspec/tests/test_utils.py index 10289b740..16a412e30 100644 --- a/fsspec/tests/test_utils.py +++ b/fsspec/tests/test_utils.py @@ -235,6 +235,36 @@ def test_infer_composite_protocol(): assert out["path"] == "" +def test_infer_options_percent_encoded_userinfo(): + # Percent-encoded characters in the userinfo component must be decoded + # so that backends (ftp, sftp, smb, ...) receive the real credentials + # rather than the encoded form that appeared in the URL. + so = infer_storage_options( + "sftp://user%40corp:p%23ass%2Fword%20!@example.com:22/path" + ) + assert so["username"] == "user@corp" + assert so["password"] == "p#ass/word !" + assert so["host"] == "example.com" + assert so["port"] == 22 + assert so["path"] == "/path" + + # Unencoded credentials pass through unchanged. + so = infer_storage_options("ftp://plainuser:plainpw@example.com/f") + assert so["username"] == "plainuser" + assert so["password"] == "plainpw" + + # Backwards-compat: URLs whose password contains a bare ``%`` that is + # not a valid percent-escape must be preserved. ``50%off`` (no hex + # after the %) and ``pass%`` (trailing %) already round-trip through + # ``urllib.parse.unquote``; ``pass%ab`` decodes to the byte 0xAB which + # is not valid UTF-8, so the loose default ``errors='replace'`` would + # silently corrupt it to U+FFFD. We fall back to the raw form in that + # case so the caller still sees the original password. + for pw in ("50%off", "pass%", "pass%ab"): + so = infer_storage_options(f"sftp://user:{pw}@example.com/f") + assert so["password"] == pw, pw + + @pytest.mark.parametrize( "urlpath, expected_path", ( diff --git a/fsspec/utils.py b/fsspec/utils.py index 5691de229..4583fe165 100644 --- a/fsspec/utils.py +++ b/fsspec/utils.py @@ -13,7 +13,7 @@ from hashlib import md5 from importlib.metadata import version from typing import IO, TYPE_CHECKING, Any, TypeVar -from urllib.parse import urlsplit +from urllib.parse import unquote, urlsplit if TYPE_CHECKING: import pathlib @@ -27,6 +27,18 @@ T = TypeVar("T") +def _unquote_userinfo(value: str) -> str: + # Percent-decode a URL userinfo component (username or password). + # Falls back to the raw input when the decoded bytes do not form valid + # UTF-8, so that a literal ``%`` followed by two hex digits that happens + # to produce a non-UTF-8 byte (e.g. a password containing ``%ab``) is + # preserved instead of being replaced by U+FFFD or raising downstream. + try: + return unquote(value, errors="strict") + except UnicodeDecodeError: + return value + + def infer_storage_options( urlpath: str, inherit_storage_options: dict[str, Any] | None = None ) -> dict[str, Any]: @@ -104,9 +116,9 @@ def infer_storage_options( if parsed_path.port: options["port"] = parsed_path.port if parsed_path.username: - options["username"] = parsed_path.username + options["username"] = _unquote_userinfo(parsed_path.username) if parsed_path.password: - options["password"] = parsed_path.password + options["password"] = _unquote_userinfo(parsed_path.password) if parsed_path.query: options["url_query"] = parsed_path.query