From c9d6d6f40164f140d9ff6570b862921aee9ea302 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 5afe46d31ac1adf994f11ff45aa95f39392c84b4 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 14 Apr 2025 10:02:49 +0100 Subject: [PATCH 2/4] linkcheck: prevent `linkcheck_allowed_redirects` value of `None` --- sphinx/builders/linkcheck.py | 7 ++++--- tests/test_builders/test_build_linkcheck.py | 12 ++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index e1a80a47c0f..c15e9fa7aeb 100644 --- a/sphinx/builders/linkcheck.py +++ b/sphinx/builders/linkcheck.py @@ -179,7 +179,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 +387,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 +722,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 True return any( from_url.match(url) and to_url.match(new_url) for from_url, to_url in allowed_redirects.items() @@ -751,7 +753,6 @@ 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): raise ConfigError diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index 2677cb3d33c..1f636b29ffc 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -712,6 +712,7 @@ def log_date_time_string(self): 'linkcheck', testroot='linkcheck-localserver', freshenv=True, + confoverrides={'linkcheck_allowed_redirects': {}}, # do not follow any redirects ) def test_follows_redirects_on_HEAD(app, capsys): with serve_application(app, make_redirect_handler(support_head=True)) as address: @@ -729,13 +730,17 @@ def test_follows_redirects_on_HEAD(app, capsys): 127.0.0.1 - - [] "HEAD /?redirected=1 HTTP/1.1" 204 - """, ) - assert app.warning.getvalue() == '' + assert ( + f'WARNING: redirect http://{address}/' + f' - with Found to http://{address}/?redirected=1' + ) in strip_escape_sequences(app.warning.getvalue()) @pytest.mark.sphinx( 'linkcheck', testroot='linkcheck-localserver', freshenv=True, + confoverrides={'linkcheck_allowed_redirects': {}}, # do not follow any redirects ) def test_follows_redirects_on_GET(app, capsys): with serve_application(app, make_redirect_handler(support_head=False)) as address: @@ -754,7 +759,10 @@ def test_follows_redirects_on_GET(app, capsys): 127.0.0.1 - - [] "GET /?redirected=1 HTTP/1.1" 204 - """, ) - assert app.warning.getvalue() == '' + assert ( + f'WARNING: redirect http://{address}/' + f' - with Found to http://{address}/?redirected=1' + ) in strip_escape_sequences(app.warning.getvalue()) def test_linkcheck_allowed_redirects_config( From a8df874236cb59e6342df4666cdfc63645d149f1 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 14 Apr 2025 10:20:25 +0100 Subject: [PATCH 3/4] linkcheck: adjust test coverage --- tests/test_builders/test_build_linkcheck.py | 47 +++++++++++++-------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index 1f636b29ffc..5feb105d7d1 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/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' @@ -712,7 +712,6 @@ def log_date_time_string(self): 'linkcheck', testroot='linkcheck-localserver', freshenv=True, - confoverrides={'linkcheck_allowed_redirects': {}}, # do not follow any redirects ) def test_follows_redirects_on_HEAD(app, capsys): with serve_application(app, make_redirect_handler(support_head=True)) as address: @@ -720,27 +719,20 @@ 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 - 127.0.0.1 - - [] "HEAD /?redirected=1 HTTP/1.1" 204 - """, ) - assert ( - f'WARNING: redirect http://{address}/' - f' - with Found to http://{address}/?redirected=1' - ) in strip_escape_sequences(app.warning.getvalue()) + assert app.warning.getvalue() == '' @pytest.mark.sphinx( 'linkcheck', testroot='linkcheck-localserver', freshenv=True, - confoverrides={'linkcheck_allowed_redirects': {}}, # do not follow any redirects ) def test_follows_redirects_on_GET(app, capsys): with serve_application(app, make_redirect_handler(support_head=False)) as address: @@ -748,21 +740,40 @@ 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 == '' + assert stderr == textwrap.dedent( + """\ + 127.0.0.1 - - [] "HEAD / HTTP/1.1" 405 - + 127.0.0.1 - - [] "GET / HTTP/1.1" 302 - + 127.0.0.1 - - [] "GET /?redirected=1 HTTP/1.1" 204 - + """, + ) + 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_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" 405 - - 127.0.0.1 - - [] "GET / HTTP/1.1" 302 - - 127.0.0.1 - - [] "GET /?redirected=1 HTTP/1.1" 204 - + 127.0.0.1 - - [] "HEAD / HTTP/1.1" 302 - + 127.0.0.1 - - [] "HEAD /?redirected=1 HTTP/1.1" 204 - """, ) - assert ( - f'WARNING: redirect http://{address}/' - f' - with Found to http://{address}/?redirected=1' - ) in strip_escape_sequences(app.warning.getvalue()) + assert len(app.warning.getvalue().splitlines()) == 1 def test_linkcheck_allowed_redirects_config( From 3fa174b0865db3747aca59b1e8f243ec109c823c Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 14 Apr 2025 10:20:51 +0100 Subject: [PATCH 4/4] Update CHANGES.rst --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index fede8b5177b..25ca682aff0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -18,7 +18,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. Bugs fixed ----------