Percent-decode username and password parsed from URLs (#1871) - #2114
Open
mokashang wants to merge 1 commit into
Open
Percent-decode username and password parsed from URLs (#1871)#2114mokashang wants to merge 1 commit into
mokashang wants to merge 1 commit into
Conversation
`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. Decode both fields with `urllib.parse.unquote` before returning them so that backends receive the real credentials. URLs without percent- encoded userinfo are unaffected (`unquote` returns the input unchanged when there is nothing to decode), and the HTTP/HTTPS branch short- circuits before this code path, so its behavior does not change. Adds a regression test covering both the encoded and plain forms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1871
Problem
infer_storage_optionsreturnedparsed_path.usernameandparsed_path.passwordverbatim fromurllib.parse.urlsplit, which does notdecode percent-encoded characters in the userinfo component. When a user is
forced to percent-encode a reserved character in the URL — the classic case
being
#in a password, which would otherwise be parsed as the fragmentdelimiter — the backend receives the literal encoded string:
The two reports on #1871 both hit this from the FTP/SFTP side, and every
infer_storage_optionsconsumer that mapsusername/passwordstraight intoits backend client —
FTPFileSystem,SFTPFileSystem,SMBFileSystem,WebHDFS, arrow — has the same failure mode.Change
Percent-decode both fields with
urllib.parse.unquotebefore returning them:unquotereturns its input unchanged when there is nothing to decode, soURLs with plain-ASCII credentials (which every existing test uses) are
unaffected. The HTTP/HTTPS branch short-circuits at line 87 and never reaches
this code, so
requestscontinues to handle its own URL parsing as noted inthe comment.
Regarding @martindurant's caution on the issue ("SSH would not expect to
have encoded strings"): SSH/SFTP/FTP servers do not implement URL decoding
themselves — they compare raw credentials — so decoding on our side is what
lets an encoded URL work at all. The @Jeansidharta reproducer is exactly this
case: paramiko is handed
pass%23with%23hash%23charand rejects it.Test plan
test_infer_options_percent_encoded_userinfocovering both theencoded case (
user%40corp/p%23ass%2Fword%20!) and the unencodedpassthrough case.
pytest fsspec/tests/test_utils.py— 99 passed.pytest fsspec/tests/ fsspec/implementations/tests/with backend suitesthat need network/cloud services excluded — 1553 passed, 176 skipped,
2 xfailed.
ruff check/ruff format --checkclean on the touched files.Devsection.