From 00856d524e04cf21f941d9dec4fcf140f6726231 Mon Sep 17 00:00:00 2001 From: Kris Foster Date: Mon, 6 Jul 2026 15:40:59 +0100 Subject: [PATCH 1/3] fix(pypi): skip malformed wheel anchors in SimpleAPI response Some index servers (e.g. proxies of PyPI) occasionally emit anchors like `foo.whl` 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. --- python/private/pypi/parse_simpleapi_html.bzl | 12 +++++++++- python/private/pypi/simpleapi_download.bzl | 22 ++++++++++++++----- tests/pypi/hub_builder/hub_builder_tests.bzl | 9 ++++++++ .../parse_simpleapi_html_tests.bzl | 14 +++++++++--- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/python/private/pypi/parse_simpleapi_html.bzl b/python/private/pypi/parse_simpleapi_html.bzl index 7f0d2776d7..77412f21a2 100644 --- a/python/private/pypi/parse_simpleapi_html.bzl +++ b/python/private/pypi/parse_simpleapi_html.bzl @@ -19,11 +19,12 @@ Parse SimpleAPI HTML in Starlark. load("//python/private:normalize_name.bzl", "normalize_name") load(":version_from_filename.bzl", "version_from_filename") -def parse_simpleapi_html(*, content, parse_index = False): +def parse_simpleapi_html(*, content, logger, parse_index = False): """Get the package URLs for given shas by parsing the Simple API HTML. Args: content: {type}`str` The Simple API HTML content. + logger: {type}`struct` the logger instance. parse_index: {type}`bool` whether to parse the content as the index page of the PyPI index, e.g. the `https://pypi.org/simple/`. This only has the URLs for the individual package. @@ -133,6 +134,15 @@ def parse_simpleapi_html(*, content, parse_index = False): ) if filename.endswith(".whl"): + # A valid wheel filename per PEP 427 has the form + # `{distribution}-{version}(-{build})?-{python}-{abi}-{platform}.whl`, + # i.e. at least 4 `-` separators. Some index servers (e.g. proxies + # of PyPI) occasionally emit malformed anchors like + # `foo.whl`. Skip them here + # rather than letting `parse_whl_name` fail later. + if filename.count("-") < 4: + logger.warn(lambda: "Ignoring malformed wheel anchor in SimpleAPI response: filename={} href={}".format(filename, href)) + continue whls[sha256] = dist else: sdists[sha256] = dist diff --git a/python/private/pypi/simpleapi_download.bzl b/python/private/pypi/simpleapi_download.bzl index 5377a08093..62437f0338 100644 --- a/python/private/pypi/simpleapi_download.bzl +++ b/python/private/pypi/simpleapi_download.bzl @@ -19,6 +19,7 @@ A file that houses private functions used in the `bzlmod` extension with the sam load("//python/private:auth.bzl", _get_auth = "get_auth") load("//python/private:envsubst.bzl", "envsubst") load("//python/private:normalize_name.bzl", "normalize_name") +load("//python/private:repo_utils.bzl", "repo_utils") load(":parse_simpleapi_html.bzl", "parse_simpleapi_html") load(":urllib.bzl", "urllib") @@ -83,6 +84,7 @@ def simpleapi_download( } read_simpleapi = read_simpleapi or _read_simpleapi + logger = repo_utils.logger(ctx, "pypi:simpleapi") ctx.report_progress("Fetch package lists from PyPI index") @@ -99,6 +101,7 @@ def simpleapi_download( get_auth = get_auth, attr = attr, block = not parallel_download, + logger = logger, _fail = _fail, ) @@ -116,6 +119,7 @@ def simpleapi_download( get_auth = get_auth, block = not parallel_download, parse_index = False, + logger = logger, ) if hasattr(result, "wait"): # We will process it in a separate loop: @@ -130,7 +134,7 @@ def simpleapi_download( return contents -def _get_dist_urls(ctx, *, default_index, index_urls, index_url_overrides, sources, read_simpleapi, attr, block, _fail = fail, **kwargs): +def _get_dist_urls(ctx, *, default_index, index_urls, index_url_overrides, sources, read_simpleapi, attr, block, logger, _fail = fail, **kwargs): # Ensure the value is not frozen index_urls = [] + (index_urls or []) if default_index not in index_urls: @@ -159,6 +163,7 @@ def _get_dist_urls(ctx, *, default_index, index_urls, index_url_overrides, sourc parse_index = True, versions = {pkg: None for pkg in sources}, block = block, + logger = logger, **kwargs ) if hasattr(download, "wait"): @@ -196,7 +201,7 @@ def _get_dist_urls(ctx, *, default_index, index_urls, index_url_overrides, sourc def _normalize_url(url): return urllib.strip_empty_path_segments(url) -def _read_simpleapi(ctx, url, attr, cache, versions, parse_index, get_auth = None, **download_kwargs): +def _read_simpleapi(ctx, url, attr, cache, versions, parse_index, logger, get_auth = None, **download_kwargs): """Read SimpleAPI. Args: @@ -210,9 +215,10 @@ def _read_simpleapi(ctx, url, attr, cache, versions, parse_index, get_auth = Non {obj}`http_file` for docs. cache: {type}`struct` the `pypi_cache` instance. versions: {type}`list[str] The versions that have been requested. - get_auth: A function to get auth information. Used in tests. parse_index: {type}`bool` Whether to parse the content as a root index page (e.g. `/simple/`) instead of a package-specific page. + logger: {type}`struct` the logger instance. + get_auth: A function to get auth information. Used in tests. **download_kwargs: Any extra params to ctx.download. Note that output and auth will be passed for you. @@ -262,6 +268,7 @@ def _read_simpleapi(ctx, url, attr, cache, versions, parse_index, get_auth = Non cache = cache, cache_key = cache_key, parse_index = parse_index, + logger = logger, ), ) @@ -272,15 +279,20 @@ def _read_simpleapi(ctx, url, attr, cache, versions, parse_index, get_auth = Non cache = cache, cache_key = cache_key, parse_index = parse_index, + logger = logger, ) -def _read_index_result(ctx, *, result, output, cache, cache_key, parse_index): +def _read_index_result(ctx, *, result, output, cache, cache_key, parse_index, logger): if not result.success: return struct(success = False) content = ctx.read(output) - output = parse_simpleapi_html(content = content, parse_index = parse_index) + output = parse_simpleapi_html( + content = content, + parse_index = parse_index, + logger = logger, + ) if output: cache.setdefault(cache_key, output) return struct(success = True, output = output) diff --git a/tests/pypi/hub_builder/hub_builder_tests.bzl b/tests/pypi/hub_builder/hub_builder_tests.bzl index 60017593fb..4b2e550da5 100644 --- a/tests/pypi/hub_builder/hub_builder_tests.bzl +++ b/tests/pypi/hub_builder/hub_builder_tests.bzl @@ -27,6 +27,13 @@ load("//tests/support/mocks:mocks.bzl", "mocks") _tests = [] +_LOGGER = repo_utils.logger(struct( + getenv = { + REPO_DEBUG_ENV_VAR: "1", + REPO_VERBOSITY_ENV_VAR: "INFO", + }.get, +), "unit-test") + def _mock_mctx(os_name = "unittest", arch_name = "exotic", environ = {}, mock_files = None): return mocks.mctx( os_name = os_name, @@ -257,6 +264,7 @@ def _test_simple_extras_vs_no_extras_simpleapi(env): output = parse_simpleapi_html( content = content, parse_index = parse_index, + logger = _LOGGER, ), success = True, ) @@ -517,6 +525,7 @@ def _test_torch_experimental_index_url(env): output = parse_simpleapi_html( content = content, parse_index = parse_index, + logger = _LOGGER, ), success = True, ) diff --git a/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl b/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl index c84140f459..a75b776d9a 100644 --- a/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl +++ b/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl @@ -16,8 +16,16 @@ load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:truth.bzl", "subjects") +load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "REPO_VERBOSITY_ENV_VAR", "repo_utils") # buildifier: disable=bzl-visibility load("//python/private/pypi:parse_simpleapi_html.bzl", "parse_simpleapi_html") # buildifier: disable=bzl-visibility +_LOGGER = repo_utils.logger(struct( + getenv = { + REPO_DEBUG_ENV_VAR: "1", + REPO_VERBOSITY_ENV_VAR: "INFO", + }.get, +), "unit-test") + _tests = [] def _generate_html(*items): @@ -59,7 +67,7 @@ def _test_index(env): for (input, want) in tests: html = _generate_html(*input) - got = parse_simpleapi_html(content = html, parse_index = True) + got = parse_simpleapi_html(content = html, parse_index = True, logger = _LOGGER) env.expect.that_dict(got).contains_exactly(want) @@ -140,7 +148,7 @@ def _test_sdist(env): for (input, want) in tests: html = _generate_html(input) - got = parse_simpleapi_html(content = html) + got = parse_simpleapi_html(content = html, logger = _LOGGER) env.expect.that_collection(got.sdists).has_size(1) env.expect.that_collection(got.whls).has_size(0) env.expect.that_collection(got.sha256s_by_version).has_size(1) @@ -268,7 +276,7 @@ def _test_whls(env): for (input, want) in tests: html = _generate_html(input) - got = parse_simpleapi_html(content = html) + got = parse_simpleapi_html(content = html, logger = _LOGGER) env.expect.that_collection(got.sdists).has_size(0) env.expect.that_collection(got.whls).has_size(1) if not got: From 33d7ed74498b47656b17646928d8b5adfd1cd330 Mon Sep 17 00:00:00 2001 From: Kris Foster Date: Mon, 6 Jul 2026 15:50:23 +0100 Subject: [PATCH 2/3] test: cover malformed wheel anchor in parse_simpleapi_html 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. --- .../parse_simpleapi_html_tests.bzl | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl b/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl index a75b776d9a..1d748b9e7a 100644 --- a/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl +++ b/tests/pypi/parse_simpleapi_html/parse_simpleapi_html_tests.bzl @@ -304,6 +304,33 @@ def _test_whls(env): _tests.append(_test_whls) +def _test_malformed_whl_anchor_is_skipped(env): + # Some index servers (e.g. proxies of PyPI) emit anchors whose filename + # does not parse as a valid PEP 427 wheel name (fewer than 4 `-` + # separators). These should be skipped rather than crash downstream. + input = struct( + attrs = [ + 'href="../../packages/six/1.16.0/six.whl#sha256=deadbeef"', + ], + filename = "six.whl", + ) + valid = struct( + attrs = [ + 'href="https://example.org/foo-0.0.1-py3-none-any.whl#sha256=cafefeed"', + ], + filename = "foo-0.0.1-py3-none-any.whl", + ) + html = _generate_html(input, valid) + + got = parse_simpleapi_html(content = html, logger = _LOGGER) + + env.expect.that_collection(got.whls).has_size(1) + env.expect.that_collection(got.sdists).has_size(0) + env.expect.that_str(got.whls["cafefeed"].filename).equals("foo-0.0.1-py3-none-any.whl") + env.expect.that_collection(got.whls.keys()).contains_exactly(["cafefeed"]) + +_tests.append(_test_malformed_whl_anchor_is_skipped) + def parse_simpleapi_html_test_suite(name): """Create the test suite. From 9e1e5d97d5a736e7a76860aa6ed73651c81dd3b6 Mon Sep 17 00:00:00 2001 From: Kris Foster Date: Mon, 6 Jul 2026 15:57:49 +0100 Subject: [PATCH 3/3] test: pass logger through simpleapi_download test mocks 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 --- .../pypi/simpleapi_download/simpleapi_download_tests.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl b/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl index 4e86b76e10..fe0d3ffee4 100644 --- a/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl +++ b/tests/pypi/simpleapi_download/simpleapi_download_tests.bzl @@ -24,7 +24,7 @@ _tests = [] def _test_simple(env): calls = [] - def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, parse_index): + def read_simpleapi(ctx, url, versions, attr, cache, get_auth, block, parse_index, logger): if parse_index: return struct( success = True, @@ -36,7 +36,7 @@ def _test_simple(env): }, ) - _ = ctx, attr, cache, get_auth, versions # buildifier: disable=unused-variable + _ = ctx, attr, cache, get_auth, versions, logger # buildifier: disable=unused-variable env.expect.that_bool(block).equals(False) calls.append(url) return struct( @@ -94,7 +94,7 @@ def _test_index_overrides(env): calls = [] fails = [] - def read_simpleapi(ctx, *, url, versions, attr, cache, get_auth, block, parse_index): + def read_simpleapi(ctx, *, url, versions, attr, cache, get_auth, block, parse_index, logger): if parse_index: return struct( success = True, @@ -108,7 +108,7 @@ def _test_index_overrides(env): }, ) - _ = ctx, attr, cache, get_auth, versions # buildifier: disable=unused-variable + _ = ctx, attr, cache, get_auth, versions, logger # buildifier: disable=unused-variable env.expect.that_bool(block).equals(False) calls.append(url) return struct(