Skip to content

Commit 4ec4c0f

Browse files
committed
fix: harden catalog edge cases
1 parent 8b7836c commit 4ec4c0f

4 files changed

Lines changed: 91 additions & 33 deletions

File tree

tests/test_catalog.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55

66
import pytest
77

8-
from tools.catalog import CatalogLoadError, load_catalog, validate_catalog
8+
from tools.catalog import (
9+
CatalogLoadError,
10+
load_catalog,
11+
normalize_url,
12+
validate_catalog,
13+
)
914
from tools.validate_catalog import run
1015

1116

@@ -49,6 +54,23 @@ def test_resource_ids_and_urls_reject_unsafe_forms(valid_catalog: dict) -> None:
4954
assert {"invalid-id", "url-credentials"} <= codes
5055

5156

57+
@pytest.mark.parametrize(
58+
("url", "expected"),
59+
[
60+
(
61+
"HTTPS://[2001:4860:4860::8888]:443/docs/",
62+
"https://[2001:4860:4860::8888]/docs",
63+
),
64+
(
65+
"https://[2001:4860:4860::8888]:8443/docs/",
66+
"https://[2001:4860:4860::8888]:8443/docs",
67+
),
68+
],
69+
)
70+
def test_normalize_url_preserves_ipv6_brackets(url: str, expected: str) -> None:
71+
assert normalize_url(url) == expected
72+
73+
5274
def test_validator_exit_code_for_invalid_catalog(tmp_path) -> None:
5375
catalog = tmp_path / "resources.yml"
5476
catalog.write_text("catalog: {}\nresources: []\n", encoding="utf-8")

tests/test_check_links.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,39 @@ def head(self, url, **kwargs):
265265
assert "programming defect" in (result.error or "")
266266

267267

268+
def test_responses_close_when_result_processing_fails(monkeypatch) -> None:
269+
head = FakeResponse(404)
270+
response = FakeResponse(200)
271+
session = FakeSession(head, response)
272+
checker = LinkChecker(
273+
guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
274+
)
275+
276+
def fail_history(_response):
277+
raise RuntimeError("cannot process response")
278+
279+
monkeypatch.setattr(check_links, "_history", fail_history)
280+
result = checker.check_one(link())
281+
282+
assert result.status == "error"
283+
assert head.closed is True
284+
assert response.closed is True
285+
286+
287+
@pytest.mark.parametrize(
288+
("option", "value"),
289+
[
290+
("--timeout", "nan"),
291+
("--timeout", "inf"),
292+
("--backoff", "nan"),
293+
("--min-interval", "inf"),
294+
],
295+
)
296+
def test_cli_rejects_non_finite_float_arguments(option: str, value: str) -> None:
297+
with pytest.raises(SystemExit, match="2"):
298+
check_links.build_parser().parse_args([option, value])
299+
300+
268301
@pytest.mark.parametrize(
269302
("status", "status_code", "expected_exit"),
270303
[

tools/catalog.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,10 @@ def normalize_url(value: str) -> str:
104104
default_port = (parsed.scheme.lower() == "https" and port == 443) or (
105105
parsed.scheme.lower() == "http" and port == 80
106106
)
107-
netloc = host
107+
display_host = f"[{host}]" if ":" in host else host
108+
netloc = display_host
108109
if port and not default_port:
109-
netloc = f"{host}:{port}"
110+
netloc = f"{display_host}:{port}"
110111
path = parsed.path or "/"
111112
if path != "/":
112113
path = path.rstrip("/")

tools/check_links.py

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import argparse
77
import ipaddress
88
import json
9+
import math
910
import socket
1011
import sys
1112
import threading
@@ -321,38 +322,39 @@ def check_one(self, link: CatalogLink) -> LinkResult:
321322
self.guard.resolve_url(link.url)
322323
session = self.sessions.get()
323324
head = self._request(session, "HEAD", link.url)
324-
if head.status_code < 400:
325-
history = _history(head)
326-
result = LinkResult(
325+
try:
326+
if head.status_code < 400:
327+
history = _history(head)
328+
return LinkResult(
329+
link.id,
330+
link.path,
331+
link.title,
332+
link.url,
333+
classify_status(head.status_code, redirected=bool(history)),
334+
status_code=head.status_code,
335+
method="HEAD",
336+
final_url=head.url,
337+
history=history,
338+
)
339+
finally:
340+
head.close()
341+
342+
response = self._request(session, "GET", link.url)
343+
try:
344+
history = _history(response)
345+
return LinkResult(
327346
link.id,
328347
link.path,
329348
link.title,
330349
link.url,
331-
classify_status(head.status_code, redirected=bool(history)),
332-
status_code=head.status_code,
333-
method="HEAD",
334-
final_url=head.url,
350+
classify_status(response.status_code, redirected=bool(history)),
351+
status_code=response.status_code,
352+
method="GET",
353+
final_url=response.url,
335354
history=history,
336355
)
337-
head.close()
338-
return result
339-
head.close()
340-
341-
response = self._request(session, "GET", link.url)
342-
history = _history(response)
343-
result = LinkResult(
344-
link.id,
345-
link.path,
346-
link.title,
347-
link.url,
348-
classify_status(response.status_code, redirected=bool(history)),
349-
status_code=response.status_code,
350-
method="GET",
351-
final_url=response.url,
352-
history=history,
353-
)
354-
response.close()
355-
return result
356+
finally:
357+
response.close()
356358
except UnsafeTarget as exc:
357359
return LinkResult(
358360
link.id, link.path, link.title, link.url, "blocked", error=str(exc)
@@ -451,15 +453,15 @@ def exit_code_for_report(report: Mapping[str, Any]) -> int:
451453

452454
def _positive_float(value: str) -> float:
453455
parsed = float(value)
454-
if parsed <= 0:
455-
raise argparse.ArgumentTypeError("must be greater than zero")
456+
if not math.isfinite(parsed) or parsed <= 0:
457+
raise argparse.ArgumentTypeError("must be a finite number greater than zero")
456458
return parsed
457459

458460

459461
def _non_negative_float(value: str) -> float:
460462
parsed = float(value)
461-
if parsed < 0:
462-
raise argparse.ArgumentTypeError("must be zero or greater")
463+
if not math.isfinite(parsed) or parsed < 0:
464+
raise argparse.ArgumentTypeError("must be a finite number zero or greater")
463465
return parsed
464466

465467

0 commit comments

Comments
 (0)