fix: make redirect and retry handling strict

Author: xxg1413

tests/test_check_links.py

@@ -122,11 +122,61 @@ def test_head_failure_falls_back_to_streaming_get_and_confirms_404() -> None:
     assert session.calls[1][2]["stream"] is True
 
 
+def test_head_redirect_without_location_falls_back_to_get() -> None:
+    head = FakeResponse(301)
+    response = FakeResponse(200)
+    session = FakeSession(head, response)
+    checker = LinkChecker(
+        guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
+    )
+
+    result = checker.check_one(link())
+
+    assert result.status == "working"
+    assert result.method == "GET"
+    assert head.closed is True
+    assert response.closed is True
+
+
+def test_get_redirect_without_location_is_fatal() -> None:
+    head = FakeResponse(301)
+    response = FakeResponse(301)
+    session = FakeSession(head, response)
+    checker = LinkChecker(
+        guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
+    )
+
+    result = checker.check_one(link())
+
+    assert result.status == "error"
+    assert "no Location" in (result.error or "")
+    assert head.closed is True
+    assert response.closed is True
+
+
+def test_unsupported_redirect_status_is_fatal() -> None:
+    response = FakeResponse(304)
+    session = FakeSession(response)
+    checker = LinkChecker(
+        guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
+    )
+
+    result = checker.check_one(link())
+
+    assert result.status == "error"
+    assert "unsupported redirect status 304" in (result.error or "")
+    assert response.closed is True
+
+
 @pytest.mark.parametrize("status_code", [403, 408, 425, 429, 500, 503])
 def test_transient_and_access_denied_statuses_need_review(status_code) -> None:
     session = FakeSession(FakeResponse(status_code), FakeResponse(status_code))
     checker = LinkChecker(
-        guard=guard_for(), session_factory=lambda: session, workers=1, min_interval=0
+        guard=guard_for(),
+        session_factory=lambda: session,
+        workers=1,
+        retries=0,
+        min_interval=0,
     )
     assert checker.check_one(link()).status == "review"
 
@@ -347,10 +397,43 @@ def test_retry_configuration_ignores_unbounded_retry_after() -> None:
         retry = session.get_adapter("https://").max_retries
         assert retry.respect_retry_after_header is False
         assert retry.backoff_max == 5.0
+        assert retry.status == 0
+        assert not retry.status_forcelist
     finally:
         session.close()
 
 
+def test_status_retry_is_manual_and_closes_each_response() -> None:
+    first = FakeResponse(503)
+    second = FakeResponse(200)
+
+    class RetrySession(FakeSession):
+        def __init__(self):
+            super().__init__(first)
+            self.responses = iter([first, second])
+
+        def head(self, url, **kwargs):
+            self.calls.append(("HEAD", url, kwargs))
+            return next(self.responses)
+
+    session = RetrySession()
+    checker = LinkChecker(
+        guard=guard_for(),
+        session_factory=lambda: session,
+        workers=1,
+        retries=1,
+        backoff_factor=0,
+        min_interval=0,
+    )
+
+    result = checker.check_one(link())
+
+    assert result.status == "working"
+    assert len(session.calls) == 2
+    assert first.closed is True
+    assert second.closed is True
+
+
 def test_responses_close_when_result_processing_fails(monkeypatch) -> None:
     head = FakeResponse(404)
     response = FakeResponse(200)

tools/check_links.py

@@ -36,6 +36,7 @@
 RETRY_STATUS_CODES = {408, 425, 429, 500, 502, 503, 504}
 REDIRECT_STATUS_CODES = {301, 302, 303, 307, 308}
 MAX_REDIRECTS = 5
+MAX_BACKOFF_SECONDS = 5.0
 KNOWN_METADATA_HOSTS = {
     "instance-data",
     "metadata",
@@ -58,6 +59,10 @@ class UnsafeTarget(ValueError):
     """Raised before a request can reach a non-public network target."""
 
 
+class RedirectProtocolError(requests.RequestException):
+    """Raised when a redirect response cannot be followed safely."""
+
+
 @dataclass(frozen=True)
 class ResolvedTarget:
     host: str
@@ -198,11 +203,11 @@ def build_session(
         total=retries,
         connect=retries,
         read=retries,
-        status=retries,
         allowed_methods=frozenset({"HEAD", "GET"}),
-        status_forcelist=RETRY_STATUS_CODES,
+        status=0,
+        status_forcelist=frozenset(),
         backoff_factor=backoff_factor,
-        backoff_max=5.0,
+        backoff_max=MAX_BACKOFF_SECONDS,
         respect_retry_after_header=False,
         raise_on_status=False,
     )
@@ -319,6 +324,8 @@ def __init__(
     ) -> None:
         self.timeout = timeout
         self.workers = workers
+        self.status_retries = retries
+        self.backoff_factor = backoff_factor
         self.guard = guard or SafeTargetGuard()
         factory = session_factory or (
             lambda: build_session(
@@ -333,6 +340,7 @@ def _request(
     ) -> tuple[Any, list[dict[str, Any]]]:
         current_url = url
         history: list[dict[str, Any]] = []
+        status_attempt = 0
         while True:
             self.guard.resolve_url(current_url)
             self.rate_limiter.wait(current_url)
@@ -346,9 +354,34 @@ def _request(
                 if method == "GET"
                 else session.head(current_url, **kwargs)
             )
+            if (
+                response.status_code in RETRY_STATUS_CODES
+                and status_attempt < self.status_retries
+            ):
+                response.close()
+                delay = min(
+                    self.backoff_factor * (2**status_attempt), MAX_BACKOFF_SECONDS
+                )
+                status_attempt += 1
+                if delay > 0:
+                    time.sleep(delay)
+                continue
+
             location = response.headers.get("Location")
-            if response.status_code not in REDIRECT_STATUS_CODES or not location:
+            if not 300 <= response.status_code < 400:
                 return response, history
+            if response.status_code not in REDIRECT_STATUS_CODES:
+                response.close()
+                raise RedirectProtocolError(
+                    f"unsupported redirect status {response.status_code}"
+                )
+            if not location:
+                if method == "HEAD":
+                    return response, history
+                response.close()
+                raise RedirectProtocolError(
+                    f"redirect status {response.status_code} has no Location header"
+                )
 
             try:
                 if len(history) >= MAX_REDIRECTS:
@@ -371,13 +404,14 @@ def _request(
             finally:
                 response.close()
             current_url = next_url
+            status_attempt = 0
 
     def check_one(self, link: CatalogLink) -> LinkResult:
         try:
             session = self.sessions.get()
             head, head_history = self._request(session, "HEAD", link.url)
             try:
-                if head.status_code < 400:
+                if head.status_code < 300:
                     return LinkResult(
                         link.id,
                         link.path,