Repository: pypa/pip
Pull request #12659 (2024-04-29 00:24:33)

Convert more record classes to dataclasses
- Removes BestCandidateResult's iter_all() and iter_applicable() methods as they were redundant
- Removes ParsedLine's is_requirement attribute as it was awkward to use (to please mypy, you would need to add asserts on .requirement)


ichard26: @pradyunsg can I kindly request another review from you? :)

(this is a very low priority PR, so please spend your time elsewhere if needed)
ichard26: I've added slots for those two requirement file dataclasses. Unfortunately, `dataclass(slots=True)` is a Python 3.10+ feature, so I've manually set `__slots__` while removing the defaults as they conflict. 
ichard26: \*bump*

This PR is a pure refactoring change and is low-risk. It should be trivial to review.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 3a0319c23..3b3f73e2b 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -2,7 +2,7 @@ exclude: 'src/pip/_vendor/'
 
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.6.0
+  rev: v5.0.0
   hooks:
   - id: check-builtin-literals
   - id: check-added-large-files
@@ -17,18 +17,18 @@ repos:
     exclude: .patch
 
 - repo: https://github.com/psf/black-pre-commit-mirror
-  rev: 24.4.2
+  rev: 24.10.0
   hooks:
   - id: black
 
 - repo: https://github.com/astral-sh/ruff-pre-commit
-  rev: v0.5.6
+  rev: v0.8.2
   hooks:
     - id: ruff
       args: [--fix, --exit-non-zero-on-fix]
 
 - repo: https://github.com/pre-commit/mirrors-mypy
-  rev: v1.12.1
+  rev: v1.13.0
   hooks:
   - id: mypy
     exclude: tests/data
diff --git a/news/13079.bugfix.rst b/news/13079.bugfix.rst
new file mode 100644
index 000000000..5b297f5a1
--- /dev/null
+++ b/news/13079.bugfix.rst
@@ -0,0 +1,1 @@
+This change fixes a security bug allowing a wheel to execute code during installation.
diff --git a/news/c33bb4df-d6ab-4d9b-8113-55c27a237dfd.trivial.rst b/news/c33bb4df-d6ab-4d9b-8113-55c27a237dfd.trivial.rst
new file mode 100644
index 000000000..e69de29bb
diff --git a/src/pip/_internal/cli/progress_bars.py b/src/pip/_internal/cli/progress_bars.py
index 1236180c0..3d9dde8ed 100644
--- a/src/pip/_internal/cli/progress_bars.py
+++ b/src/pip/_internal/cli/progress_bars.py
@@ -63,7 +63,7 @@ def _raw_progress_bar(
     size: Optional[int],
 ) -> Generator[bytes, None, None]:
     def write_progress(current: int, total: int) -> None:
-        sys.stdout.write("Progress %d of %d\n" % (current, total))
+        sys.stdout.write(f"Progress {current} of {total}\n")
         sys.stdout.flush()
 
     current = 0
diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py
index ad45a2f2a..70acf202b 100644
--- a/src/pip/_internal/commands/install.py
+++ b/src/pip/_internal/commands/install.py
@@ -10,6 +10,13 @@ from typing import List, Optional
 from pip._vendor.packaging.utils import canonicalize_name
 from pip._vendor.rich import print_json
 
+# Eagerly import self_outdated_check to avoid crashes. Otherwise,
+# this module would be imported *after* pip was replaced, resulting
+# in crashes if the new self_outdated_check module was incompatible
+# with the rest of pip that's already imported, or allowing a
+# wheel to execute arbitrary code on install by replacing
+# self_outdated_check.
+import pip._internal.self_outdated_check  # noqa: F401
 from pip._internal.cache import WheelCache
 from pip._internal.cli import cmdoptions
 from pip._internal.cli.cmdoptions import make_target_python
@@ -408,12 +415,6 @@ class InstallCommand(RequirementCommand):
                 # If we're not replacing an already installed pip,
                 # we're not modifying it.
                 modifying_pip = pip_req.satisfied_by is None
-                if modifying_pip:
-                    # Eagerly import this module to avoid crashes. Otherwise, this
-                    # module would be imported *after* pip was replaced, resulting in
-                    # crashes if the new self_outdated_check module was incompatible
-                    # with the rest of pip that's already imported.
-                    import pip._internal.self_outdated_check  # noqa: F401
             protect_pip_from_modification_on_windows(modifying_pip=modifying_pip)
 
             reqs_to_build = [
diff --git a/src/pip/_internal/index/package_finder.py b/src/pip/_internal/index/package_finder.py
index 0d65ce35f..8b89e7682 100644
--- a/src/pip/_internal/index/package_finder.py
+++ b/src/pip/_internal/index/package_finder.py
@@ -334,44 +334,30 @@ class CandidatePreferences:
     allow_all_prereleases: bool = False
 
 
+@dataclass(frozen=True)
 class BestCandidateResult:
     """A collection of candidates, returned by `PackageFinder.find_best_candidate`.
 
     This class is only intended to be instantiated by CandidateEvaluator's
     `compute_best_candidate()` method.
-    """
-
-    def __init__(
-        self,
-        candidates: List[InstallationCandidate],
-        applicable_candidates: List[InstallationCandidate],
-        best_candidate: Optional[InstallationCandidate],
-    ) -> None:
-        """
-        :param candidates: A sequence of all available candidates found.
-        :param applicable_candidates: The applicable candidates.
-        :param best_candidate: The most preferred candidate found, or None
-            if no applicable candidates were found.
-        """
-        assert set(applicable_candidates) <= set(candidates)
-
-        if best_candidate is None:
-            assert not applicable_candidates
-        else:
-            assert best_candidate in applicable_candidates
 
-        self._applicable_candidates = applicable_candidates
-        self._candidates = candidates
+    :param all_candidates: A sequence of all available candidates found.
+    :param applicable_candidates: The applicable candidates.
+    :param best_candidate: The most preferred candidate found, or None
+        if no applicable candidates were found.
+    """
 
-        self.best_candidate = best_candidate
+    all_candidates: List[InstallationCandidate]
+    applicable_candidates: List[InstallationCandidate]
+    best_candidate: Optional[InstallationCandidate]
 
-    def iter_all(self) -> Iterable[InstallationCandidate]:
-        """Iterate through all candidates."""
-        return iter(self._candidates)
+    def __post_init__(self) -> None:
+        assert set(self.applicable_candidates) <= set(self.all_candidates)
 
-    def iter_applicable(self) -> Iterable[InstallationCandidate]:
-        """Iterate through the applicable candidates."""
-        return iter(self._applicable_candidates)
+        if self.best_candidate is None:
+            assert not self.applicable_candidates
+        else:
+            assert self.best_candidate in self.applicable_candidates
 
 
 class CandidateEvaluator:
@@ -929,7 +915,7 @@ class PackageFinder:
                 "Could not find a version that satisfies the requirement %s "
                 "(from versions: %s)",
                 req,
-                _format_versions(best_candidate_result.iter_all()),
+                _format_versions(best_candidate_result.all_candidates),
             )
 
             raise DistributionNotFound(f"No matching distribution found for {req}")
@@ -963,7 +949,7 @@ class PackageFinder:
             logger.debug(
                 "Using version %s (newest of versions: %s)",
                 best_candidate.version,
-                _format_versions(best_candidate_result.iter_applicable()),
+                _format_versions(best_candidate_result.applicable_candidates),
             )
             return best_candidate
 
@@ -971,7 +957,7 @@ class PackageFinder:
         logger.debug(
             "Installed version (%s) is most up-to-date (past versions: %s)",
             installed_version,
-            _format_versions(best_candidate_result.iter_applicable()),
+            _format_versions(best_candidate_result.applicable_candidates),
         )
         raise BestVersionAlreadyInstalled
 
diff --git a/src/pip/_internal/operations/freeze.py b/src/pip/_internal/operations/freeze.py
index bb1039fb7..ae5dd37f9 100644
--- a/src/pip/_internal/operations/freeze.py
+++ b/src/pip/_internal/operations/freeze.py
@@ -1,9 +1,10 @@
 import collections
 import logging
 import os
+from dataclasses import dataclass, field
 from typing import Container, Dict, Generator, Iterable, List, NamedTuple, Optional, Set
 
-from pip._vendor.packaging.utils import canonicalize_name
+from pip._vendor.packaging.utils import NormalizedName, canonicalize_name
 from pip._vendor.packaging.version import InvalidVersion
 
 from pip._internal.exceptions import BadCommand, InstallationError
@@ -220,19 +221,16 @@ def _get_editable_info(dist: BaseDistribution) -> _EditableInfo:
     )
 
 
+@dataclass(frozen=True)
 class FrozenRequirement:
-    def __init__(
-        self,
-        name: str,
-        req: str,
-        editable: bool,
-        comments: Iterable[str] = (),
-    ) -> None:
-        self.name = name
-        self.canonical_name = canonicalize_name(name)
-        self.req = req
-        self.editable = editable
-        self.comments = comments
+    name: str
+    req: str
+    editable: bool
+    comments: Iterable[str] = field(default_factory=tuple)
+
+    @property
+    def canonical_name(self) -> NormalizedName:
+        return canonicalize_name(self.name)
 
     @classmethod
     def from_dist(cls, dist: BaseDistribution) -> "FrozenRequirement":
diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py
index eb2a1f699..dee7f2fe8 100644
--- a/src/pip/_internal/req/req_file.py
+++ b/src/pip/_internal/req/req_file.py
@@ -8,6 +8,7 @@ import os
 import re
 import shlex
 import urllib.parse
+from dataclasses import dataclass
 from optparse import Values
 from typing import (
     TYPE_CHECKING,
@@ -84,49 +85,48 @@ SUPPORTED_OPTIONS_EDITABLE_REQ_DEST = [
 logger = logging.getLogger(__name__)
 
 
+@dataclass(frozen=True)
 class ParsedRequirement:
-    def __init__(
-        self,
-        requirement: str,
-        is_editable: bool,
-        comes_from: str,
-        constraint: bool,
-        options: Optional[Dict[str, Any]] = None,
-        line_source: Optional[str] = None,
-    ) -> None:
-        self.requirement = requirement
-        self.is_editable = is_editable
-        self.comes_from = comes_from
-        self.options = options
-        self.constraint = constraint
-        self.line_source = line_source
+    # TODO: replace this with slots=True when dropping Python 3.9 support.
+    __slots__ = (
+        "requirement",
+        "is_editable",
+        "comes_from",
+        "constraint",
+        "options",
+        "line_source",
+    )
+
+    requirement: str
+    is_editable: bool
+    comes_from: str
+    constraint: bool
+    options: Optional[Dict[str, Any]]
+    line_source: Optional[str]
 
 
+@dataclass(frozen=True)
 class ParsedLine:
-    def __init__(
-        self,
-        filename: str,
-        lineno: int,
-        args: str,
-        opts: Values,
-        constraint: bool,
-    ) -> None:
-        self.filename = filename
-        self.lineno = lineno
-        self.opts = opts
-        self.constraint = constraint
-
-        if args:
-            self.is_requirement = True
-            self.is_editable = False
-            self.requirement = args
-        elif opts.editables:
-            self.is_requirement = True
-            self.is_editable = True
+    __slots__ = ("filename", "lineno", "args", "opts", "constraint")
+
+    filename: str
+    lineno: int
+    args: str
+    opts: Values
+    constraint: bool
+
+    @property
+    def is_editable(self) -> bool:
+        return bool(self.opts.editables)
+
+    @property
+    def requirement(self) -> Optional[str]:
+        if self.args:
+            return self.args
+        elif self.is_editable:
             # We don't support multiple -e on one line
-            self.requirement = opts.editables[0]
-        else:
-            self.is_requirement = False
+            return self.opts.editables[0]
+        return None
 
 
 def parse_requirements(
@@ -179,7 +179,7 @@ def handle_requirement_line(
         line.lineno,
     )
 
-    assert line.is_requirement
+    assert line.requirement is not None
 
     # get the options that apply to requirements
     if line.is_editable:
@@ -301,7 +301,7 @@ def handle_line(
     affect the finder.
     """
 
-    if line.is_requirement:
+    if line.requirement is not None:
         parsed_req = handle_requirement_line(line, options)
         return parsed_req
     else:
@@ -340,7 +340,7 @@ class RequirementsFileParser:
         parsed_files_stack: List[Dict[str, Optional[str]]],
     ) -> Generator[ParsedLine, None, None]:
         for line in self._parse_file(filename, constraint):
-            if not line.is_requirement and (
+            if line.requirement is None and (
                 line.opts.requirements or line.opts.constraints
             ):
                 # parse a nested requirements file
diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py
index dc6e2e12e..6c273eb88 100644
--- a/src/pip/_internal/resolution/resolvelib/factory.py
+++ b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -309,7 +309,7 @@ class Factory:
                 specifier=specifier,
                 hashes=hashes,
             )
-            icans = list(result.iter_applicable())
+            icans = result.applicable_candidates
 
             # PEP 592: Yanked releases are ignored unless the specifier
             # explicitly pins a version (via '==' or '===') that can be


Test patch:
diff --git a/tests/unit/test_index.py b/tests/unit/test_index.py
index 8faa4a56a..cbbca0381 100644
--- a/tests/unit/test_index.py
+++ b/tests/unit/test_index.py
@@ -467,13 +467,13 @@ class TestCandidateEvaluator:
         )
         result = evaluator.compute_best_candidate(candidates)
 
-        assert result._candidates == candidates
+        assert result.all_candidates == candidates
         expected_applicable = candidates[:2]
         assert [str(c.version) for c in expected_applicable] == [
             "1.10",
             "1.11",
         ]
-        assert result._applicable_candidates == expected_applicable
+        assert result.applicable_candidates == expected_applicable
 
         assert result.best_candidate is expected_applicable[1]
 
@@ -490,8 +490,8 @@ class TestCandidateEvaluator:
         )
         result = evaluator.compute_best_candidate(candidates)
 
-        assert result._candidates == candidates
-        assert result._applicable_candidates == []
+        assert result.all_candidates == candidates
+        assert result.applicable_candidates == []
         assert result.best_candidate is None
 
     @pytest.mark.parametrize(