From eeb109a0d039e5a2606c350fb8848be7f0cc76a7 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Wed, 26 Aug 2026 17:49:22 +0500 Subject: [PATCH] fix(presets): validate catalog URL port, not just hostname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PresetCatalog._validate_catalog_url` is a standalone reimplementation (PresetCatalog does not inherit `CatalogStackBase`) of the same guard that exists in `specify_cli.catalogs` and `bundler/services/adapters.py`, but it had drifted: it accessed `parsed.hostname` inside the malformed-URL try/except but never `parsed.port`. `urlparse(url).hostname` does not perform port validation — only accessing `.port` does, lazily. So a catalog URL like `https://example.com:99999/catalog.json` (or a non-numeric port) sailed straight through this validator with no error at all, and would only fail later, at actual fetch time, with a raw untranslated exception instead of the clean `PresetValidationError` this function's docstring promises. The sibling `preset add --from ` download-URL guard already probes `.port` for exactly this reason (see `test_preset_add_from_url_out_of_range_port_exits_cleanly`) — this catalog-*source*-URL validator is a different function that just never got the same fix applied to it. Fix: add the same `_ = parsed.port` probe inside the try/except, matching `specify_cli.catalogs._validate_catalog_url` and `bundler/services/adapters.py::_validate_remote_url`. ## Test plan - Added `test_validate_catalog_url_out_of_range_port_rejected` to `tests/test_presets.py::TestPresetCatalog`, next to the existing malformed-IPv6-URL test for the same function. - Verified the test fails without the fix (test-the-test): asserted `PresetValidationError` but got `Failed: DID NOT RAISE` — the malformed port silently passed validation. - Ran `TestPresetCatalog`/`TestPresetCatalogMultiCatalog`/ `TestPresetCatalogEntry`/`TestPresetCatalogRichMarkup` (106 tests) and the full `tests/test_presets.py` suite (605 passed, 8 pre-existing Windows symlink-elevation failures needing admin rights, unrelated to this change, 2 skipped). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PJHJ2dHP2RVCNncHqN8Qm9 --- src/specify_cli/presets/__init__.py | 7 +++++++ tests/test_presets.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index a5cea4f958..b8c8f390db 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4165,6 +4165,13 @@ def _validate_catalog_url(self, url: str) -> None: 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 a raw error this function does not translate + # into PresetValidationError. Mirrors specify_cli.catalogs and + # bundler/services/adapters.py's copy of this same guard. + _ = parsed.port except ValueError: raise PresetValidationError(f"Catalog URL is malformed: {url}") from None is_localhost = hostname in ("localhost", "127.0.0.1", "::1") diff --git a/tests/test_presets.py b/tests/test_presets.py index f30ab4909e..8f9a2e358f 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2086,6 +2086,25 @@ def test_validate_catalog_url_malformed_rejected(self, project_dir): with pytest.raises(PresetValidationError, match="malformed"): catalog._validate_catalog_url("https://[::1") + def test_validate_catalog_url_out_of_range_port_rejected(self, project_dir): + """An out-of-range port raises ValueError lazily on ``.port`` access. + + ``urlparse(...).hostname`` alone does not validate the port, so + without a ``_ = parsed.port`` probe inside the try/except, a URL like + ``https://example.com:99999/catalog.json`` sails through this + validator and only fails later, at fetch time, with a raw + untranslated error instead of a clean ``PresetValidationError``. The + sibling ``preset add --from `` download-URL guard already + catches this shape (see + ``test_preset_add_from_url_out_of_range_port_exits_cleanly``); this + catalog-source-URL validator had drifted from it and from the + original guard in ``specify_cli.catalogs``/ + ``bundler/services/adapters.py``. + """ + catalog = PresetCatalog(project_dir) + with pytest.raises(PresetValidationError, match="malformed"): + catalog._validate_catalog_url("https://example.com:99999/catalog.json") + def test_env_var_catalog_url(self, project_dir, monkeypatch): """Test catalog URL from environment variable.""" monkeypatch.setenv("SPECKIT_PRESET_CATALOG_URL", "https://custom.example.com/catalog.json")