diff --git a/news/5968.bugfix b/news/5968.bugfix new file mode 100644 index 00000000000..c4401b3d112 --- /dev/null +++ b/news/5968.bugfix @@ -0,0 +1 @@ +Percent-decode special characters in SVN URL credentials. diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 6fc8c7e3f3e..d03a507a57d 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -26,7 +26,6 @@ from pip._vendor.six.moves import xmlrpc_client # type: ignore from pip._vendor.six.moves.urllib import parse as urllib_parse from pip._vendor.six.moves.urllib import request as urllib_request -from pip._vendor.six.moves.urllib.parse import unquote as urllib_unquote from pip._vendor.urllib3.util import IS_PYOPENSSL import pip @@ -39,8 +38,8 @@ from pip._internal.utils.logging import indent_log from pip._internal.utils.misc import ( ARCHIVE_EXTENSIONS, ask_path_exists, backup_dir, call_subprocess, consume, - display_path, format_size, get_installed_version, rmtree, splitext, - unpack_file, + display_path, format_size, get_installed_version, rmtree, + split_auth_from_netloc, splitext, unpack_file, ) from pip._internal.utils.setuptools_build import SETUPTOOLS_SHIM from pip._internal.utils.temp_dir import TempDirectory @@ -142,8 +141,8 @@ def __init__(self, prompting=True): def __call__(self, req): parsed = urllib_parse.urlparse(req.url) - # Get the netloc without any embedded credentials - netloc = parsed.netloc.rsplit("@", 1)[-1] + # Split the credentials from the netloc. + netloc, url_user_password = split_auth_from_netloc(parsed.netloc) # Set the url of the request to the url without any credentials req.url = urllib_parse.urlunparse(parsed[:1] + (netloc,) + parsed[2:]) @@ -151,9 +150,9 @@ def __call__(self, req): # Use any stored credentials that we have for this netloc username, password = self.passwords.get(netloc, (None, None)) - # Extract credentials embedded in the url if we have none stored + # Use the credentials embedded in the url if we have none stored if username is None: - username, password = self.parse_credentials(parsed.netloc) + username, password = url_user_password # Get creds from netrc if we still don't have them if username is None and password is None: @@ -213,15 +212,6 @@ def warn_on_401(self, resp, **kwargs): logger.warning('401 Error, Credentials not correct for %s', resp.request.url) - def parse_credentials(self, netloc): - if "@" in netloc: - userinfo = netloc.rsplit("@", 1)[0] - if ":" in userinfo: - user, pwd = userinfo.split(":", 1) - return (urllib_unquote(user), urllib_unquote(pwd)) - return urllib_unquote(userinfo), None - return None, None - class LocalFSAdapter(BaseAdapter): diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index c5a46a25565..1415ca06089 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -25,6 +25,7 @@ from pip._vendor.six import PY2 from pip._vendor.six.moves import input from pip._vendor.six.moves.urllib import parse as urllib_parse +from pip._vendor.six.moves.urllib.parse import unquote as urllib_unquote from pip._internal.exceptions import CommandError, InstallationError from pip._internal.locations import ( @@ -878,10 +879,14 @@ def split_auth_from_netloc(netloc): # Split from the left because that's how urllib.parse.urlsplit() # behaves if more than one : is present (which again can be checked # using the password attribute of the return value) - user_pass = tuple(auth.split(':', 1)) + user_pass = auth.split(':', 1) else: user_pass = auth, None + user_pass = tuple( + None if x is None else urllib_unquote(x) for x in user_pass + ) + return netloc, user_pass @@ -895,7 +900,7 @@ def redact_netloc(netloc): if user is None: return netloc password = '' if password is None else ':****' - return '{user}{password}@{netloc}'.format(user=user, + return '{user}{password}@{netloc}'.format(user=urllib_parse.quote(user), password=password, netloc=netloc) diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index c575b498dc5..532eb3fc3eb 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -10,8 +10,8 @@ import pip from pip._internal.download import ( - MultiDomainBasicAuth, PipSession, SafeFileCache, path_to_url, - unpack_file_url, unpack_http_url, url_to_path, + PipSession, SafeFileCache, path_to_url, unpack_file_url, unpack_http_url, + url_to_path, ) from pip._internal.exceptions import HashMismatch from pip._internal.models.link import Link @@ -328,14 +328,3 @@ def test_insecure_host_cache_is_not_enabled(self, tmpdir): ) assert not hasattr(session.adapters["https://example.com/"], "cache") - - -def test_parse_credentials(): - auth = MultiDomainBasicAuth() - assert auth.parse_credentials("foo:bar@example.com") == ('foo', 'bar') - assert auth.parse_credentials("foo@example.com") == ('foo', None) - assert auth.parse_credentials("example.com") == (None, None) - - # URL-encoded reserved characters: - assert auth.parse_credentials("user%3Aname:%23%40%5E@example.com") \ - == ("user:name", "#@^") diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6aab631c2b2..89af7f4e589 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -665,6 +665,9 @@ def test_make_vcs_requirement_url(args, expected): ('user:pass@word@example.com', ('example.com', ('user', 'pass@word'))), # Test the password containing a : symbol. ('user:pass:word@example.com', ('example.com', ('user', 'pass:word'))), + # Test URL-encoded reserved characters. + ('user%3Aname:%23%40%5E@example.com', + ('example.com', ('user:name', '#@^'))), ]) def test_split_auth_from_netloc(netloc, expected): actual = split_auth_from_netloc(netloc) @@ -684,6 +687,8 @@ def test_split_auth_from_netloc(netloc, expected): ('user:pass@word@example.com', 'user:****@example.com'), # Test the password containing a : symbol. ('user:pass:word@example.com', 'user:****@example.com'), + # Test URL-encoded reserved characters. + ('user%3Aname:%23%40%5E@example.com', 'user%3Aname:****@example.com'), ]) def test_redact_netloc(netloc, expected): actual = redact_netloc(netloc) @@ -715,7 +720,10 @@ def test_remove_auth_from_url(auth_url, expected_url): ('https://user@example.com/abc', 'https://user@example.com/abc'), ('https://user:password@example.com', 'https://user:****@example.com'), ('https://user:@example.com', 'https://user:****@example.com'), - ('https://example.com', 'https://example.com') + ('https://example.com', 'https://example.com'), + # Test URL-encoded reserved characters. + ('https://user%3Aname:%23%40%5E@example.com', + 'https://user%3Aname:****@example.com'), ]) def test_redact_password_from_url(auth_url, expected_url): url = redact_password_from_url(auth_url) diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 61661d61f44..c63c17ec4ed 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -192,6 +192,9 @@ def test_git__get_netloc_and_auth(args, expected): (('user@example.com', 'https'), ('example.com', ('user', None))), # Test https with username and password. (('user:pass@example.com', 'https'), ('example.com', ('user', 'pass'))), + # Test https with URL-encoded reserved characters. + (('user%3Aname:%23%40%5E@example.com', 'https'), + ('example.com', ('user:name', '#@^'))), # Test ssh with username and password. (('user:pass@example.com', 'ssh'), ('user:pass@example.com', (None, None))),