From 90befa049f54ee510722f37d7da622ae893133c2 Mon Sep 17 00:00:00 2001 From: mokashang Date: Thu, 3 Sep 2026 09:19:49 -0700 Subject: [PATCH] Percent-decode username and password parsed from URLs (#1871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `infer_storage_options` was returning `parsed_path.username` and `parsed_path.password` verbatim from `urllib.parse.urlsplit`, which does not decode percent-encoded characters in the userinfo component. When a user was forced to percent-encode a reserved character in the URL — e.g. `sftp://user:pass%23with%23hash@host/path` to keep `#` out of the fragment — the FTP/SFTP/SMB backends then received the literal `pass%23with%23hash` and authentication failed. Route both fields through a small `_unquote_userinfo` helper that runs `urllib.parse.unquote(..., errors='strict')` and falls back to the raw input on `UnicodeDecodeError`. URLs without percent-encoded userinfo are unaffected (`unquote` returns the input unchanged when there is nothing to decode), and passwords with a bare `%` that happens to be followed by two hex digits producing bytes outside UTF-8 — e.g. the literal `pass%ab` password from a pre-existing URL — are preserved rather than corrupted to U+FFFD or raising downstream, which addresses the backwards-compat concern raised in review. Adds a regression test covering the encoded case, the plain passthrough, and three shapes of literal `%` in passwords. --- docs/source/changelog.rst | 7 +++++++ fsspec/tests/test_utils.py | 30 ++++++++++++++++++++++++++++++ fsspec/utils.py | 18 +++++++++++++++--- 3 files changed, 52 insertions(+), 3 deletions(-) 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