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

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
+            # `<a href=".../foo.whl#sha256=...">foo.whl</a>`. 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

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)

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,
         )

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:
@@ -296,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.
 

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(