Repository: pypa/pip (python)

Issue:
pip parses `dist-info-metadata` key from JSON indexes incorrectly
### Description

When using a JSON-based package index, pip expects the `dist-info-metadata` key to always have a string value and does not parse the values described by the spec.

[PEP 691 states](https://peps.python.org/pep-0691/#project-detail):

> `dist-info-metadata`: An optional key that indicates that metadata for this file is available, via the same location as specified in [PEP 658](https://peps.python.org/pep-0658) (`{file_url}.metadata`). **Where this is present, it MUST be either a boolean to indicate if the file has an associated metadata file, or a dictionary mapping hash names to a hex encoded digest of the metadata’s hash.**

pip tries to apply the same string parsing for this value as it does for the `data-dist-info-metadata` value in [PEP 658](https://peps.python.org/pep-0658/#specification) and does not handle booleans or dicts.

This does not work as the entries according to PEP 691 look like:

```json
  {
    "dist-info-metadata": {
      "sha256": "8737b5dbb73f6b05e596c7c659f694a031de98b926e634716d9855fd365a5f2f"
    },
    "filename": "pre_commit-3.3.2-py2.py3-none-any.whl",
    [...]
  },
```

### Expected behavior

pip should handle values with boolean or dict type as described in [PEP 691](https://peps.python.org/pep-0691/#project-detail).

### pip version

Tested with 23.1.2 and current `main`

### Python version

3.11

### OS

Linux (debian bullseye)

### How to Reproduce

To reproduce, we need access to a registry which supports PEP 691 JSON indexes and also PEP 658 metadata.

pypi.org is supposed to support both of these for new file uploads, but there is currently a bug where it provides the metadata under the wrong JSON key (https://github.com/pypi/warehouse/issues/13705), so it is hard to test at the moment. For now, the easiest way to reproduce is to temporarily patch your pip code to use the (incorrect) key to trigger the bug.

1. Temporarily patch your `pip` code so that it uses the key name provided by pypi.org:

    ```diff
    diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py
    index e741c3283..5d1e0be4e 100644
    --- a/src/pip/_internal/models/link.py
    +++ b/src/pip/_internal/models/link.py
    @@ -262,7 +262,7 @@ class Link(KeyBasedCompareMixin):
             url = _ensure_quoted_url(urllib.parse.urljoin(page_url, file_url))
             pyrequire = file_data.get("requires-python")
             yanked_reason = file_data.get("yanked")
    -        dist_info_metadata = file_data.get("dist-info-metadata")
    +        dist_info_metadata = file_data.get("data-dist-info-metadata")
             hashes = file_data.get("hashes", {})
     
             # The Link.yanked_reason expects an empty string instead of a boolean.
    ```

  2. Try to install a package.
  
      *  `pip install --no-cache --only-binary :all: pre-commit==3.3.2` to reproduce pip trying to parse a metadata dict as a string
      *  `pip install --no-cache --only-binary :all: pre-commit==3.0.0` to reproduce pip trying to parse a boolean `false` as a string

### Output

```
$ pip install --no-cache --only-binary :all: pre-commit==3.3.2          
Collecting pre-commit==3.3.2
ERROR: Exception:
Traceback (most recent call last):
  File "/home/ckuehl/proj/pip/src/pip/_internal/cli/base_command.py", line 169, in exc_logging_wrapper
    status = run_func(*args)
             ^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/cli/req_command.py", line 248, in wrapper
    return func(self, options, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/commands/install.py", line 377, in run
    requirement_set = resolver.resolve(
                      ^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/resolver.py", line 92, in resolve
    result = self._result = resolver.resolve(
                            ^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_vendor/resolvelib/resolvers.py", line 546, in resolve
    state = resolution.resolve(requirements, max_rounds=max_rounds)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_vendor/resolvelib/resolvers.py", line 397, in resolve
    self._add_to_criteria(self.state.criteria, r, parent=None)
  File "/home/ckuehl/proj/pip/src/pip/_vendor/resolvelib/resolvers.py", line 173, in _add_to_criteria
    if not criterion.candidates:
  File "/home/ckuehl/proj/pip/src/pip/_vendor/resolvelib/structs.py", line 156, in __bool__
    return bool(self._sequence)
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/found_candidates.py", line 155, in __bool__
    return any(self)
           ^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/found_candidates.py", line 143, in <genexpr>
    return (c for c in iterator if id(c) not in self._incompatible_ids)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/found_candidates.py", line 47, in _iter_built
    candidate = func()
                ^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/factory.py", line 206, in _make_candidate_from_link
    self._link_candidate_cache[link] = LinkCandidate(
                                       ^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/candidates.py", line 293, in __init__
    super().__init__(
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/candidates.py", line 156, in __init__
    self.dist = self._prepare()
                ^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/candidates.py", line 225, in _prepare
    dist = self._prepare_distribution()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/resolution/resolvelib/candidates.py", line 304, in _prepare_distribution
    return preparer.prepare_linked_requirement(self._ireq, parallel_builds=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/operations/prepare.py", line 510, in prepare_linked_requirement
    metadata_dist = self._fetch_metadata_only(req)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/operations/prepare.py", line 374, in _fetch_metadata_only
    return self._fetch_metadata_using_link_data_attr(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/operations/prepare.py", line 384, in _fetch_metadata_using_link_data_attr
    metadata_link = req.link.metadata_link()
                    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/models/link.py", line 417, in metadata_link
    metadata_link_hash = LinkHash.parse_pep658_hash(self.dist_info_metadata)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ckuehl/proj/pip/src/pip/_internal/models/link.py", line 77, in parse_pep658_hash
    name, sep, value = dist_info_metadata.partition("=")
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'dict' object has no attribute 'partition'
```

Same traceback for `pre-commit==3.0.0` except the last line is instead

```
AttributeError: 'bool' object has no attribute 'partition'
```

### Code of Conduct

- [X] I agree to follow the [PSF Code of Conduct](https://www.python.org/psf/conduct/).

Pull request:
Fix parsing of JSON index dist-info-metadata values
Fixes #12042

Patch:
diff --git a/news/12042.bugfix.rst b/news/12042.bugfix.rst
new file mode 100644
index 000000000..34d977435
--- /dev/null
+++ b/news/12042.bugfix.rst
@@ -0,0 +1,1 @@
+Correctly parse ``dist-info-metadata`` values from JSON-format index data.
diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py
index e741c3283..4453519ad 100644
--- a/src/pip/_internal/models/link.py
+++ b/src/pip/_internal/models/link.py
@@ -69,18 +69,6 @@ class LinkHash:
     def __post_init__(self) -> None:
         assert self.name in _SUPPORTED_HASHES
 
-    @classmethod
-    def parse_pep658_hash(cls, dist_info_metadata: str) -> Optional["LinkHash"]:
-        """Parse a PEP 658 data-dist-info-metadata hash."""
-        if dist_info_metadata == "true":
-            return None
-        name, sep, value = dist_info_metadata.partition("=")
-        if not sep:
-            return None
-        if name not in _SUPPORTED_HASHES:
-            return None
-        return cls(name=name, value=value)
-
     @classmethod
     @functools.lru_cache(maxsize=None)
     def find_hash_url_fragment(cls, url: str) -> Optional["LinkHash"]:
@@ -107,6 +95,28 @@ class LinkHash:
         return hashes.is_hash_allowed(self.name, hex_digest=self.value)
 
 
+@dataclass(frozen=True)
+class MetadataFile:
+    """Information about a core metadata file associated with a distribution."""
+
+    hashes: Optional[Dict[str, str]]
+
+    def __post_init__(self) -> None:
+        if self.hashes is not None:
+            assert all(name in _SUPPORTED_HASHES for name in self.hashes)
+
+
+def supported_hashes(hashes: Optional[Dict[str, str]]) -> Optional[Dict[str, str]]:
+    # Remove any unsupported hash types from the mapping. If this leaves no
+    # supported hashes, return None
+    if hashes is None:
+        return None
+    hashes = {n: v for n, v in hashes.items() if n in _SUPPORTED_HASHES}
+    if not hashes:
+        return None
+    return hashes
+
+
 def _clean_url_path_part(part: str) -> str:
     """
     Clean a "part" of a URL path (i.e. after splitting on "@" characters).
@@ -179,7 +189,7 @@ class Link(KeyBasedCompareMixin):
         "comes_from",
         "requires_python",
         "yanked_reason",
-        "dist_info_metadata",
+        "metadata_file_data",
         "cache_link_parsing",
         "egg_fragment",
     ]
@@ -190,7 +200,7 @@ class Link(KeyBasedCompareMixin):
         comes_from: Optional[Union[str, "IndexContent"]] = None,
         requires_python: Optional[str] = None,
         yanked_reason: Optional[str] = None,
-        dist_info_metadata: Optional[str] = None,
+        metadata_file_data: Optional[MetadataFile] = None,
         cache_link_parsing: bool = True,
         hashes: Optional[Mapping[str, str]] = None,
     ) -> None:
@@ -208,11 +218,10 @@ class Link(KeyBasedCompareMixin):
             a simple repository HTML link. If the file has been yanked but
             no reason was provided, this should be the empty string. See
             PEP 592 for more information and the specification.
-        :param dist_info_metadata: the metadata attached to the file, or None if no such
-            metadata is provided. This is the value of the "data-dist-info-metadata"
-            attribute, if present, in a simple repository HTML link. This may be parsed
-            into its own `Link` by `self.metadata_link()`. See PEP 658 for more
-            information and the specification.
+        :param metadata_file_data: the metadata attached to the file, or None if
+            no such metadata is provided. This argument, if not None, indicates
+            that a separate metadata file exists, and also optionally supplies
+            hashes for that file.
         :param cache_link_parsing: A flag that is used elsewhere to determine
             whether resources retrieved from this link should be cached. PyPI
             URLs should generally have this set to False, for example.
@@ -220,6 +229,10 @@ class Link(KeyBasedCompareMixin):
             determine the validity of a download.
         """
 
+        # The comes_from, requires_python, and metadata_file_data arguments are
+        # only used by classmethods of this class, and are not used in client
+        # code directly.
+
         # url can be a UNC windows share
         if url.startswith("\\\\"):
             url = path_to_url(url)
@@ -239,7 +252,7 @@ class Link(KeyBasedCompareMixin):
         self.comes_from = comes_from
         self.requires_python = requires_python if requires_python else None
         self.yanked_reason = yanked_reason
-        self.dist_info_metadata = dist_info_metadata
+        self.metadata_file_data = metadata_file_data
 
         super().__init__(key=url, defining_class=Link)
 
@@ -262,9 +275,25 @@ class Link(KeyBasedCompareMixin):
         url = _ensure_quoted_url(urllib.parse.urljoin(page_url, file_url))
         pyrequire = file_data.get("requires-python")
         yanked_reason = file_data.get("yanked")
-        dist_info_metadata = file_data.get("dist-info-metadata")
         hashes = file_data.get("hashes", {})
 
+        # PEP 714: Indexes must use the name core-metadata, but
+        # clients should support the old name as a fallback for compatibility.
+        metadata_info = file_data.get("core-metadata")
+        if metadata_info is None:
+            metadata_info = file_data.get("dist-info-metadata")
+
+        # The metadata info value may be a boolean, or a dict of hashes.
+        if isinstance(metadata_info, dict):
+            # The file exists, and hashes have been supplied
+            metadata_file_data = MetadataFile(supported_hashes(metadata_info))
+        elif metadata_info:
+            # The file exists, but there are no hashes
+            metadata_file_data = MetadataFile(None)
+        else:
+            # False or not present: the file does not exist
+            metadata_file_data = None
+
         # The Link.yanked_reason expects an empty string instead of a boolean.
         if yanked_reason and not isinstance(yanked_reason, str):
             yanked_reason = ""
@@ -278,7 +307,7 @@ class Link(KeyBasedCompareMixin):
             requires_python=pyrequire,
             yanked_reason=yanked_reason,
             hashes=hashes,
-            dist_info_metadata=dist_info_metadata,
+            metadata_file_data=metadata_file_data,
         )
 
     @classmethod
@@ -298,14 +327,39 @@ class Link(KeyBasedCompareMixin):
         url = _ensure_quoted_url(urllib.parse.urljoin(base_url, href))
         pyrequire = anchor_attribs.get("data-requires-python")
         yanked_reason = anchor_attribs.get("data-yanked")
-        dist_info_metadata = anchor_attribs.get("data-dist-info-metadata")
+
+        # PEP 714: Indexes must use the name data-core-metadata, but
+        # clients should support the old name as a fallback for compatibility.
+        metadata_info = anchor_attribs.get("data-core-metadata")
+        if metadata_info is None:
+            metadata_info = anchor_attribs.get("data-dist-info-metadata")
+        # The metadata info value may be the string "true", or a string of
+        # the form "hashname=hashval"
+        if metadata_info == "true":
+            # The file exists, but there are no hashes
+            metadata_file_data = MetadataFile(None)
+        elif metadata_info is None:
+            # The file does not exist
+            metadata_file_data = None
+        else:
+            # The file exists, and hashes have been supplied
+            hashname, sep, hashval = metadata_info.partition("=")
+            if sep == "=":
+                metadata_file_data = MetadataFile(supported_hashes({hashname: hashval}))
+            else:
+                # Error - data is wrong. Treat as no hashes supplied.
+                logger.debug(
+                    "Index returned invalid data-dist-info-metadata value: %s",
+                    metadata_info,
+                )
+                metadata_file_data = MetadataFile(None)
 
         return cls(
             url,
             comes_from=page_url,
             requires_python=pyrequire,
             yanked_reason=yanked_reason,
-            dist_info_metadata=dist_info_metadata,
+            metadata_file_data=metadata_file_data,
         )
 
     def __str__(self) -> str:
@@ -407,17 +461,13 @@ class Link(KeyBasedCompareMixin):
         return match.group(1)
 
     def metadata_link(self) -> Optional["Link"]:
-        """Implementation of PEP 658 parsing."""
-        # Note that Link.from_element() parsing the "data-dist-info-metadata" attribute
-        # from an HTML anchor tag is typically how the Link.dist_info_metadata attribute
-        # gets set.
-        if self.dist_info_metadata is None:
+        """Return a link to the associated core metadata file (if any)."""
+        if self.metadata_file_data is None:
             return None
         metadata_url = f"{self.url_without_fragment}.metadata"
-        metadata_link_hash = LinkHash.parse_pep658_hash(self.dist_info_metadata)
-        if metadata_link_hash is None:
+        if self.metadata_file_data.hashes is None:
             return Link(metadata_url)
-        return Link(metadata_url, hashes=metadata_link_hash.as_dict())
+        return Link(metadata_url, hashes=self.metadata_file_data.hashes)
 
     def as_hashes(self) -> Hashes:
         return Hashes({k: [v] for k, v in self._hashes.items()})


Test patch:
diff --git a/tests/unit/test_collector.py b/tests/unit/test_collector.py
index e855d78e1..5410a4afc 100644
--- a/tests/unit/test_collector.py
+++ b/tests/unit/test_collector.py
@@ -30,6 +30,7 @@ from pip._internal.models.index import PyPI
 from pip._internal.models.link import (
     Link,
     LinkHash,
+    MetadataFile,
     _clean_url_path,
     _ensure_quoted_url,
 )
@@ -485,13 +486,30 @@ def test_parse_links_json() -> None:
                     "requires-python": ">=3.7",
                     "dist-info-metadata": False,
                 },
-                # Same as above, but parsing dist-info-metadata.
+                # Same as above, but parsing core-metadata.
                 {
                     "filename": "holygrail-1.0-py3-none-any.whl",
                     "url": "/files/holygrail-1.0-py3-none-any.whl",
                     "hashes": {"sha256": "sha256 hash", "blake2b": "blake2b hash"},
                     "requires-python": ">=3.7",
-                    "dist-info-metadata": "sha512=aabdd41",
+                    "core-metadata": {"sha512": "aabdd41"},
+                },
+                # Ensure fallback to dist-info-metadata works
+                {
+                    "filename": "holygrail-1.0-py3-none-any.whl",
+                    "url": "/files/holygrail-1.0-py3-none-any.whl",
+                    "hashes": {"sha256": "sha256 hash", "blake2b": "blake2b hash"},
+                    "requires-python": ">=3.7",
+                    "dist-info-metadata": {"sha512": "aabdd41"},
+                },
+                # Ensure that core-metadata gets priority.
+                {
+                    "filename": "holygrail-1.0-py3-none-any.whl",
+                    "url": "/files/holygrail-1.0-py3-none-any.whl",
+                    "hashes": {"sha256": "sha256 hash", "blake2b": "blake2b hash"},
+                    "requires-python": ">=3.7",
+                    "core-metadata": {"sha512": "aabdd41"},
+                    "dist-info-metadata": {"sha512": "this_is_wrong"},
                 },
             ],
         }
@@ -527,7 +545,23 @@ def test_parse_links_json() -> None:
             requires_python=">=3.7",
             yanked_reason=None,
             hashes={"sha256": "sha256 hash", "blake2b": "blake2b hash"},
-            dist_info_metadata="sha512=aabdd41",
+            metadata_file_data=MetadataFile({"sha512": "aabdd41"}),
+        ),
+        Link(
+            "https://example.com/files/holygrail-1.0-py3-none-any.whl",
+            comes_from=page.url,
+            requires_python=">=3.7",
+            yanked_reason=None,
+            hashes={"sha256": "sha256 hash", "blake2b": "blake2b hash"},
+            metadata_file_data=MetadataFile({"sha512": "aabdd41"}),
+        ),
+        Link(
+            "https://example.com/files/holygrail-1.0-py3-none-any.whl",
+            comes_from=page.url,
+            requires_python=">=3.7",
+            yanked_reason=None,
+            hashes={"sha256": "sha256 hash", "blake2b": "blake2b hash"},
+            metadata_file_data=MetadataFile({"sha512": "aabdd41"}),
         ),
     ]
 
@@ -585,30 +619,42 @@ _pkg1_requirement = Requirement("pkg1==1.0")
         ),
         # Test with value "true".
         (
-            '<a href="/pkg1-1.0.tar.gz" data-dist-info-metadata="true"></a>',
-            "true",
+            '<a href="/pkg1-1.0.tar.gz" data-core-metadata="true"></a>',
+            MetadataFile(None),
             {},
         ),
         # Test with a provided hash value.
         (
-            '<a href="/pkg1-1.0.tar.gz" data-dist-info-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
-            "sha256=aa113592bbe",
+            '<a href="/pkg1-1.0.tar.gz" data-core-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
+            MetadataFile({"sha256": "aa113592bbe"}),
             {},
         ),
         # Test with a provided hash value for both the requirement as well as metadata.
         (
-            '<a href="/pkg1-1.0.tar.gz#sha512=abc132409cb" data-dist-info-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
-            "sha256=aa113592bbe",
+            '<a href="/pkg1-1.0.tar.gz#sha512=abc132409cb" data-core-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
+            MetadataFile({"sha256": "aa113592bbe"}),
             {"sha512": "abc132409cb"},
         ),
+        # Ensure the fallback to the old name works.
+        (
+            '<a href="/pkg1-1.0.tar.gz" data-dist-info-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
+            MetadataFile({"sha256": "aa113592bbe"}),
+            {},
+        ),
+        # Ensure that the data-core-metadata name gets priority.
+        (
+            '<a href="/pkg1-1.0.tar.gz" data-core-metadata="sha256=aa113592bbe" data-dist-info-metadata="sha256=invalid_value"></a>',  # noqa: E501
+            MetadataFile({"sha256": "aa113592bbe"}),
+            {},
+        ),
     ],
 )
-def test_parse_links__dist_info_metadata(
+def test_parse_links__metadata_file_data(
     anchor_html: str,
     expected: Optional[str],
     hashes: Dict[str, str],
 ) -> None:
-    link = _test_parse_links_data_attribute(anchor_html, "dist_info_metadata", expected)
+    link = _test_parse_links_data_attribute(anchor_html, "metadata_file_data", expected)
     assert link._hashes == hashes
 
 
@@ -1080,17 +1126,26 @@ def test_link_hash_parsing(url: str, result: Optional[LinkHash]) -> None:
 
 
 @pytest.mark.parametrize(
-    "dist_info_metadata, result",
+    "metadata_attrib, expected",
     [
-        ("sha256=aa113592bbe", LinkHash("sha256", "aa113592bbe")),
-        ("sha256=", LinkHash("sha256", "")),
-        ("sha500=aa113592bbe", None),
-        ("true", None),
-        ("", None),
-        ("aa113592bbe", None),
+        ("sha256=aa113592bbe", MetadataFile({"sha256": "aa113592bbe"})),
+        ("sha256=", MetadataFile({"sha256": ""})),
+        ("sha500=aa113592bbe", MetadataFile(None)),
+        ("true", MetadataFile(None)),
+        (None, None),
+        # Attribute is present but invalid
+        ("", MetadataFile(None)),
+        ("aa113592bbe", MetadataFile(None)),
     ],
 )
-def test_pep658_hash_parsing(
-    dist_info_metadata: str, result: Optional[LinkHash]
+def test_metadata_file_info_parsing_html(
+    metadata_attrib: str, expected: Optional[MetadataFile]
 ) -> None:
-    assert LinkHash.parse_pep658_hash(dist_info_metadata) == result
+    attribs: Dict[str, Optional[str]] = {
+        "href": "something",
+        "data-dist-info-metadata": metadata_attrib,
+    }
+    page_url = "dummy_for_comes_from"
+    base_url = "https://index.url/simple"
+    link = Link.from_element(attribs, page_url, base_url)
+    assert link is not None and link.metadata_file_data == expected