Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/specify_cli/commands/bundle/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,16 @@ def _download_manifest(resolved, *, offline: bool):
f"Catalog entry '{resolved.entry.id}' has no download_url; cannot resolve "
"its manifest."
)
parsed = urlparse(url)
# A malformed authority (e.g. an unclosed IPv6 bracket ``https://[::1``)
# makes urlparse raise ValueError. Surface it as the documented
# BundlerError, like the sibling ``_validate_remote_url``, rather than
# leaking a raw ValueError past the callers, which only catch BundlerError.
try:
parsed = urlparse(url)
except ValueError:
raise BundlerError(
f"Catalog entry '{resolved.entry.id}' has a malformed download_url: {url}"
) from None
scheme = parsed.scheme.lower()

# ``file://`` URLs and bare filesystem paths (including Windows drive paths
Expand Down Expand Up @@ -802,8 +811,17 @@ def _download_manifest(resolved, *, offline: bool):
def _require_https(label: str, url: str) -> None:
from urllib.parse import urlparse

parsed = urlparse(url)
is_localhost = parsed.hostname in ("localhost", "127.0.0.1", "::1")
# urlparse / hostname access raise ValueError on a malformed authority;
# keep the documented BundlerError contract (older Pythons surface this via
# the .hostname access below rather than at the urlparse call).
try:
parsed = urlparse(url)
hostname = parsed.hostname
except ValueError:
raise BundlerError(
f"Refusing to download {label}: URL is malformed: {url}"
) from None
is_localhost = hostname in ("localhost", "127.0.0.1", "::1")
if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost):
raise BundlerError(
f"Refusing to download {label} over non-HTTPS URL: {url}"
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/test_bundle_download_url.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""Unit tests for malformed download-URL handling in bundle manifest resolution."""
from __future__ import annotations

from types import SimpleNamespace

import pytest

from specify_cli.bundler import BundlerError
from specify_cli.commands.bundle import _download_manifest, _require_https

_MALFORMED_URLS = [
"https://[::1", # unclosed IPv6 bracket
"https://[not-an-ip]/bundle.yml",
]


@pytest.mark.parametrize("url", _MALFORMED_URLS)
def test_download_manifest_rejects_malformed_url_cleanly(url):
"""A malformed download_url must raise BundlerError, not a raw ValueError.

``urlparse`` raises ``ValueError`` on a malformed authority (e.g. an
unclosed IPv6 bracket). The bundle CLI commands only catch BundlerError, so
a raw ValueError would escape as an uncaught traceback. Sibling of the
guarded ``_validate_remote_url`` (adapters) and the merged #3576 fix.
"""
resolved = SimpleNamespace(
entry=SimpleNamespace(id="mybundle", download_url=url)
)
with pytest.raises(BundlerError):
_download_manifest(resolved, offline=True)


@pytest.mark.parametrize("url", _MALFORMED_URLS)
def test_require_https_rejects_malformed_url_cleanly(url):
"""``_require_https`` must also surface BundlerError on a malformed authority.

On older Python versions the ValueError is raised at ``.hostname`` access
rather than at ``urlparse``, so guarding both keeps the contract across the
CI Python matrix.
"""
with pytest.raises(BundlerError):
_require_https("bundle 'x'", url)