linkcheck: Fix linkcheck_allowed_redirects default sentinel (#13483)

Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>

Author: jayaddison

CHANGES.rst

@@ -47,7 +47,7 @@ Features added
   Patch by Till Hoffmann.
 * #13439: linkcheck: Permit warning on every redirect with
   ``linkcheck_allowed_redirects = {}``.
-  Patch by Adam Turner.
+  Patch by Adam Turner and James Addison.
 * #13497: Support C domain objects in the table of contents.
 * #13500: LaTeX: add support for ``fontawesome6`` package.
   Patch by Jean-François B.

sphinx/builders/linkcheck.py

@@ -71,6 +71,15 @@ class _Status(StrEnum):
 DEFAULT_DELAY = 60.0
 
 
+@object.__new__
+class _SENTINEL_LAR:
+    def __repr__(self) -> str:
+        return '_SENTINEL_LAR'
+
+    def __reduce__(self) -> str:
+        return self.__class__.__name__
+
+
 class CheckExternalLinksBuilder(DummyBuilder):
     """Checks for broken external links."""
 
@@ -179,7 +188,7 @@ def process_result(self, result: CheckResult) -> None:
                         text = 'with unknown code'
                 linkstat['text'] = text
                 redirection = f'{text} to {result.message}'
-                if self.config.linkcheck_allowed_redirects is not None:
+                if self.config.linkcheck_allowed_redirects is not _SENTINEL_LAR:
                     msg = f'redirect  {res_uri} - {redirection}'
                     logger.warning(msg, location=(result.docname, result.lineno))
                 else:
@@ -387,7 +396,7 @@ def __init__(
         )
         self.check_anchors: bool = config.linkcheck_anchors
         self.allowed_redirects: dict[re.Pattern[str], re.Pattern[str]]
-        self.allowed_redirects = config.linkcheck_allowed_redirects or {}
+        self.allowed_redirects = config.linkcheck_allowed_redirects
         self.retries: int = config.linkcheck_retries
         self.rate_limit_timeout = config.linkcheck_rate_limit_timeout
         self._allow_unauthorized = config.linkcheck_allow_unauthorized
@@ -722,6 +731,8 @@ def handle_starttag(self, tag: Any, attrs: Any) -> None:
 def _allowed_redirect(
     url: str, new_url: str, allowed_redirects: dict[re.Pattern[str], re.Pattern[str]]
 ) -> bool:
+    if allowed_redirects is _SENTINEL_LAR:
+        return False
     return any(
         from_url.match(url) and to_url.match(new_url)
         for from_url, to_url in allowed_redirects.items()
@@ -750,8 +761,7 @@ def rewrite_github_anchor(app: Sphinx, uri: str) -> str | None:
 
 def compile_linkcheck_allowed_redirects(app: Sphinx, config: Config) -> None:
     """Compile patterns to the regexp objects."""
-    if config.linkcheck_allowed_redirects is _sentinel_lar:
-        config.linkcheck_allowed_redirects = None
+    if config.linkcheck_allowed_redirects is _SENTINEL_LAR:
         return
     if not isinstance(config.linkcheck_allowed_redirects, dict):
         msg = __(
@@ -772,9 +782,6 @@ def compile_linkcheck_allowed_redirects(app: Sphinx, config: Config) -> None:
     config.linkcheck_allowed_redirects = allowed_redirects
 
 
-_sentinel_lar = object()
-
-
 def setup(app: Sphinx) -> ExtensionMetadata:
     app.add_builder(CheckExternalLinksBuilder)
     app.add_post_transform(HyperlinkCollector)
@@ -784,7 +791,7 @@ def setup(app: Sphinx) -> ExtensionMetadata:
         'linkcheck_exclude_documents', [], '', types=frozenset({list, tuple})
     )
     app.add_config_value(
-        'linkcheck_allowed_redirects', _sentinel_lar, '', types=frozenset({dict})
+        'linkcheck_allowed_redirects', _SENTINEL_LAR, '', types=frozenset({dict})
     )
     app.add_config_value('linkcheck_auth', [], '', types=frozenset({list, tuple}))
     app.add_config_value('linkcheck_request_headers', {}, '', types=frozenset({dict}))

tests/test_builders/test_build_linkcheck.py

@@ -680,7 +680,7 @@ def check_headers(self):
     assert content['status'] == 'working'
 
 
-def make_redirect_handler(*, support_head: bool) -> type[BaseHTTPRequestHandler]:
+def make_redirect_handler(*, support_head: bool = True) -> type[BaseHTTPRequestHandler]:
     class RedirectOnceHandler(BaseHTTPRequestHandler):
         protocol_version = 'HTTP/1.1'
 
@@ -715,6 +715,7 @@ def log_date_time_string(self):
 )
 def test_follows_redirects_on_HEAD(app, capsys):
     with serve_application(app, make_redirect_handler(support_head=True)) as address:
+        compile_linkcheck_allowed_redirects(app, app.config)
         app.build()
     _stdout, stderr = capsys.readouterr()
     content = (app.outdir / 'output.txt').read_text(encoding='utf8')
@@ -728,6 +729,9 @@ def test_follows_redirects_on_HEAD(app, capsys):
         127.0.0.1 - - [] "HEAD /?redirected=1 HTTP/1.1" 204 -
         """,
     )
+    assert (
+        f'redirect  http://{address}/ - with Found to http://{address}/?redirected=1\n'
+    ) in strip_escape_sequences(app.status.getvalue())
     assert app.warning.getvalue() == ''
 
 
@@ -738,6 +742,7 @@ def test_follows_redirects_on_HEAD(app, capsys):
 )
 def test_follows_redirects_on_GET(app, capsys):
     with serve_application(app, make_redirect_handler(support_head=False)) as address:
+        compile_linkcheck_allowed_redirects(app, app.config)
         app.build()
     _stdout, stderr = capsys.readouterr()
     content = (app.outdir / 'output.txt').read_text(encoding='utf8')
@@ -752,9 +757,37 @@ def test_follows_redirects_on_GET(app, capsys):
         127.0.0.1 - - [] "GET /?redirected=1 HTTP/1.1" 204 -
         """,
     )
+    assert (
+        f'redirect  http://{address}/ - with Found to http://{address}/?redirected=1\n'
+    ) in strip_escape_sequences(app.status.getvalue())
     assert app.warning.getvalue() == ''
 
 
+@pytest.mark.sphinx(
+    'linkcheck',
+    testroot='linkcheck-localserver',
+    freshenv=True,
+    confoverrides={'linkcheck_allowed_redirects': {}},  # warn about any redirects
+)
+def test_warns_disallowed_redirects(app, capsys):
+    with serve_application(app, make_redirect_handler()) as address:
+        compile_linkcheck_allowed_redirects(app, app.config)
+        app.build()
+    _stdout, stderr = capsys.readouterr()
+    content = (app.outdir / 'output.txt').read_text(encoding='utf8')
+    assert content == (
+        'index.rst:1: [redirected with Found] '
+        f'http://{address}/ to http://{address}/?redirected=1\n'
+    )
+    assert stderr == textwrap.dedent(
+        """\
+        127.0.0.1 - - [] "HEAD / HTTP/1.1" 302 -
+        127.0.0.1 - - [] "HEAD /?redirected=1 HTTP/1.1" 204 -
+        """,
+    )
+    assert len(app.warning.getvalue().splitlines()) == 1
+
+
 def test_linkcheck_allowed_redirects_config(
     make_app: Callable[..., SphinxTestApp], tmp_path: Path
 ) -> None: