linkcheck: retain default do-not-follow for redirects

Author: jayaddison

CHANGES.rst

@@ -16,9 +16,9 @@ Features added
 * #13332: Add :confval:`doctest_fail_fast` option to exit after the first failed
   test.
   Patch by Till Hoffmann.
-* #13439: linkcheck: Permit warning on every redirect with
+* #13439, #13462: linkcheck: Permit warning on every redirect with
   ``linkcheck_allowed_redirects = {}``.
-  Patch by Adam Turner.
+  Patch by Adam Turner and James Addison.
 
 Bugs fixed
 ----------

doc/usage/configuration.rst

@@ -3642,6 +3642,7 @@ and which failures and redirects it ignores.
 
 .. confval:: linkcheck_allowed_redirects
    :type: :code-py:`dict[str, str]`
+   :default: :code-py:`{}` (do not follow)
 
    A dictionary that maps a pattern of the source URI
    to a pattern of the canonical URI.
@@ -3655,6 +3656,10 @@ and which failures and redirects it ignores.
    It can be useful to detect unexpected redirects when using
    :option:`the fail-on-warnings mode <sphinx-build --fail-on-warning>`.
 
+   To deny all redirects, configure an empty dictionary (the default).
+
+   To follow all redirections, configure a value of :code-py:`None`.
+
    Example:
 
    .. code-block:: python

sphinx/builders/linkcheck.py

@@ -25,7 +25,6 @@
 
 from sphinx._cli.util.colour import darkgray, darkgreen, purple, red, turquoise
 from sphinx.builders.dummy import DummyBuilder
-from sphinx.errors import ConfigError
 from sphinx.locale import __
 from sphinx.transforms.post_transforms import SphinxPostTransform
 from sphinx.util import logging, requests
@@ -387,7 +386,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,10 +721,13 @@ 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:
-    return any(
-        from_url.match(url) and to_url.match(new_url)
-        for from_url, to_url in allowed_redirects.items()
-    )
+    if allowed_redirects is None:  # no restrictions configured
+        return True
+    else:
+        return any(
+            from_url.match(url) and to_url.match(new_url)
+            for from_url, to_url in allowed_redirects.items()
+        )
 
 
 class RateLimit(NamedTuple):
@@ -750,29 +752,18 @@ 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
-        return
-    if not isinstance(config.linkcheck_allowed_redirects, dict):
-        msg = __(
-            f'Invalid value `{config.linkcheck_allowed_redirects!r}` in '
-            'linkcheck_allowed_redirects. Expected a dictionary.'
-        )
-        raise ConfigError(msg)
-    allowed_redirects = {}
-    for url, pattern in config.linkcheck_allowed_redirects.items():
-        try:
-            allowed_redirects[re.compile(url)] = re.compile(pattern)
-        except re.error as exc:
-            logger.warning(
-                __('Failed to compile regex in linkcheck_allowed_redirects: %r %s'),
-                exc.pattern,
-                exc.msg,
-            )
-    config.linkcheck_allowed_redirects = allowed_redirects
-
-
-_sentinel_lar = object()
+    if config.linkcheck_allowed_redirects is not None:
+        allowed_redirects = {}
+        for url, pattern in config.linkcheck_allowed_redirects.items():
+            try:
+                allowed_redirects[re.compile(url)] = re.compile(pattern)
+            except re.error as exc:
+                logger.warning(
+                    __('Failed to compile regex in linkcheck_allowed_redirects: %r %s'),
+                    exc.pattern,
+                    exc.msg,
+                )
+        config.linkcheck_allowed_redirects = allowed_redirects
 
 
 def setup(app: Sphinx) -> ExtensionMetadata:
@@ -784,7 +775,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', None, '', types=frozenset({dict, type(None)})
     )
     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

@@ -10,7 +10,6 @@
 import wsgiref.handlers
 from base64 import b64encode
 from http.server import BaseHTTPRequestHandler
-from io import StringIO
 from queue import Queue
 from typing import TYPE_CHECKING
 from unittest import mock
@@ -28,7 +27,6 @@
     RateLimit,
     compile_linkcheck_allowed_redirects,
 )
-from sphinx.errors import ConfigError
 from sphinx.testing.util import SphinxTestApp
 from sphinx.util import requests
 from sphinx.util._pathlib import _StrPath
@@ -680,7 +678,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'
 
@@ -712,17 +710,15 @@ def log_date_time_string(self):
     'linkcheck',
     testroot='linkcheck-localserver',
     freshenv=True,
+    confoverrides={'linkcheck_allowed_redirects': None},
 )
 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')
-    assert content == (
-        'index.rst:1: [redirected with Found] '
-        f'http://{address}/ to http://{address}/?redirected=1\n'
-    )
+    assert content == ''
     assert stderr == textwrap.dedent(
         """\
         127.0.0.1 - - [] "HEAD / HTTP/1.1" 302 -
@@ -736,17 +732,15 @@ def test_follows_redirects_on_HEAD(app, capsys):
     'linkcheck',
     testroot='linkcheck-localserver',
     freshenv=True,
+    confoverrides={'linkcheck_allowed_redirects': None},
 )
 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')
-    assert content == (
-        'index.rst:1: [redirected with Found] '
-        f'http://{address}/ to http://{address}/?redirected=1\n'
-    )
+    assert content == ''
     assert stderr == textwrap.dedent(
         """\
         127.0.0.1 - - [] "HEAD / HTTP/1.1" 405 -
@@ -757,25 +751,37 @@ def test_follows_redirects_on_GET(app, capsys):
     assert app.warning.getvalue() == ''
 
 
+@pytest.mark.sphinx(
+    'linkcheck',
+    testroot='linkcheck-localserver',
+    freshenv=True,
+    confoverrides={'linkcheck_allowed_redirects': {}},  # do not follow any redirects
+)
+def test_warns_redirects_on_GET(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:
     tmp_path.joinpath('conf.py').touch()
     tmp_path.joinpath('index.rst').touch()
 
-    # ``linkcheck_allowed_redirects = None`` is rejected
-    warning_stream = StringIO()
-    with pytest.raises(ConfigError):
-        make_app(
-            'linkcheck',
-            srcdir=tmp_path,
-            confoverrides={'linkcheck_allowed_redirects': None},
-            warning=warning_stream,
-        )
-    assert strip_escape_sequences(warning_stream.getvalue()).splitlines() == [
-        "WARNING: The config value `linkcheck_allowed_redirects' has type `NoneType'; expected `dict'."
-    ]
-
     # ``linkcheck_allowed_redirects = {}`` is permitted
     app = make_app(
         'linkcheck',