From 87f28dc927e328d69b6ba4f36f703feeaadd2764 Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 4 Apr 2025 18:35:42 +0100 Subject: [PATCH 1/4] Tests: uncover a quirk in our ``linkcheck`` tests --- tests/test_builders/test_build_linkcheck.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index bdd8dea54c1..2677cb3d33c 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -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') @@ -738,6 +739,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') From 27f2de77e7d5be753a9fbda6e9f97e063e8dc7fc Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 4 Apr 2025 19:10:42 +0100 Subject: [PATCH 2/4] linkcheck: retain default do-not-follow for redirects --- CHANGES.rst | 4 +- doc/usage/configuration.rst | 5 ++ sphinx/builders/linkcheck.py | 51 ++++++++----------- tests/test_builders/test_build_linkcheck.py | 54 ++++++++++++--------- 4 files changed, 58 insertions(+), 56 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index fede8b5177b..a83ad594d0d 100644 --- a/CHANGES.rst +++ b/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 ---------- diff --git a/doc/usage/configuration.rst b/doc/usage/configuration.rst index d14b5d4ec6b..b2f27984518 100644 --- a/doc/usage/configuration.rst +++ b/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 `. + 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 diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index ff6878f2acb..c66d08e8f47 100644 --- a/sphinx/builders/linkcheck.py +++ b/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})) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index 2677cb3d33c..de789077e63 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/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,6 +710,7 @@ 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: @@ -719,10 +718,7 @@ def test_follows_redirects_on_HEAD(app, capsys): 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,6 +732,7 @@ 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: @@ -743,10 +740,7 @@ def test_follows_redirects_on_GET(app, capsys): 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', From d16e135bddb680ab7080cb9ab8064f3f0c7f58ea Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 4 Apr 2025 19:16:08 +0100 Subject: [PATCH 3/4] Revert "Tests: uncover a quirk in our ``linkcheck`` tests" This reverts commit c9d6d6f40164f140d9ff6570b862921aee9ea302. Conflicts: tests/test_builders/test_build_linkcheck.py --- tests/test_builders/test_build_linkcheck.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index de789077e63..180fa8cebf0 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -714,7 +714,6 @@ 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') @@ -736,7 +735,6 @@ 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') From c5dea9406f3e3e2594179f9818bec1cc2bf7eede Mon Sep 17 00:00:00 2001 From: James Addison Date: Fri, 4 Apr 2025 19:43:53 +0100 Subject: [PATCH 4/4] Reapply "Tests: uncover a quirk in our ``linkcheck`` tests" This reverts commit d16e135bddb680ab7080cb9ab8064f3f0c7f58ea. --- tests/test_builders/test_build_linkcheck.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index 180fa8cebf0..de789077e63 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -714,6 +714,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') @@ -735,6 +736,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')