From cb7c36c95b04ab1e612177636e564ca5125c7bde Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Tue, 30 Jun 2026 17:18:39 +0500 Subject: [PATCH] fix: reject host-less catalog URLs in base and preset validators (#3209) (#3227) `CatalogStackBase._validate_catalog_url` (inherited by `IntegrationCatalog`) and `PresetCatalog._validate_catalog_url` checked `parsed.netloc`, which is truthy for host-less URLs like `https://:8080` (port only) or `https://user@` (userinfo only). Such URLs slipped past validation despite the error message promising "a valid URL with a host", then failed later with a confusing fetch error. Switch both validators to `parsed.hostname` (None for those inputs), matching the workflow, step, and bundler catalog validators that already do this. Add regression tests covering port-only and userinfo-only URLs for both validators. Co-authored-by: Claude Opus 4.8 (1M context) --- src/specify_cli/catalogs.py | 2 +- src/specify_cli/presets/__init__.py | 2 +- tests/integrations/test_integration_catalog.py | 11 ++++++----- tests/test_presets.py | 11 ++++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/specify_cli/catalogs.py b/src/specify_cli/catalogs.py index d14e8ec42..ec8014127 100644 --- a/src/specify_cli/catalogs.py +++ b/src/specify_cli/catalogs.py @@ -80,7 +80,7 @@ class CatalogStackBase: ) # Check hostname, not netloc: netloc is truthy for host-less URLs like # "https://:8080" or "https://user@", so the host guarantee this error - # promises would not actually hold. hostname is None in those cases. + # promises would not actually hold. hostname is None in those cases (#3209). if not parsed.hostname: raise cls._error("Catalog URL must be a valid URL with a host.") diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 354cc8239..cf89b788a 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1863,7 +1863,7 @@ class PresetCatalog: ) # Check hostname, not netloc: netloc is truthy for host-less URLs like # "https://:8080" or "https://user@", so the host guarantee this error - # promises would not actually hold. hostname is None in those cases. + # promises would not actually hold. hostname is None in those cases (#3209). if not parsed.hostname: raise PresetValidationError( "Catalog URL must be a valid URL with a host." diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 6b6831e05..274ec1315 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -70,16 +70,17 @@ class TestCatalogURLValidation: @pytest.mark.parametrize( "url", [ - "https://:8080", # port only, no host - "https://:0", # port only, no host - "https://user@", # userinfo only, no host - "https://user:pw@", # userinfo only, no host + "https://:8080", # port only, no host + "https://:8080/catalog.json", # port only, with path + "https://:0", # port only, no host + "https://user@", # userinfo only, no host + "https://user:pass@", # userinfo only, no host ], ) def test_hostless_url_with_truthy_netloc_rejected(self, url): # These have a truthy netloc (":8080", "user@") but no actual host, # so a netloc-based check would wrongly accept them despite the - # "valid URL with a host" promise. hostname is None for all of them. + # "valid URL with a host" promise. hostname is None for all of them (#3209). with pytest.raises(IntegrationCatalogError, match="valid URL"): IntegrationCatalog._validate_catalog_url(url) diff --git a/tests/test_presets.py b/tests/test_presets.py index 58dcdc711..ff3f3dffc 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1427,14 +1427,15 @@ class TestPresetCatalog: @pytest.mark.parametrize( "url", [ - "https://:8080", # port only, no host - "https://:0", # port only, no host - "https://user@", # userinfo only, no host - "https://user:pw@", # userinfo only, no host + "https://:8080", # port only, no host + "https://:8080/catalog.json", # port only, with path + "https://:0", # port only, no host + "https://user@", # userinfo only, no host + "https://user:pass@", # userinfo only, no host ], ) def test_validate_catalog_url_hostless_rejected(self, project_dir, url): - """Reject host-less URLs whose netloc is truthy but hostname is None. + """Reject host-less URLs whose netloc is truthy but hostname is None (#3209). ``urlparse('https://:8080').netloc`` is ``':8080'`` (truthy) but its ``hostname`` is ``None``, so a netloc-based check would accept a URL