Skip to content

fix(pypi): skip malformed wheel anchors in SimpleAPI response#3907

Closed
kriscfoster wants to merge 3 commits into
bazel-contrib:mainfrom
kriscfoster:fix-pypi-fallback-empty-index-url
Closed

fix(pypi): skip malformed wheel anchors in SimpleAPI response#3907
kriscfoster wants to merge 3 commits into
bazel-contrib:mainfrom
kriscfoster:fix-pypi-fallback-empty-index-url

Conversation

@kriscfoster

Copy link
Copy Markdown
Contributor

Summary

Some SimpleAPI index servers (in particular certain PyPI proxies) occasionally
emit anchors whose filename is not a valid PEP 427 wheel name, for example:

<a href=\"../../packages/six/1.16.0/six.whl#sha256=...\">six.whl</a>

The filename six.whl has only one - separator instead of the required four
({distribution}-{version}(-{build})?-{python}-{abi}-{platform}.whl), so when
parse_whl_name later runs on this entry it fails and aborts the whole
resolve.

This change skips such anchors at parse time in parse_simpleapi_html and
warns via the existing pypi:simpleapi repo logger, so operators can see and
report the bad index entry to the index maintainer.

To make the warning possible, the existing logger already built in
simpleapi_download is threaded through _get_dist_urls /
_read_simpleapi / _read_index_result / parse_simpleapi_html in the same
style as the logger passed to select_whl.

What happens downstream when an anchor is skipped

Skipping only removes the malformed anchor from whls; it does not hide a
required wheel. In parse_requirements.bzl:

  1. If the same sha256 also appears on a well-formed anchor for the same
    wheel (the common case in practice — proxies typically emit the wheel
    twice, once well-formed and once with a stripped filename), the resolver
    finds the well-formed entry and everything proceeds normally.
  2. If the sha256 only ever appeared on the malformed anchor, the resolver
    logs Could not find a whl or an sdist with sha256=... and, since no
    candidate remains for the target platform, returns None, True — i.e.
    falls back to letting pip resolve that one package normally. The build
    does not fail silently and the dependency is not dropped.

Either way the behavior is strictly better than the previous crash in
parse_whl_name.

Test plan

  • Added unit test test_malformed_whl_anchor_is_skipped in
    tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl that
    passes a mixed response (one six.whl-shaped anchor, one valid wheel)
    and asserts the malformed one is skipped while the valid one survives.
  • Existing parse_simpleapi_html and hub_builder unit tests updated
    to pass the now-required logger and continue to pass.
  • Verified end-to-end against a real proxy that emits the malformed
    six.whl anchor: the warn fires with the exact filename/href, and
    bazel build @pip//:requirements.bzl succeeds.

Some index servers (e.g. proxies of PyPI) occasionally emit anchors like
`<a href=".../foo.whl#sha256=...">foo.whl</a>` whose filename does not
parse as a valid PEP 427 wheel name (fewer than 4 `-` separators).
Previously these would crash later in `parse_whl_name`. Skip them at
parse time and warn via the repo logger so operators can spot the bad
index entry.

Wire the existing `pypi:simpleapi` logger through `simpleapi_download`
so `parse_simpleapi_html` can emit the warning, and update the unit
tests to pass a logger.
Add a regression test for the previous fix: an anchor whose filename
does not parse as a valid PEP 427 wheel name (fewer than 4 `-`
separators, e.g. `six.whl`) is skipped, and a valid wheel in the same
response is still returned.
The `read_simpleapi` mocks in simpleapi_download_tests.bzl need to
accept the new `logger` keyword that `simpleapi_download` now threads
through, otherwise they error with:

    read_simpleapi() got unexpected keyword argument: logger
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@kriscfoster kriscfoster marked this pull request as draft July 6, 2026 15:15
@kriscfoster kriscfoster closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant