fix(bundler): raise BundlerError, not raw ValueError, on a malformed catalog URL#3433
Merged
mnriem merged 1 commit intoJul 10, 2026
Merged
Conversation
…catalog URL adapters._validate_remote_url reads parsed.hostname, which raises ValueError on a malformed authority (e.g. an unclosed ipv6 bracket https://[::1). the function's contract is to raise BundlerError for any bad url - every other reject path does - so the raw ValueError leaked to the caller and crashed the fetch instead of failing cleanly. wrap the parse and convert to BundlerError. bundler sibling of github#3369, which fixed the cli extension/preset/workflow add paths but not this validator. added a regression test that fails pre-fix.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes bundler catalog URL validation to consistently raise BundlerError (not a raw ValueError) when urlparse(...).hostname fails on malformed authorities (e.g., unclosed IPv6 brackets), aligning with the validator’s documented contract and ensuring callers fail cleanly.
Changes:
- Wrapped
urlparse()+parsed.hostnameaccess in_validate_remote_url()to convertValueErrorinto aBundlerError. - Reused the parsed
hostnamefor the existing localhost + host-presence checks. - Added a regression test covering malformed bracketed hosts (unclosed IPv6 bracket and non-IP bracket host).
Show a summary per file
| File | Description |
|---|---|
| src/specify_cli/bundler/services/adapters.py | Converts malformed-URL ValueError into BundlerError and reuses hostname for subsequent validation. |
| tests/unit/test_bundler_adapters.py | Adds regression coverage ensuring malformed URLs raise BundlerError instead of leaking ValueError. |
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: Low
Collaborator
|
Thank you! |
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.
fixes #3432
_validate_remote_urlreadparsed.hostname, which raisesValueErroron a malformed authority (e.g. an unclosed ipv6 brackethttps://[::1). the function's contract is to raiseBundlerErrorfor any bad url - every other reject path does - so the raw ValueError leaked to the caller and crashed the fetch instead of failing cleanly.bundler sibling of #3369, which fixed the cli extension/preset/workflow add paths but not this validator (reached via
bundle catalog add, #3367).fix: wrap the parse + hostname access and convert
ValueErrortoBundlerError, then reuse the parsed hostname for the existing host check.added a regression test (
test_validate_remote_url_rejects_malformed_url_cleanly) covering an unclosed ipv6 bracket and a non-ip bracket host. i confirmed it fails on the pre-fix code by stashing the source and re-running; the adapters suite passes with the fix.