Skip to content

Commit 5941491

Browse files
test(workflows): cover post-redirect malformed-URL guard (#3484 review)
Copilot review asked for regression tests on the fetch-path validators that re-check resp.geturl() after redirects — the branch that turns a malformed redirect target into a domain error instead of a raw ValueError. - test_fetch_malformed_redirect_target_raises_catalog_error on both TestWorkflowCatalog and TestStepCatalog: stub open_url with a response whose geturl() is malformed (https://[::1 / https://[not-an-ip]/x) while entry.url is valid, so validation only trips on the redirect target, and assert _fetch_single_catalog raises WorkflowCatalogError / StepCatalogError with a "malformed" message (force_refresh + fresh project_dir so no cache masks it). - Test-the-test: both fail on pre-fix source (raw ValueError re-wrapped as "...Invalid IPv6 URL", no "malformed" match) and pass with the guard. Also merges latest upstream/main into the branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5aa0a63 commit 5941491

1 file changed

Lines changed: 103 additions & 0 deletions

File tree

tests/test_workflows.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5097,6 +5097,58 @@ def test_validate_url_malformed_raises_validation_error(self, project_dir, url):
50975097
with pytest.raises(WorkflowValidationError, match="malformed"):
50985098
catalog._validate_catalog_url(url)
50995099

5100+
def test_fetch_malformed_redirect_target_raises_catalog_error(
5101+
self, project_dir, monkeypatch
5102+
):
5103+
"""A malformed post-redirect URL must raise WorkflowCatalogError, not a
5104+
raw ValueError.
5105+
5106+
The fetch path re-validates ``resp.geturl()`` after following redirects,
5107+
so a hostile/broken redirect to a malformed authority
5108+
(``https://[::1``) hits ``urlparse``/``.hostname`` and raises
5109+
``ValueError``. Without the guard that ValueError is re-wrapped by the
5110+
broad ``except`` as ``Failed to fetch catalog ...: Invalid IPv6 URL``;
5111+
the guard turns it into a clean ``... malformed URL ...`` refusal. The
5112+
initial ``entry.url`` is valid so validation only trips on the redirect
5113+
target. Mirrors specify_cli.catalogs (#3435).
5114+
"""
5115+
from specify_cli.workflows.catalog import (
5116+
WorkflowCatalog,
5117+
WorkflowCatalogEntry,
5118+
WorkflowCatalogError,
5119+
)
5120+
from specify_cli.authentication import http as auth_http
5121+
5122+
class _FakeResponse:
5123+
def __enter__(self):
5124+
return self
5125+
5126+
def __exit__(self, exc_type, exc, tb):
5127+
return False
5128+
5129+
def read(self):
5130+
return b"{}"
5131+
5132+
def geturl(self):
5133+
# A redirect landing on a malformed IPv6 authority.
5134+
return "https://[::1"
5135+
5136+
monkeypatch.setattr(
5137+
auth_http, "open_url", lambda url, timeout=30: _FakeResponse()
5138+
)
5139+
5140+
catalog = WorkflowCatalog(project_dir)
5141+
entry = WorkflowCatalogEntry(
5142+
url="https://example.com/catalog.json",
5143+
name="test",
5144+
priority=1,
5145+
install_allowed=True,
5146+
)
5147+
# A fresh project_dir has no cache to fall back to, so the error
5148+
# propagates instead of being masked by a stale-cache read.
5149+
with pytest.raises(WorkflowCatalogError, match="malformed"):
5150+
catalog._fetch_single_catalog(entry, force_refresh=True)
5151+
51005152
def test_add_catalog(self, project_dir):
51015153
from specify_cli.workflows.catalog import WorkflowCatalog
51025154

@@ -5561,6 +5613,57 @@ def test_validate_url_malformed_raises_validation_error(self, project_dir, url):
55615613
with pytest.raises(StepValidationError, match="malformed"):
55625614
catalog._validate_catalog_url(url)
55635615

5616+
def test_fetch_malformed_redirect_target_raises_catalog_error(
5617+
self, project_dir, monkeypatch
5618+
):
5619+
"""A malformed post-redirect URL must raise StepCatalogError, not a raw
5620+
ValueError.
5621+
5622+
The fetch path re-validates ``resp.geturl()`` after redirects, so a
5623+
broken redirect to a bracketed non-IP host (``https://[not-an-ip]/x``)
5624+
makes ``urlparse``/``.hostname`` raise ``ValueError``. Without the guard
5625+
that leaks out as ``... Invalid IPv6 URL`` re-wrapping; the guard turns
5626+
it into a clean ``... malformed URL ...`` refusal. The initial
5627+
``entry.url`` is valid so validation only trips on the redirect target.
5628+
Mirrors specify_cli.catalogs (#3435).
5629+
"""
5630+
from specify_cli.workflows.catalog import (
5631+
StepCatalog,
5632+
StepCatalogEntry,
5633+
StepCatalogError,
5634+
)
5635+
from specify_cli.authentication import http as auth_http
5636+
5637+
class _FakeResponse:
5638+
def __enter__(self):
5639+
return self
5640+
5641+
def __exit__(self, exc_type, exc, tb):
5642+
return False
5643+
5644+
def read(self):
5645+
return b"{}"
5646+
5647+
def geturl(self):
5648+
# A redirect landing on a bracketed non-IP authority.
5649+
return "https://[not-an-ip]/x"
5650+
5651+
monkeypatch.setattr(
5652+
auth_http, "open_url", lambda url, timeout=30: _FakeResponse()
5653+
)
5654+
5655+
catalog = StepCatalog(project_dir)
5656+
entry = StepCatalogEntry(
5657+
url="https://example.com/steps.json",
5658+
name="test",
5659+
priority=1,
5660+
install_allowed=True,
5661+
)
5662+
# A fresh project_dir has no cache to fall back to, so the error
5663+
# propagates instead of being masked by a stale-cache read.
5664+
with pytest.raises(StepCatalogError, match="malformed"):
5665+
catalog._fetch_single_catalog(entry, force_refresh=True)
5666+
55645667
def test_add_catalog(self, project_dir):
55655668
from specify_cli.workflows.catalog import StepCatalog
55665669

0 commit comments

Comments
 (0)