Merge pull request #4899 from pypa/fix/reject-illegal-requirements-txt

Fix the index parsing to reject illegal requirements

news/4899.bugfix.rst

@@ -0,0 +1 @@
+Fix the index parsing to reject illegal requirements.txt.

pipenv/core.py

@@ -163,9 +163,13 @@ def import_requirements(project, r=None, dev=False):
     trusted_hosts = []
     # Find and add extra indexes.
     for line in contents.split("\n"):
-        line_indexes, _trusted_hosts, _ = parse_indexes(line.strip())
-        indexes.extend(line_indexes)
-        trusted_hosts.extend(_trusted_hosts)
+        index, extra_index, trusted_host, _ = parse_indexes(line.strip(), strict=True)
+        if index:
+            indexes = [index]
+        if extra_index:
+            indexes.append(extra_index)
+        if trusted_host:
+            trusted_hosts.append(trusted_host)
     indexes = sorted(set(indexes))
     trusted_hosts = sorted(set(trusted_hosts))
     reqs = [install_req_from_parsed_requirement(f) for f in parse_requirements(r, session=pip_requests)]

pipenv/patched/notpip/_internal/build_env.py

@@ -17,7 +17,7 @@
 from pipenv.patched.notpip._vendor.packaging.requirements import Requirement
 from pipenv.patched.notpip._vendor.packaging.version import Version
 
-from pipenv.patched.notpip import __file__ as pip_location
+from pip import __file__ as pip_location
 from pipenv.patched.notpip._internal.cli.spinners import open_spinner
 from pipenv.patched.notpip._internal.locations import get_platlib, get_prefixed_libs, get_purelib
 from pipenv.patched.notpip._internal.metadata import get_environment

pipenv/resolver.py

@@ -649,7 +649,7 @@ def parse_packages(packages, pre, clear, system, requirements_dir=None):
     from pipenv.utils import parse_indexes
     parsed_packages = []
     for package in packages:
-        indexes, trusted_hosts, line = parse_indexes(package)
+        *_, line = parse_indexes(package)
         line = " ".join(line)
         pf = dict()
         req = Requirement.from_line(line)

pipenv/utils.py

@@ -521,10 +521,7 @@ def parse_line(
         if project is None:
             from .project import Project
             project = Project()
-        url = None
-        indexes, trusted_hosts, remainder = parse_indexes(line)
-        if indexes:
-            url = indexes[0]
+        index, extra_index, trust_host, remainder = parse_indexes(line)
         line = " ".join(remainder)
         req = None  # type: Requirement
         try:
@@ -539,10 +536,10 @@ def parse_line(
                     raise ResolutionFailure(f"Failed to resolve requirement from line: {line!s}")
             else:
                 raise ResolutionFailure(f"Failed to resolve requirement from line: {line!s}")
-        if url:
+        if index:
             try:
                 index_lookup[req.normalized_name] = project.get_source(
-                    url=url, refresh=True).get("name")
+                    url=index, refresh=True).get("name")
             except TypeError:
                 pass
         try:
@@ -556,12 +553,6 @@ def parse_line(
             markers_lookup[req.normalized_name] = req.markers.replace('"', "'")
         return req, index_lookup, markers_lookup
 
-    @classmethod
-    def get_deps_from_line(cls, line):
-        # type: (str) -> Tuple[Set[str], Dict[str, Dict[str, Union[str, bool, List[str]]]]]
-        req, _, _ = cls.parse_line(line)
-        return cls.get_deps_from_req(req)
-
     @classmethod
     def get_deps_from_req(cls, req, resolver=None, resolve_vcs=True):
         # type: (Requirement, Optional["Resolver"], bool) -> Tuple[Set[str], Dict[str, Dict[str, Union[str, bool, List[str]]]]]
@@ -725,7 +716,7 @@ def pip_command(self):
             self._pip_command = self._get_pip_command()
         return self._pip_command
 
-    def prepare_pip_args(self, use_pep517=False, build_isolation=True):
+    def prepare_pip_args(self, use_pep517=None, build_isolation=True):
         pip_args = []
         if self.sources:
             pip_args = prepare_pip_source_args(self.sources, pip_args)
@@ -839,7 +830,6 @@ def get_resolver(self, clear=False):
         )
 
         with global_tempdir_manager(), get_requirement_tracker() as req_tracker, TemporaryDirectory(suffix="-build", prefix="pipenv-") as directory:
-            os.environ["PIP_USE_PEP517"] = "false"
             pip_options = self.pip_options
             finder = self.finder
             wheel_cache = WheelCache(pip_options.cache_dir, pip_options.format_control)
@@ -2058,24 +2048,24 @@ def looks_like_dir(path):
     return any(sep in path for sep in seps)
 
 
-def parse_indexes(line):
+def parse_indexes(line, strict=False):
     from argparse import ArgumentParser
+
+    comment_re = re.compile(r"(?:^|\s+)#.*$")
+    line = comment_re.sub("", line)
     parser = ArgumentParser("indexes")
-    parser.add_argument(
-        "--index", "-i", "--index-url",
-        metavar="index_url", action="store", nargs="?",
-    )
-    parser.add_argument(
-        "--extra-index-url", "--extra-index",
-        metavar="extra_indexes", action="append",
-    )
-    parser.add_argument("--trusted-host", metavar="trusted_hosts", action="append")
+    parser.add_argument("-i", "--index-url", dest="index")
+    parser.add_argument("--extra-index-url", dest="extra_index")
+    parser.add_argument("--trusted-host", dest="trusted_host")
     args, remainder = parser.parse_known_args(line.split())
-    index = [] if not args.index else [args.index]
-    extra_indexes = [] if not args.extra_index_url else args.extra_index_url
-    indexes = index + extra_indexes
-    trusted_hosts = args.trusted_host if args.trusted_host else []
-    return indexes, trusted_hosts, remainder
+    index = args.index
+    extra_index = args.extra_index
+    trusted_host = args.trusted_host
+    if strict and sum(
+        bool(arg) for arg in (index, extra_index, trusted_host, remainder)
+    ) > 1:
+        raise ValueError("Index arguments must be on their own lines.")
+    return index, extra_index, trusted_host, remainder
 
 
 @contextmanager

tasks/vendoring/patches/patched/_post_pip_import.patch

@@ -26,3 +26,16 @@ index 0ba06c52..6fdb59b7 100644
 
 
  class LinkCandidate(_InstallRequirementBackedCandidate):
+diff --git a/pipenv/patched/notpip/_internal/build_env.py b/pipenv/patched/notpip/_internal/build_env.py
+index 05457c5a..d8c66b3f 100644
+--- a/pipenv/patched/notpip/_internal/build_env.py
++++ b/pipenv/patched/notpip/_internal/build_env.py
+@@ -17,7 +17,7 @@ from pipenv.patched.notpip._vendor.certifi import where
+ from pipenv.patched.notpip._vendor.packaging.requirements import Requirement
+ from pipenv.patched.notpip._vendor.packaging.version import Version
+
+-from pipenv.patched.notpip import __file__ as pip_location
++from pip import __file__ as pip_location
+ from pipenv.patched.notpip._internal.cli.spinners import open_spinner
+ from pipenv.patched.notpip._internal.locations import get_platlib, get_prefixed_libs, get_purelib
+ from pipenv.patched.notpip._internal.metadata import get_environment

tests/integration/test_install_basic.py

@@ -285,18 +285,23 @@ def test_requirements_to_pipfile(PipenvInstance, pypi):
 
         # Write a requirements file
         with open("requirements.txt", "w") as f:
-            f.write(f"-i {pypi.url}\nrequests[socks]==2.19.1\n")
+            f.write(
+                f"-i {pypi.url}\n"
+                "# -i https://private.pypi.org/simple\n"
+                "requests[socks]==2.19.1\n"
+            )
 
         c = p.pipenv("install")
         assert c.returncode == 0
         print(c.stdout)
         print(c.stderr)
-        print(subprocess_run(["ls", "-l"]).stdout)
-
         # assert stuff in pipfile
         assert "requests" in p.pipfile["packages"]
         assert "extras" in p.pipfile["packages"]["requests"]
-
+        assert not any(
+            source['url'] == 'https://private.pypi.org/simple'
+            for source in p.pipfile['source']
+        )
         # assert stuff in lockfile
         assert "requests" in p.lockfile["default"]
         assert "chardet" in p.lockfile["default"]

tests/unit/test_utils.py

@@ -137,6 +137,29 @@ def test_convert_deps_to_pip_unicode():
     assert deps[0] == "django==1.10"
 
 
+@pytest.mark.parametrize("line,result", [
+    ("-i https://example.com/simple/", ("https://example.com/simple/", None, None, [])),
+    ("--extra-index-url=https://example.com/simple/", (None, "https://example.com/simple/", None, [])),
+    ("--trusted-host=example.com", (None, None, "example.com", [])),
+    ("# -i https://example.com/simple/", (None, None, None, [])),
+    ("requests", (None, None, None, ["requests"]))
+])
+@pytest.mark.utils
+def test_parse_indexes(line, result):
+    assert pipenv.utils.parse_indexes(line) == result
+
+
+@pytest.mark.parametrize("line", [
+    "-i https://example.com/simple/ --extra-index-url=https://extra.com/simple/",
+    "--extra-index-url https://example.com/simple/ --trusted-host=example.com",
+    "requests -i https://example.com/simple/",
+])
+@pytest.mark.utils
+def test_parse_indexes_individual_lines(line):
+    with pytest.raises(ValueError):
+        pipenv.utils.parse_indexes(line, strict=True)
+
+
 class TestUtils:
     """Test utility functions in pipenv"""