Improve metadata validation and handling of invalid distributions

news/13638.bugfix.rst

@@ -0,0 +1,3 @@
+Warn when installed distributions contain invalid metadata and skip
+invalid distributions consistently while allowing recovery commands to
+operate.

src/pip/_internal/commands/show.py

@@ -94,7 +94,10 @@ def search_packages_info(
     """
     env = get_default_environment()
 
-    installed = {dist.canonical_name: dist for dist in env.iter_all_distributions()}
+    installed = {
+        dist.canonical_name: dist
+        for dist in env.iter_all_distributions(skip_invalid=False)
+    }
     query_names = [canonicalize_name(name) for name in query]
     missing = sorted(
         [name for name, pkg in zip(query, query_names) if pkg not in installed]

src/pip/_internal/metadata/base.py

@@ -5,6 +5,7 @@
 import functools
 import json
 import logging
+import os
 import pathlib
 import re
 import zipfile
@@ -19,7 +20,7 @@
 from pip._vendor.packaging.requirements import Requirement
 from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet
 from pip._vendor.packaging.utils import NormalizedName, canonicalize_name
-from pip._vendor.packaging.version import Version
+from pip._vendor.packaging.version import InvalidVersion, Version
 
 from pip._internal.exceptions import NoneMetadataError
 from pip._internal.locations import site_packages, user_site
@@ -46,6 +47,11 @@
 
 logger = logging.getLogger(__name__)
 
+_VALID_PROJECT_NAME_RE = re.compile(
+    r"^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$",
+    flags=re.IGNORECASE,
+)
+
 
 class BaseEntryPoint(Protocol):
     @property
@@ -592,7 +598,9 @@ def default(cls) -> BaseEnvironment:
     def from_paths(cls, paths: list[str] | None) -> BaseEnvironment:
         raise NotImplementedError()
 
-    def get_distribution(self, name: str) -> BaseDistribution | None:
+    def get_distribution(
+        self, name: str, skip_invalid: bool = False
+    ) -> BaseDistribution | None:
         """Given a requirement name, return the installed distributions.
 
         The name may not be normalized. The implementation must canonicalize
@@ -609,24 +617,132 @@ def _iter_distributions(self) -> Iterator[BaseDistribution]:
         """
         raise NotImplementedError()
 
-    def iter_all_distributions(self) -> Iterator[BaseDistribution]:
-        """Iterate through all installed distributions without any filtering."""
-        for dist in self._iter_distributions():
-            # Make sure the distribution actually comes from a valid Python
-            # packaging distribution. Pip's AdjacentTempDirectory leaves folders
-            # e.g. ``~atplotlib.dist-info`` if cleanup was interrupted. The
-            # valid project name pattern is taken from PEP 508.
-            project_name_valid = re.match(
-                r"^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$",
-                dist.canonical_name,
-                flags=re.IGNORECASE,
+    def _get_name_and_version_from_info_location(
+        self, dist: BaseDistribution
+    ) -> tuple[str, str] | None:
+        info_location = dist.info_location
+        if not info_location:
+            return None
+        stem, suffix = os.path.splitext(pathlib.Path(info_location).name)
+        if suffix == ".dist-info":
+            name, sep, version = stem.partition("-")
+            if sep and name and version:
+                return name, version
+        return None
+
+    def _is_valid_project_name(self, name: str) -> bool:
+        return bool(_VALID_PROJECT_NAME_RE.match(name))
+
+    def validate_distribution(self, dist: BaseDistribution) -> bool:
+        # do dir name and version exist?
+        dir_info = self._get_name_and_version_from_info_location(dist)
+        if not dist.info_location or not dist.info_location.endswith(".dist-info"):
+            # Validation is only implemented for modern .dist-info installations.
+            # Legacy egg-info distributions are passed through without validation.
+            return True
+        if not dir_info:
+            logger.warning(
+                "Ignoring distribution at %s: could not determine "
+                "package name and/or version from the installation directory.",
+                dist.location,
             )
-            if not project_name_valid:
+            return False
+
+        dir_name, dir_version = dir_info
+        # is dir name valid?
+        # Make sure the distribution actually comes from a valid Python
+        # packaging distribution. Pip's AdjacentTempDirectory leaves folders
+        # e.g. ``~atplotlib.dist-info`` if cleanup was interrupted. The
+        # valid project name pattern is taken from PEP 508.
+        if not self._is_valid_project_name(dir_name):
+            logger.warning(
+                "Ignoring distribution at %s: %r is not a valid package name. "
+                "This may be a partial or interrupted installation.",
+                dist.location,
+                dir_name,
+            )
+            return False
+
+        # does METADATA have a Name value?
+        name = dist.metadata.get("Name")
+        if not name:
+            logger.warning(
+                "Ignoring distribution %r at %s: METADATA is missing the "
+                "required Name field.",
+                dir_name,
+                dist.location,
+            )
+            return False
+
+        # is Name valid?
+        if not self._is_valid_project_name(name):
+            logger.warning(
+                "Ignoring distribution at %s: %r is not a valid package name. "
+                "This may be a partial or interrupted installation.",
+                dist.location,
+                name,
+            )
+            return False
+
+        # do METADATA Name and directory name agree with each other?
+        if canonicalize_name(name) != canonicalize_name(dir_name):
+            logger.warning(
+                "Ignoring distribution %r at %s: package name in METADATA "
+                "(%r) does not match the installation directory name (%r).",
+                dir_name,
+                dist.location,
+                name,
+                dir_name,
+            )
+            return False
+
+        # does METADATA have a Version value?
+        version = dist.metadata.get("Version")
+        if not version:
+            logger.warning(
+                "Ignoring distribution %r at %s: METADATA is missing the "
+                "required Version field.",
+                dir_name,
+                dist.location,
+            )
+            return False
+
+        # do METADATA Version and directory version agree with each other?
+        # also checking if METADATA Version or directory version are invalid
+        try:
+            if Version(version) != Version(dir_version):
                 logger.warning(
-                    "Ignoring invalid distribution %s (%s)",
-                    dist.canonical_name,
+                    "Ignoring distribution %r at %s: version in METADATA (%r) "
+                    "does not match the installation directory version (%r).",
+                    dir_name,
                     dist.location,
+                    version,
+                    dir_version,
                 )
+                return False
+        except InvalidVersion:
+            logger.warning(
+                "Ignoring distribution %r at %s: version %r is not a valid "
+                "PEP 440 version string.",
+                dir_name,
+                dist.location,
+                version,
+            )
+            return False
+
+        return True
+
+    def iter_all_distributions(
+        self, skip_invalid: bool = True
+    ) -> Iterator[BaseDistribution]:
+        """
+        Iterate through all installed distributions.
+
+        If skip_invalid is True (the default), invalid distributions are
+        logged and skipped.
+        """
+        for dist in self._iter_distributions():
+            if skip_invalid and not self.validate_distribution(dist):
                 continue
             yield dist
 

src/pip/_internal/metadata/importlib/_envs.py

@@ -138,11 +138,13 @@ def _iter_distributions(self) -> Iterator[BaseDistribution]:
             yield from finder.find(location)
             yield from finder.find_legacy_editables(location)
 
-    def get_distribution(self, name: str) -> BaseDistribution | None:
+    def get_distribution(
+        self, name: str, skip_invalid: bool = False
+    ) -> BaseDistribution | None:
         canonical_name = canonicalize_name(name)
         matches = (
             distribution
-            for distribution in self.iter_all_distributions()
+            for distribution in self.iter_all_distributions(skip_invalid=skip_invalid)
             if distribution.canonical_name == canonical_name
         )
         return next(matches, None)

src/pip/_internal/metadata/pkg_resources.py

@@ -263,21 +263,25 @@ def _iter_distributions(self) -> Iterator[BaseDistribution]:
         for dist in self._ws:
             yield Distribution(dist)
 
-    def _search_distribution(self, name: str) -> BaseDistribution | None:
+    def _search_distribution(
+        self, name: str, skip_invalid: bool = False
+    ) -> BaseDistribution | None:
         """Find a distribution matching the ``name`` in the environment.
 
         This searches from *all* distributions available in the environment, to
         match the behavior of ``pkg_resources.get_distribution()``.
         """
         canonical_name = canonicalize_name(name)
-        for dist in self.iter_all_distributions():
+        for dist in self.iter_all_distributions(skip_invalid=skip_invalid):
             if dist.canonical_name == canonical_name:
                 return dist
         return None
 
-    def get_distribution(self, name: str) -> BaseDistribution | None:
+    def get_distribution(
+        self, name: str, skip_invalid: bool = False
+    ) -> BaseDistribution | None:
         # Search the distribution by looking through the working set.
-        dist = self._search_distribution(name)
+        dist = self._search_distribution(name, skip_invalid=skip_invalid)
         if dist:
             return dist
 
@@ -295,4 +299,4 @@ def get_distribution(self, name: str) -> BaseDistribution | None:
             self._ws.require(name)
         except pkg_resources.DistributionNotFound:
             return None
-        return self._search_distribution(name)
+        return self._search_distribution(name, skip_invalid=skip_invalid)

src/pip/_internal/req/req_install.py

@@ -421,7 +421,9 @@ def check_if_exists(self, use_user_site: bool) -> None:
         """
         if self.req is None:
             return
-        existing_dist = get_default_environment().get_distribution(self.req.name)
+        existing_dist = get_default_environment().get_distribution(
+            self.req.name, skip_invalid=False
+        )
         if not existing_dist:
             return
 
@@ -686,7 +688,9 @@ def uninstall(
 
         """
         assert self.req
-        dist = get_default_environment().get_distribution(self.req.name)
+        dist = get_default_environment().get_distribution(
+            self.req.name, skip_invalid=False
+        )
         if not dist:
             logger.warning("Skipping %s as it is not installed.", self.name)
             return None

tests/functional/test_freeze.py

@@ -191,6 +191,27 @@ def fake_install(pkgname: str, dest: str) -> None:
         assert f"Ignoring invalid distribution {name} (" in result.stderr
 
 
+def test_freeze_skips_malformed_dist(script: PipTestEnvironment) -> None:
+    """
+    Test that pip freeze skips malformed distributions with a warning.
+    """
+    dist_info_path = os.path.join(os.fspath(script.site_packages_path), "foo.dist-info")
+    os.makedirs(dist_info_path)
+    with open(os.path.join(dist_info_path, "METADATA"), "w") as f:
+        f.write(textwrap.dedent("""\
+            Metadata-Version: 1.0
+            Name: foo
+            Version: 1.0
+            """))
+
+    result = script.pip("freeze", expect_stderr=True)
+    output_lines = {line.strip() for line in result.stdout.splitlines()}
+    assert "foo==1.0" not in output_lines
+    assert (
+        "package name and/or version from the installation directory." in result.stderr
+    )
+
+
 @pytest.mark.git
 def test_freeze_editable_not_vcs(script: PipTestEnvironment) -> None:
     """

tests/functional/test_list.py

@@ -1,5 +1,6 @@
 import json
 import os
+import textwrap
 from pathlib import Path
 
 import pytest
@@ -905,3 +906,23 @@ def test_outdated_all_releases_for_specific_package(
     assert len(outdated) == 1
     assert outdated[0]["name"] == "simple"
     assert outdated[0]["latest_version"] == "2.0a1"
+
+
+def test_list_skips_malformed_dist(script: PipTestEnvironment) -> None:
+    """
+    Test that pip list skips malformed distributions with a warning.
+    """
+    dist_info_path = os.path.join(os.fspath(script.site_packages_path), "foo.dist-info")
+    os.makedirs(dist_info_path)
+    with open(os.path.join(dist_info_path, "METADATA"), "w") as f:
+        f.write(textwrap.dedent("""\
+            Metadata-Version: 1.0
+            Name: foo
+            Version: 1.0
+            """))
+
+    result = script.pip("list", expect_stderr=True)
+    output_lines = {line.strip() for line in result.stdout.splitlines()}
+
+    assert "foo==1.0" not in output_lines
+    assert "could not determine package name and/or version" in result.stderr

tests/functional/test_show.py

@@ -233,6 +233,28 @@ def test_show_verbose(script: PipTestEnvironment) -> None:
     assert "Project-URLs:" in lines
 
 
+def test_show_includes_malformed_dist(script: PipTestEnvironment) -> None:
+    """
+    Test that show can still find and display a malformed distribution,
+    so the user has the option to recover.
+    """
+    dist_info_path = os.path.join(
+        os.fspath(script.site_packages_path),
+        "foo-1.0.dist-info",
+    )
+    os.makedirs(dist_info_path)
+    with open(os.path.join(dist_info_path, "METADATA"), "w") as f:
+        f.write(textwrap.dedent("""\
+            Metadata-Version: 1.0
+            Name: foo
+            Version: 2.0
+            """))
+
+    result = script.pip("show", "foo", allow_stderr_warning=True)
+    assert "Name: foo" in result.stdout
+    assert len(result.stderr) == 0
+
+
 def test_all_fields(script: PipTestEnvironment) -> None:
     """
     Test that all the fields are present

tests/functional/test_uninstall.py

@@ -720,6 +720,30 @@ def test_uninstall_editable_and_pip_install_easy_install_remove(
     script.assert_not_installed("FSPkg")
 
 
+def test_uninstall_removes_malformed_dist(script: PipTestEnvironment) -> None:
+    """
+    Test that uninstall can still find and remove a malformed distribution.
+    """
+    dist_info_path = os.path.join(
+        os.fspath(script.site_packages_path),
+        "foo-1.0.dist-info",
+    )
+    os.makedirs(dist_info_path)
+    with open(os.path.join(dist_info_path, "METADATA"), "w") as f:
+        f.write(textwrap.dedent("""\
+            Metadata-Version: 1.0
+            Name: foo
+            Version: 2.0
+            """))
+    with open(os.path.join(dist_info_path, "RECORD"), "w") as f:
+        f.write("foo-1.0.dist-info/METADATA,,\n")
+        f.write("foo-1.0.dist-info/")
+
+    result = script.pip("uninstall", "foo", "-y", allow_stderr_warning=True)
+    assert not os.path.exists(dist_info_path)
+    assert len(result.stderr) == 0
+
+
 def test_uninstall_ignores_missing_packages(
     script: PipTestEnvironment, data: TestData
 ) -> None: