Merge pull request #6635 from cjerdonek/link-parsed-attribute

Parse the url when creating a Link object

src/pip/_internal/models/link.py

@@ -4,7 +4,8 @@
 from pip._vendor.six.moves.urllib import parse as urllib_parse
 
 from pip._internal.utils.misc import (
-    WHEEL_EXTENSION, path_to_url, redact_password_from_url, splitext,
+    WHEEL_EXTENSION, path_to_url, redact_password_from_url,
+    split_auth_from_netloc, splitext,
 )
 from pip._internal.utils.models import KeyBasedCompareMixin
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
@@ -35,52 +36,67 @@ def __init__(self, url, comes_from=None, requires_python=None):
         if url.startswith('\\\\'):
             url = path_to_url(url)
 
-        self.url = url
+        self._parsed_url = urllib_parse.urlsplit(url)
+        # Store the url as a private attribute to prevent accidentally
+        # trying to set a new value.
+        self._url = url
+
         self.comes_from = comes_from
         self.requires_python = requires_python if requires_python else None
 
-        super(Link, self).__init__(
-            key=(self.url),
-            defining_class=Link
-        )
+        super(Link, self).__init__(key=url, defining_class=Link)
 
     def __str__(self):
         if self.requires_python:
             rp = ' (requires-python:%s)' % self.requires_python
         else:
             rp = ''
         if self.comes_from:
-            return '%s (from %s)%s' % (redact_password_from_url(self.url),
+            return '%s (from %s)%s' % (redact_password_from_url(self._url),
                                        self.comes_from, rp)
         else:
-            return redact_password_from_url(str(self.url))
+            return redact_password_from_url(str(self._url))
 
     def __repr__(self):
         return '<Link %s>' % self
 
+    @property
+    def url(self):
+        # type: () -> str
+        return self._url
+
     @property
     def filename(self):
         # type: () -> str
-        _, netloc, path, _, _ = urllib_parse.urlsplit(self.url)
-        name = posixpath.basename(path.rstrip('/')) or netloc
+        path = self.path.rstrip('/')
+        name = posixpath.basename(path)
+        if not name:
+            # Make sure we don't leak auth information if the netloc
+            # includes a username and password.
+            netloc, user_pass = split_auth_from_netloc(self.netloc)
+            return netloc
+
         name = urllib_parse.unquote(name)
-        assert name, ('URL %r produced no filename' % self.url)
+        assert name, ('URL %r produced no filename' % self._url)
         return name
 
     @property
     def scheme(self):
         # type: () -> str
-        return urllib_parse.urlsplit(self.url)[0]
+        return self._parsed_url[0]
 
     @property
     def netloc(self):
         # type: () -> str
-        return urllib_parse.urlsplit(self.url)[1]
+        """
+        This can contain auth information.
+        """
+        return self._parsed_url[1]
 
     @property
     def path(self):
         # type: () -> str
-        return urllib_parse.unquote(urllib_parse.urlsplit(self.url)[2])
+        return urllib_parse.unquote(self._parsed_url[2])
 
     def splitext(self):
         # type: () -> Tuple[str, str]
@@ -94,15 +110,15 @@ def ext(self):
     @property
     def url_without_fragment(self):
         # type: () -> str
-        scheme, netloc, path, query, fragment = urllib_parse.urlsplit(self.url)
+        scheme, netloc, path, query, fragment = self._parsed_url
         return urllib_parse.urlunsplit((scheme, netloc, path, query, None))
 
     _egg_fragment_re = re.compile(r'[#&]egg=([^&]*)')
 
     @property
     def egg_fragment(self):
         # type: () -> Optional[str]
-        match = self._egg_fragment_re.search(self.url)
+        match = self._egg_fragment_re.search(self._url)
         if not match:
             return None
         return match.group(1)
@@ -112,7 +128,7 @@ def egg_fragment(self):
     @property
     def subdirectory_fragment(self):
         # type: () -> Optional[str]
-        match = self._subdirectory_fragment_re.search(self.url)
+        match = self._subdirectory_fragment_re.search(self._url)
         if not match:
             return None
         return match.group(1)
@@ -124,23 +140,23 @@ def subdirectory_fragment(self):
     @property
     def hash(self):
         # type: () -> Optional[str]
-        match = self._hash_re.search(self.url)
+        match = self._hash_re.search(self._url)
         if match:
             return match.group(2)
         return None
 
     @property
     def hash_name(self):
         # type: () -> Optional[str]
-        match = self._hash_re.search(self.url)
+        match = self._hash_re.search(self._url)
         if match:
             return match.group(1)
         return None
 
     @property
     def show_url(self):
         # type: () -> Optional[str]
-        return posixpath.basename(self.url.split('#', 1)[0].split('?', 1)[0])
+        return posixpath.basename(self._url.split('#', 1)[0].split('?', 1)[0])
 
     @property
     def is_wheel(self):

tests/unit/test_download.py

@@ -366,9 +366,10 @@ def test_unpack_file_url_bad_hash(self, tmpdir, data,
         Test when the file url hash fragment is wrong
         """
         self.prep(tmpdir, data)
-        self.dist_url.url = "%s#md5=bogus" % self.dist_url.url
+        url = '{}#md5=bogus'.format(self.dist_url.url)
+        dist_url = Link(url)
         with pytest.raises(HashMismatch):
-            unpack_file_url(self.dist_url,
+            unpack_file_url(dist_url,
                             self.build_dir,
                             hashes=Hashes({'md5': ['bogus']}))
 
@@ -392,11 +393,9 @@ def test_unpack_file_url_download_bad_hash(self, tmpdir, data,
 
         assert dist_path_md5 != dist_path2_md5
 
-        self.dist_url.url = "%s#md5=%s" % (
-            self.dist_url.url,
-            dist_path_md5
-        )
-        unpack_file_url(self.dist_url, self.build_dir,
+        url = '{}#md5={}'.format(self.dist_url.url, dist_path_md5)
+        dist_url = Link(url)
+        unpack_file_url(dist_url, self.build_dir,
                         download_dir=self.download_dir,
                         hashes=Hashes({'md5': [dist_path_md5]}))
 

tests/unit/test_index.py

@@ -286,55 +286,6 @@ def test_sort_locations_non_existing_path():
     assert not urls and not files, "nothing should have been found"
 
 
-class TestLink(object):
-
-    def test_splitext(self):
-        assert ('wheel', '.whl') == Link('http://yo/wheel.whl').splitext()
-
-    @pytest.mark.parametrize(
-        ("url", "expected"),
-        [
-            ("http://yo/wheel.whl", "wheel.whl"),
-            ("http://yo/wheel", "wheel"),
-            (
-                "http://yo/myproject-1.0%2Bfoobar.0-py2.py3-none-any.whl",
-                "myproject-1.0+foobar.0-py2.py3-none-any.whl",
-            ),
-        ],
-    )
-    def test_filename(self, url, expected):
-        assert Link(url).filename == expected
-
-    def test_no_ext(self):
-        assert '' == Link('http://yo/wheel').ext
-
-    def test_ext(self):
-        assert '.whl' == Link('http://yo/wheel.whl').ext
-
-    def test_ext_fragment(self):
-        assert '.whl' == Link('http://yo/wheel.whl#frag').ext
-
-    def test_ext_query(self):
-        assert '.whl' == Link('http://yo/wheel.whl?a=b').ext
-
-    def test_is_wheel(self):
-        assert Link('http://yo/wheel.whl').is_wheel
-
-    def test_is_wheel_false(self):
-        assert not Link('http://yo/not_a_wheel').is_wheel
-
-    def test_fragments(self):
-        url = 'git+https://example.com/package#egg=eggname'
-        assert 'eggname' == Link(url).egg_fragment
-        assert None is Link(url).subdirectory_fragment
-        url = 'git+https://example.com/package#egg=eggname&subdirectory=subdir'
-        assert 'eggname' == Link(url).egg_fragment
-        assert 'subdir' == Link(url).subdirectory_fragment
-        url = 'git+https://example.com/package#subdirectory=subdir&egg=eggname'
-        assert 'eggname' == Link(url).egg_fragment
-        assert 'subdir' == Link(url).subdirectory_fragment
-
-
 @pytest.mark.parametrize(
     ("html", "url", "expected"),
     [

tests/unit/test_link.py

@@ -0,0 +1,73 @@
+import pytest
+
+from pip._internal.models.link import Link
+
+
+class TestLink:
+
+    @pytest.mark.parametrize('url, expected', [
+        (
+            'https://user:[email protected]/path/page.html',
+            '<Link https://user:****@example.com/path/page.html>',
+        ),
+    ])
+    def test_repr(self, url, expected):
+        link = Link(url)
+        assert repr(link) == expected
+
+    @pytest.mark.parametrize('url, expected', [
+        ('http://yo/wheel.whl', 'wheel.whl'),
+        ('http://yo/wheel', 'wheel'),
+        ('https://example.com/path/page.html', 'page.html'),
+        # Test a quoted character.
+        ('https://example.com/path/page%231.html', 'page#1.html'),
+        (
+            'http://yo/myproject-1.0%2Bfoobar.0-py2.py3-none-any.whl',
+            'myproject-1.0+foobar.0-py2.py3-none-any.whl',
+        ),
+        # Test a path that ends in a slash.
+        ('https://example.com/path/', 'path'),
+        ('https://example.com/path//', 'path'),
+        # Test a url with no filename.
+        ('https://example.com/', 'example.com'),
+        # Test a url with no filename and with auth information.
+        (
+            'https://user:[email protected]/',
+            'example.com',
+        ),
+    ])
+    def test_filename(self, url, expected):
+        link = Link(url)
+        assert link.filename == expected
+
+    def test_splitext(self):
+        assert ('wheel', '.whl') == Link('http://yo/wheel.whl').splitext()
+
+    def test_no_ext(self):
+        assert '' == Link('http://yo/wheel').ext
+
+    def test_ext(self):
+        assert '.whl' == Link('http://yo/wheel.whl').ext
+
+    def test_ext_fragment(self):
+        assert '.whl' == Link('http://yo/wheel.whl#frag').ext
+
+    def test_ext_query(self):
+        assert '.whl' == Link('http://yo/wheel.whl?a=b').ext
+
+    def test_is_wheel(self):
+        assert Link('http://yo/wheel.whl').is_wheel
+
+    def test_is_wheel_false(self):
+        assert not Link('http://yo/not_a_wheel').is_wheel
+
+    def test_fragments(self):
+        url = 'git+https://example.com/package#egg=eggname'
+        assert 'eggname' == Link(url).egg_fragment
+        assert None is Link(url).subdirectory_fragment
+        url = 'git+https://example.com/package#egg=eggname&subdirectory=subdir'
+        assert 'eggname' == Link(url).egg_fragment
+        assert 'subdir' == Link(url).subdirectory_fragment
+        url = 'git+https://example.com/package#subdirectory=subdir&egg=eggname'
+        assert 'eggname' == Link(url).egg_fragment
+        assert 'subdir' == Link(url).subdirectory_fragment