fix(catalogs): validate the port in the shared catalog-URL validator, like its mirrors do (#3804)

* fix(catalogs): validate the port in the shared catalog-URL validator

`CatalogStackBase._validate_catalog_url()` reads `parsed.hostname` inside
its `try/except ValueError` but never reads `parsed.port`. `urlparse()` and
`.hostname` do not perform port validation — only `.port` does — so a
catalog URL with a non-numeric or out-of-range port passes validation.

Every implementation that documents itself as mirroring this function
already reads `.port` inside the same try: workflows/catalog.py (4 sites),
bundler/services/adapters.py (2), bundler/commands_impl/catalog_config.py,
and commands/bundle/__init__.py. The shared base — inherited by
ExtensionCatalog and IntegrationCatalog — is the only one without it.

The accepted URL then escapes as a raw `http.client.InvalidURL`, which is
neither `urllib.error.URLError` nor `json.JSONDecodeError` (the only two
the fetcher converts), so it surfaces as an unhandled traceback rather
than the validator's normal error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(catalogs): describe both bad-port failure modes accurately

The comment attributed both malformed-port cases to
http.client.InvalidURL. Only a non-numeric port raises that (when the
connection object is built); an out-of-range port constructs fine and
fails later in the socket layer. Measured:

  example.invalid:notaport -> http.client.InvalidURL: nonnumeric port
  example.invalid:65536    -> HTTPSConnection() OK, connect() fails

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-30 23:41:17 +05:00
committed by GitHub
parent 43a54bf2d6
commit e4318a3d1a
2 changed files with 13 additions and 3 deletions

View File

@@ -74,6 +74,13 @@ class CatalogStackBase:
try:
parsed = urlparse(url)
hostname = parsed.hostname
# Accessing ``port`` performs urllib's syntax/range validation;
# ``hostname`` alone does not, so a non-numeric or out-of-range
# port would otherwise pass validation here and only fail later,
# at fetch time, as an error this module does not translate --
# a raw http.client.InvalidURL for a non-numeric port, and a
# socket-layer failure for one that is merely out of range.
_ = parsed.port
except ValueError:
raise cls._error(f"Catalog URL is malformed: {url}") from None
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")

View File

@@ -116,12 +116,15 @@ class TestCatalogURLValidation:
[
"https://[::1", # unclosed ipv6 bracket
"https://[not-an-ip]/c.json", # bracketed non-ip host
"https://example.com:notaport/c.json", # non-numeric port
"https://example.com:65536/c.json", # out-of-range port
],
)
def test_malformed_url_rejected_cleanly(self, url):
# A malformed authority makes urlparse/hostname raise ValueError. The
# validator must turn that into its normal catalog error, not leak a
# raw ValueError to the caller.
# A malformed authority makes urlparse/hostname raise ValueError, and a
# bad port makes ``parsed.port`` raise it. The validator must turn that
# into its normal catalog error, not leak a raw ValueError to the caller
# (or, for a bad port, accept the URL and fail later at fetch time).
with pytest.raises(IntegrationCatalogError, match="malformed"):
IntegrationCatalog._validate_catalog_url(url)