Skip to content

fix(bundler): guard lazy .hostname ValueError in catalog add_source#3644

Merged
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/catalog-add-hostname-valueerror
Jul 22, 2026
Merged

fix(bundler): guard lazy .hostname ValueError in catalog add_source#3644
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/catalog-add-hostname-valueerror

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

What

add_source() in catalog_config.py read parsed.hostname outside its try/except ValueError guard. 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. 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 ValueError moved eager into urlparse() 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().hostname leaking a raw ValueError).

Fix

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.

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-.hostname raiser to reproduce the pre-3.14 shape independently of the running interpreter. Test-the-test: fails with a raw ValueError before the fix, passes after.

Full tests/unit/test_bundler_catalog_config.py passes locally (the one unrelated failure is the Windows-without-elevation symlink test).

🤖 Generated with Claude Code

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Guards lazy .hostname parsing failures so malformed catalog URLs consistently raise BundlerError.

Changes:

  • Reads and reuses hostname inside the existing ValueError guard.
  • 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

@mnriem
mnriem merged commit bd90f76 into github:main Jul 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants