fix(presets): validate catalog URL port, not just hostname - #4341
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix(presets): validate catalog URL port, not just hostname#4341Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
`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 <url>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJHJ2dHP2RVCNncHqN8Qm9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PresetCatalog._validate_catalog_urlis a standalone reimplementation (PresetCatalogdoes not inheritCatalogStackBase) of the same guard that exists inspecify_cli.catalogsandbundler/services/adapters.py, but it had drifted: it accessedparsed.hostnameinside the malformed-URL try/except but neverparsed.port.urlparse(url).hostnamedoes not perform port validation — only accessing.portdoes, lazily. So a catalog URL likehttps://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 cleanPresetValidationErrorthis function's docstring promises.preset add --from <url>download-URL guard already probes.portfor exactly this reason (seetest_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._ = parsed.portprobe inside the try/except, matchingspecify_cli.catalogs._validate_catalog_urlandbundler/services/adapters.py::_validate_remote_url.Test plan
test_validate_catalog_url_out_of_range_port_rejectedtotests/test_presets.py::TestPresetCatalog, next to the existing malformed-IPv6-URL test for the same function.PresetValidationErrorbut gotFailed: DID NOT RAISE— the malformed port silently passed validation.TestPresetCatalog/TestPresetCatalogMultiCatalog/TestPresetCatalogEntry/TestPresetCatalogRichMarkup(106 tests) and the fulltests/test_presets.pysuite (605 passed, 8 pre-existing Windows symlink-elevation failures needing admin rights, unrelated to this change, 2 skipped).Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01PJHJ2dHP2RVCNncHqN8Qm9