fix(bundler): raise BundlerError, not raw ValueError, on a malformed catalog URL (#3433)

adapters._validate_remote_url reads parsed.hostname, which raises ValueError
on a malformed authority (e.g. an unclosed ipv6 bracket https://[::1). the
function's contract is to raise BundlerError for any bad url - every other
reject path does - so the raw ValueError leaked to the caller and crashed the
fetch instead of failing cleanly. wrap the parse and convert to BundlerError.

bundler sibling of #3369, which fixed the cli extension/preset/workflow add
paths but not this validator. added a regression test that fails pre-fix.

Author: Quratulain-bilal

src/specify_cli/bundler/services/adapters.py

@@ -68,8 +68,18 @@ def _validate_remote_url(source_id: str, url: str) -> None:
     Mirrors ``specify_cli.catalogs`` URL validation to avoid MITM/downgrade
     issues before any network call.
     """
-    parsed = urlparse(url)
-    is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
+    # A malformed authority (e.g. an unclosed IPv6 bracket ``https://[::1``)
+    # makes urlparse / hostname access raise ValueError. This function's
+    # contract is to raise BundlerError for a bad URL, so surface that as a
+    # clean error rather than leaking a raw ValueError to the caller.
+    try:
+        parsed = urlparse(url)
+        hostname = parsed.hostname
+    except ValueError:
+        raise BundlerError(
+            f"Catalog '{source_id}' URL is malformed: {url}"
+        ) from None
+    is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
     if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
         raise BundlerError(
             f"Catalog '{source_id}' URL must use HTTPS (got {parsed.scheme}://). "
@@ -79,7 +89,7 @@ def _validate_remote_url(source_id: str, url: str) -> None:
     # "https://:8080" or "https://user@...", so requiring netloc would let
     # those through even though they carry no host. hostname is None in those
     # cases. Mirrors the fix in ``specify_cli.catalogs`` (#3210).
-    if not parsed.hostname:
+    if not hostname:
         raise BundlerError(
             f"Catalog '{source_id}' URL must be a valid URL with a host: {url}"
         )

tests/unit/test_bundler_adapters.py

@@ -96,3 +96,21 @@ def test_validate_remote_url_rejects_host_less_urls(url):
 def test_validate_remote_url_accepts_normal_https_url():
     # Sanity: a real host with a port still passes.
     adapters._validate_remote_url("team", "https://example.com:8080/c.json")
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "https://[::1",  # unclosed IPv6 bracket
+        "https://[not-an-ip]/c.json",
+    ],
+)
+def test_validate_remote_url_rejects_malformed_url_cleanly(url):
+    """A malformed URL must raise BundlerError, not a raw ValueError.
+
+    ``urlparse``/``hostname`` raise ``ValueError`` on a malformed authority
+    (e.g. an unclosed IPv6 bracket). The validator's contract is to raise
+    BundlerError for any bad URL, so the raw ValueError must not escape to the
+    caller. Bundler sibling of #3369."""
+    with pytest.raises(BundlerError):
+        adapters._validate_remote_url("team", url)