Merge pull request #6245 from nicolasbock/ipv6_uri

Fix the URL quoting in _clean_link() for IPv6 addresses

Author: cjerdonek

news/6285.bugfix

@@ -0,0 +1 @@
+Fix incorrect URL quoting of IPv6 addresses.

src/pip/_internal/index.py

@@ -939,15 +939,28 @@ def _get_encoding_from_headers(headers):
     return None
 
 
-_CLEAN_LINK_RE = re.compile(r'[^a-z0-9$&+,/:;=?@.#%_\\|-]', re.I)
-
-
 def _clean_link(url):
     # type: (str) -> str
     """Makes sure a link is fully encoded.  That is, if a ' ' shows up in
     the link, it will be rewritten to %20 (while not over-quoting
     % or other characters)."""
-    return _CLEAN_LINK_RE.sub(lambda match: '%%%2x' % ord(match.group(0)), url)
+    # Split the URL into parts according to the general structure
+    # `scheme://netloc/path;parameters?query#fragment`. Note that the
+    # `netloc` can be empty and the URI will then refer to a local
+    # filesystem path.
+    result = urllib_parse.urlparse(url)
+    # In both cases below we unquote prior to quoting to make sure
+    # nothing is double quoted.
+    if result.netloc == "":
+        # On Windows the path part might contain a drive letter which
+        # should not be quoted. On Linux where drive letters do not
+        # exist, the colon should be quoted. We rely on urllib.request
+        # to do the right thing here.
+        path = urllib_request.pathname2url(
+            urllib_request.url2pathname(result.path))
+    else:
+        path = urllib_parse.quote(urllib_parse.unquote(result.path))
+    return urllib_parse.urlunparse(result._replace(path=path))
 
 
 class HTMLPage(object):

tests/unit/test_index.py

@@ -7,7 +7,7 @@
 
 from pip._internal.download import PipSession
 from pip._internal.index import (
-    Link, PackageFinder, _determine_base_url, _egg_info_matches,
+    Link, PackageFinder, _clean_link, _determine_base_url, _egg_info_matches,
     _find_name_version_sep, _get_html_page,
 )
 
@@ -280,3 +280,65 @@ def test_request_retries(caplog):
         'Could not fetch URL http://localhost: Retry error - skipping'
         in caplog.text
     )
+
+
+@pytest.mark.parametrize(
+    ("url", "clean_url"),
+    [
+        # URL with hostname and port. Port separator should not be quoted.
+        ("https://localhost.localdomain:8181/path/with space/",
+         "https://localhost.localdomain:8181/path/with%20space/"),
+        # URL that is already properly quoted. The quoting `%`
+        # characters should not be quoted again.
+        ("https://localhost.localdomain:8181/path/with%20quoted%20space/",
+         "https://localhost.localdomain:8181/path/with%20quoted%20space/"),
+        # URL with IPv4 address and port.
+        ("https://127.0.0.1:8181/path/with space/",
+         "https://127.0.0.1:8181/path/with%20space/"),
+        # URL with IPv6 address and port. The `[]` brackets around the
+        # IPv6 address should not be quoted.
+        ("https://[fd00:0:0:236::100]:8181/path/with space/",
+         "https://[fd00:0:0:236::100]:8181/path/with%20space/"),
+        # URL with query. The leading `?` should not be quoted.
+        ("https://localhost.localdomain:8181/path/with/query?request=test",
+         "https://localhost.localdomain:8181/path/with/query?request=test"),
+        # URL with colon in the path portion.
+        ("https://localhost.localdomain:8181/path:/with:/colon",
+         "https://localhost.localdomain:8181/path%3A/with%3A/colon"),
+        # URL with something that looks like a drive letter, but is
+        # not. The `:` should be quoted.
+        ("https://localhost.localdomain/T:/path/",
+         "https://localhost.localdomain/T%3A/path/")
+    ]
+)
+def test_clean_link(url, clean_url):
+    assert(_clean_link(url) == clean_url)
+
+
+@pytest.mark.parametrize(
+    ("url", "clean_url"),
+    [
+        # URL with Windows drive letter. The `:` after the drive
+        # letter should not be quoted. The trailing `/` should be
+        # removed.
+        ("file:///T:/path/with spaces/",
+         "file:///T:/path/with%20spaces")
+    ]
+)
+@pytest.mark.skipif("sys.platform != 'win32'")
+def test_clean_link_windows(url, clean_url):
+    assert(_clean_link(url) == clean_url)
+
+
+@pytest.mark.parametrize(
+    ("url", "clean_url"),
+    [
+        # URL with Windows drive letter, running on non-windows
+        # platform. The `:` after the drive should be quoted.
+        ("file:///T:/path/with spaces/",
+         "file:///T%3A/path/with%20spaces/")
+    ]
+)
+@pytest.mark.skipif("sys.platform == 'win32'")
+def test_clean_link_non_windows(url, clean_url):
+    assert(_clean_link(url) == clean_url)