Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the index parsing to reject illegal requirements #4899

Merged
merged 4 commits into from
Dec 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/4899.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix the index parsing to reject illegal requirements.txt.
10 changes: 7 additions & 3 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion pipenv/patched/notpip/_internal/build_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pipenv/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 19 additions & 29 deletions pipenv/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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]]]]]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions tasks/vendoring/patches/patched/_post_pip_import.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 9 additions & 4 deletions tests/integration/test_install_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""

Expand Down