Skip to content

Refactor checking of hashes embedded in user-provided URLs #11923

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
3 changes: 3 additions & 0 deletions src/pip/_internal/cli/req_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/index/package_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
24 changes: 18 additions & 6 deletions src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
InstallRequirement.
"""

import copy
import logging
import os
import re
Expand Down Expand Up @@ -205,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,
Expand All @@ -224,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,
)
Expand Down Expand Up @@ -380,20 +381,29 @@ 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,
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
trusted_hashes = copy.deepcopy(trusted_hashes) or {}
trusted_hashes.setdefault(parts.link.hash_name, []).append(parts.link.hash)

return InstallRequirement(
parts.requirement,
comes_from,
Expand All @@ -402,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,
Expand Down Expand Up @@ -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(
Expand All @@ -477,13 +488,14 @@ 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,
line_source=parsed_req.line_source,
user_supplied=user_supplied,
config_settings=config_settings,
trust_link_hash=trust_link_hash,
)
return req

Expand All @@ -500,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,
)
34 changes: 6 additions & 28 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = (),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -273,33 +273,11 @@ def has_hash_options(self) -> bool:
URL do not.

"""
return bool(self.hash_options)
return bool(self.trusted_hashes)

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.trusted_hashes)

def from_path(self) -> Optional[str]:
"""Format a nice indicator to show where this "comes from" """
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/resolution/resolvelib/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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])
Expand Down
6 changes: 3 additions & 3 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_req_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down