fix(bundler): guard lazy .hostname ValueError in catalog add_source#3644
Merged
mnriem merged 1 commit intoJul 22, 2026
Merged
Conversation
A bracketed-but-invalid IPv6 authority (e.g. "https://[not-an-ip]/c.json") parses cleanly under urlparse() on Python < 3.14 and only raises ValueError lazily on the first .hostname access. add_source read parsed.hostname outside the try/except ValueError guard, so on the interpreters spec-kit supports (>=3.11) that raw ValueError leaked past the CLI's `except BundlerError`, surfacing an uncaught traceback instead of the clean "Invalid catalog url" domain error. (The raise moved eager into urlparse() only in 3.14.) Read parsed.hostname inside the try and reuse the value for both the HTTPS/localhost check and the require-host check. This also protects the later _derive_id() call on the same URL. Regression tests: a bracketed-non-IP URL, plus a monkeypatched lazy-.hostname raiser that reproduces the pre-3.14 shape independently of the running interpreter (fails with a raw ValueError before the fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Guards lazy .hostname parsing failures so malformed catalog URLs consistently raise BundlerError.
Changes:
- Reads and reuses
hostnameinside the existingValueErrorguard. - Adds regression coverage for bracketed invalid hosts and lazy hostname failures.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/bundler/commands_impl/catalog_config.py |
Prevents raw ValueError leakage from hostname parsing. |
tests/unit/test_bundler_catalog_config.py |
Adds cross-version regression tests. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Medium
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
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.
What
add_source()incatalog_config.pyreadparsed.hostnameoutside itstry/except ValueErrorguard. A bracketed-but-invalid IPv6 authority (e.g.https://[not-an-ip]/c.json) parses cleanly underurlparse()on Python < 3.14 and only raisesValueErrorlazily on the first.hostnameaccess. On the interpreters spec-kit supports (>=3.11), that rawValueErrorleaked past the CLI'sexcept BundlerError, surfacing an uncaught traceback instead of the cleanInvalid catalog urldomain error.(The
ValueErrormoved eager intourlparse()only in 3.14, so the pre-fix code happened to be safe there — but not on 3.11–3.13.)This is the same bug class as the maintainer-fixed #3433 / #3435 / #3437 (unguarded
urlparse().hostnameleaking a rawValueError).Fix
Read
parsed.hostnameinside thetryand reuse the value for both the HTTPS/localhost check and the require-host check. This also protects the later_derive_id()call on the same URL.Tests
test_add_source_wraps_bracketed_non_ip_host_as_bundler_error— the[not-an-ip]URL.test_add_source_wraps_lazy_hostname_valueerror— monkeypatches a lazy-.hostnameraiser to reproduce the pre-3.14 shape independently of the running interpreter. Test-the-test: fails with a rawValueErrorbefore the fix, passes after.Full
tests/unit/test_bundler_catalog_config.pypasses locally (the one unrelated failure is the Windows-without-elevation symlink test).🤖 Generated with Claude Code