From e4318a3d1a07d60325399ba81890e0205793c5ec Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:41:17 +0500 Subject: [PATCH] fix(catalogs): validate the port in the shared catalog-URL validator, like its mirrors do (#3804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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) --------- Co-authored-by: Claude Opus 5 (1M context) --- src/specify_cli/catalogs.py | 7 +++++++ tests/integrations/test_integration_catalog.py | 9 ++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/catalogs.py b/src/specify_cli/catalogs.py index 774aaa51d..323bb4f74 100644 --- a/src/specify_cli/catalogs.py +++ b/src/specify_cli/catalogs.py @@ -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") diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 4688c6a21..e8a9029db 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -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)