From 8ab041ca58cfdb3ebf16278ee51ffdb0eced964b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Wed, 5 Apr 2023 00:00:22 +0200 Subject: [PATCH 1/2] Refactor link hash checking --- src/pip/_internal/cli/req_command.py | 3 ++ src/pip/_internal/index/package_finder.py | 2 +- src/pip/_internal/operations/prepare.py | 4 +-- src/pip/_internal/req/constructors.py | 12 ++++++++ src/pip/_internal/req/req_install.py | 28 ++----------------- .../_internal/resolution/resolvelib/base.py | 4 +-- .../resolution/resolvelib/factory.py | 2 +- 7 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index c2f4e38bed8..5aaf69c897a 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -404,6 +404,7 @@ def get_requirements( parsed_req, isolated=options.isolated_mode, user_supplied=False, + trust_link_hash=True, ) requirements.append(req_to_add) @@ -415,6 +416,7 @@ def get_requirements( use_pep517=options.use_pep517, user_supplied=True, config_settings=getattr(options, "config_settings", None), + trust_link_hash=True, ) requirements.append(req_to_add) @@ -441,6 +443,7 @@ def get_requirements( config_settings=parsed_req.options.get("config_settings") if parsed_req.options else None, + trust_link_hash=True, ) requirements.append(req_to_add) diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py index b6f8d57e854..cdfbc369f8e 100644 --- a/src/pip/_internal/index/package_finder.py +++ b/src/pip/_internal/index/package_finder.py @@ -904,7 +904,7 @@ def find_requirement( Returns a InstallationCandidate if found, Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise """ - hashes = req.hashes(trust_internet=False) + hashes = req.hashes() best_candidate_result = self.find_best_candidate( req.name, specifier=req.specifier, diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 2273315234d..477d84130f0 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -336,7 +336,7 @@ def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes: # (For example, we can raise VcsHashUnsupported for a VCS URL # rather than HashMissing.) if not self.require_hashes: - return req.hashes(trust_internet=True) + return req.hashes() # We could check these first 2 conditions inside unpack_url # and save repetition of conditions, but then we would @@ -359,7 +359,7 @@ def _get_linked_req_hashes(self, req: InstallRequirement) -> Hashes: # shim it with a facade object that will provoke hash # computation and then raise a HashMissing exception # showing the user what the hash should be. - return req.hashes(trust_internet=False) or MissingHashes() + return req.hashes() or MissingHashes() def _fetch_metadata_only( self, diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index c5ca2d85d51..ca6634a2eb4 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -8,6 +8,7 @@ InstallRequirement. """ +import copy import logging import os import re @@ -385,15 +386,24 @@ def install_req_from_line( line_source: Optional[str] = None, user_supplied: bool = False, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, + trust_link_hash: bool = False, ) -> InstallRequirement: """Creates an InstallRequirement from a name, which might be a requirement, directory containing 'setup.py', filename, or URL. :param line_source: An optional string describing where the line is from, for logging purposes in case of an error. + :param trust_link_hash: If True, consider hashes provided as URL fragments + are trusted on the same foot as hases provided as --hash options. """ parts = parse_req_from_line(name, line_source) + # + if parts.link and parts.link.hash and trust_link_hash: + assert parts.link.hash_name + hash_options = copy.deepcopy(hash_options) or {} + hash_options.setdefault(parts.link.hash_name, []).append(parts.link.hash) + return InstallRequirement( parts.requirement, comes_from, @@ -454,6 +464,7 @@ def install_req_from_parsed_requirement( use_pep517: Optional[bool] = None, user_supplied: bool = False, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, + trust_link_hash: bool = False, ) -> InstallRequirement: if parsed_req.is_editable: req = install_req_from_editable( @@ -484,6 +495,7 @@ def install_req_from_parsed_requirement( line_source=parsed_req.line_source, user_supplied=user_supplied, config_settings=config_settings, + trust_link_hash=trust_link_hash, ) return req diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index d01b24a9189..82ddb8fd41c 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -275,31 +275,9 @@ def has_hash_options(self) -> bool: """ return bool(self.hash_options) - def hashes(self, trust_internet: bool = True) -> Hashes: - """Return a hash-comparer that considers my option- and URL-based - hashes to be known-good. - - Hashes in URLs--ones embedded in the requirements file, not ones - downloaded from an index server--are almost peers with ones from - flags. They satisfy --require-hashes (whether it was implicitly or - explicitly activated) but do not activate it. md5 and sha224 are not - allowed in flags, which should nudge people toward good algos. We - always OR all hashes together, even ones from URLs. - - :param trust_internet: Whether to trust URL-based (#md5=...) hashes - downloaded from the internet, as by populate_link() - - """ - good_hashes = self.hash_options.copy() - if trust_internet: - link = self.link - elif self.original_link and self.user_supplied: - link = self.original_link - else: - link = None - if link and link.hash: - good_hashes.setdefault(link.hash_name, []).append(link.hash) - return Hashes(good_hashes) + def hashes(self) -> Hashes: + """Return a hash-comparer that considers my option-hashes to be known-good.""" + return Hashes(self.hash_options) def from_path(self) -> Optional[str]: """Format a nice indicator to show where this "comes from" """ diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index b206692a0a9..b26b9543099 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -34,7 +34,7 @@ def empty(cls) -> "Constraint": @classmethod def from_ireq(cls, ireq: InstallRequirement) -> "Constraint": links = frozenset([ireq.link]) if ireq.link else frozenset() - return Constraint(ireq.specifier, ireq.hashes(trust_internet=False), links) + return Constraint(ireq.specifier, ireq.hashes(), links) def __bool__(self) -> bool: return bool(self.specifier) or bool(self.hashes) or bool(self.links) @@ -43,7 +43,7 @@ def __and__(self, other: InstallRequirement) -> "Constraint": if not isinstance(other, InstallRequirement): return NotImplemented specifier = self.specifier & other.specifier - hashes = self.hashes & other.hashes(trust_internet=False) + hashes = self.hashes & other.hashes() links = self.links if other.link: links = links.union([other.link]) diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 0331297b85b..bd943b10f29 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -248,7 +248,7 @@ def _iter_found_candidates( for ireq in ireqs: assert ireq.req, "Candidates found on index must be PEP 508" specifier &= ireq.req.specifier - hashes &= ireq.hashes(trust_internet=False) + hashes &= ireq.hashes() extras |= frozenset(ireq.extras) def _get_installed_candidate() -> Optional[Candidate]: From 87a49913cc057d81e62cb1e11656d3f927d4e5f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Wed, 5 Apr 2023 15:20:17 +0200 Subject: [PATCH 2/2] Rename hash_options to trusted_hashes This better reflects the purpose. --- src/pip/_internal/req/constructors.py | 16 ++++++++-------- src/pip/_internal/req/req_install.py | 8 ++++---- .../resolution/resolvelib/candidates.py | 6 +++--- tests/unit/test_req_file.py | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index ca6634a2eb4..a1d12a3dfe4 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -206,7 +206,7 @@ def install_req_from_editable( use_pep517: Optional[bool] = None, isolated: bool = False, global_options: Optional[List[str]] = None, - hash_options: Optional[Dict[str, List[str]]] = None, + trusted_hashes: Optional[Dict[str, List[str]]] = None, constraint: bool = False, user_supplied: bool = False, permit_editable_wheels: bool = False, @@ -225,7 +225,7 @@ def install_req_from_editable( use_pep517=use_pep517, isolated=isolated, global_options=global_options, - hash_options=hash_options, + trusted_hashes=trusted_hashes, config_settings=config_settings, extras=parts.extras, ) @@ -381,7 +381,7 @@ def install_req_from_line( use_pep517: Optional[bool] = None, isolated: bool = False, global_options: Optional[List[str]] = None, - hash_options: Optional[Dict[str, List[str]]] = None, + trusted_hashes: Optional[Dict[str, List[str]]] = None, constraint: bool = False, line_source: Optional[str] = None, user_supplied: bool = False, @@ -401,8 +401,8 @@ def install_req_from_line( # if parts.link and parts.link.hash and trust_link_hash: assert parts.link.hash_name - hash_options = copy.deepcopy(hash_options) or {} - hash_options.setdefault(parts.link.hash_name, []).append(parts.link.hash) + trusted_hashes = copy.deepcopy(trusted_hashes) or {} + trusted_hashes.setdefault(parts.link.hash_name, []).append(parts.link.hash) return InstallRequirement( parts.requirement, @@ -412,7 +412,7 @@ def install_req_from_line( use_pep517=use_pep517, isolated=isolated, global_options=global_options, - hash_options=hash_options, + trusted_hashes=trusted_hashes, config_settings=config_settings, constraint=constraint, extras=parts.extras, @@ -488,7 +488,7 @@ def install_req_from_parsed_requirement( if parsed_req.options else [] ), - hash_options=( + trusted_hashes=( parsed_req.options.get("hashes", {}) if parsed_req.options else {} ), constraint=parsed_req.constraint, @@ -512,7 +512,7 @@ def install_req_from_link_and_ireq( use_pep517=ireq.use_pep517, isolated=ireq.isolated, global_options=ireq.global_options, - hash_options=ireq.hash_options, + trusted_hashes=ireq.trusted_hashes, config_settings=ireq.config_settings, user_supplied=ireq.user_supplied, ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 82ddb8fd41c..a627ba59447 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -79,7 +79,7 @@ def __init__( isolated: bool = False, *, global_options: Optional[List[str]] = None, - hash_options: Optional[Dict[str, List[str]]] = None, + trusted_hashes: Optional[Dict[str, List[str]]] = None, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, constraint: bool = False, extras: Collection[str] = (), @@ -144,7 +144,7 @@ def __init__( self.install_succeeded: Optional[bool] = None # Supplied options self.global_options = global_options if global_options else [] - self.hash_options = hash_options if hash_options else {} + self.trusted_hashes = trusted_hashes if trusted_hashes else {} self.config_settings = config_settings # Set to True after successful preparation of this requirement self.prepared = False @@ -273,11 +273,11 @@ def has_hash_options(self) -> bool: URL do not. """ - return bool(self.hash_options) + return bool(self.trusted_hashes) def hashes(self) -> Hashes: """Return a hash-comparer that considers my option-hashes to be known-good.""" - return Hashes(self.hash_options) + return Hashes(self.trusted_hashes) def from_path(self) -> Optional[str]: """Format a nice indicator to show where this "comes from" """ diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 31020e27ad1..b1c2d491346 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -66,7 +66,7 @@ def make_install_req_from_link( isolated=template.isolated, constraint=template.constraint, global_options=template.global_options, - hash_options=template.hash_options, + trusted_hashes=template.trusted_hashes, config_settings=template.config_settings, ) ireq.original_link = template.original_link @@ -88,7 +88,7 @@ def make_install_req_from_editable( constraint=template.constraint, permit_editable_wheels=template.permit_editable_wheels, global_options=template.global_options, - hash_options=template.hash_options, + trusted_hashes=template.trusted_hashes, config_settings=template.config_settings, ) ireq.extras = template.extras @@ -112,7 +112,7 @@ def _make_install_req_from_dist( isolated=template.isolated, constraint=template.constraint, global_options=template.global_options, - hash_options=template.hash_options, + trusted_hashes=template.trusted_hashes, config_settings=template.config_settings, ) ireq.satisfied_by = dist diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index 439c41563b7..95b67d47961 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -374,7 +374,7 @@ def test_hash_options(self, line_processor: LineProcessor) -> None: ) filename = "filename" req = line_processor(line, filename, 1)[0] - assert req.hash_options == { + assert req.trusted_hashes == { "sha256": [ "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824", "486ea46224d1bb4fb680f34f7c9ad96a8f24ec88be73ea8e5a6c65260e9cb8a7",