Skip to content

fix(github-http): return None on malformed host in resolve_github_release_asset_api_url#3715

Merged
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/gh-asset-resolve-hostname-valueerror
Jul 24, 2026
Merged

fix(github-http): return None on malformed host in resolve_github_release_asset_api_url#3715
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/gh-asset-resolve-hostname-valueerror

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

resolve_github_release_asset_api_url() documents a resolve-or-return-None contract ("Returns ... None when the URL is not a resolvable GitHub release download or the lookup fails"), but reading the parsed authority could raise ValueError for a malformed host and leak a raw traceback past the caller.

The function already guards the twin case for parsed.port (malformed port -> return None). This applies the same guard to the host read: on Python 3.14 urlparse() itself raises ValueError for an invalid bracketed IPv6 host such as https://[not-an-ip]/...; on older versions the raise happens on .hostname. Wrapping both urlparse and .hostname in the try covers every version.

This matters because download_url is server-controlled — it comes from a catalog download_url payload — so a malformed value must not crash resolution.

Test plan

  • Added test_returns_none_on_malformed_host, mirroring the existing test_returns_none_on_malformed_ghes_port. It fails without the fix (raw ValueError) and passes with it.
  • pytest tests/test_github_http.py -> 30 passed.

🤖 Generated with Claude Code

…ease_asset_api_url

Accessing the parsed authority (via urlparse/.hostname) raises ValueError
on a malformed bracketed host, e.g. https://[not-an-ip]/..., mirroring
the existing .port guard below. download_url is server-controlled (a
catalog download_url payload), so the function's resolve-or-return-None
contract must hold rather than leaking a raw traceback to the caller.

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

Prevents malformed GitHub release hosts from violating the resolver’s return-None contract.

Changes:

  • Catches ValueError during URL parsing and hostname access.
  • Adds regression coverage for invalid bracketed IPv6 hosts.
Show a summary per file
File Description
src/specify_cli/_github_http.py Safely rejects malformed authorities.
tests/test_github_http.py Verifies malformed hosts return None.

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 3675452 into github:main Jul 24, 2026
14 checks passed
@mnriem

mnriem commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

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