Merge pull request #5968 from cjerdonek/dry-up-parse-credentials

Use split_auth_from_netloc() inside MultiDomainBasicAuth

Author: pradyunsg

news/5968.bugfix

@@ -0,0 +1 @@
+Percent-decode special characters in SVN URL credentials.

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,18 +141,18 @@ 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:])
 
         # 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):
 

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 (
@@ -880,10 +881,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
 
 
@@ -897,7 +902,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)
 

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", "#@^")

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)

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))),