Skip to content

Warn on all redirects if linkcheck_allowed_redirects is an empty dictionary #13452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +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
``linkcheck_allowed_redirects = {}``.
Patch by Adam Turner.

Bugs fixed
----------
Expand Down
5 changes: 5 additions & 0 deletions doc/usage/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3668,6 +3668,11 @@ and which failures and redirects it ignores.

.. versionadded:: 4.1

.. versionchanged:: 8.3
Setting :confval:`!linkcheck_allowed_redirects` to the empty directory
may now be used to warn on all redirects encountered
by the *linkcheck* builder.

.. confval:: linkcheck_anchors
:type: :code-py:`bool`
:default: :code-py:`True`
Expand Down
32 changes: 21 additions & 11 deletions sphinx/builders/linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

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
Expand Down Expand Up @@ -178,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:
if self.config.linkcheck_allowed_redirects is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line/diff is confusing without context -- but the way I understand this: this code is within the _Status.REDIRECTED case -- and we can only reach this part of the code if a disallowed redirect occurred, or if no filtering of allowed redirects is configured. So the conditional check here is a shortcut, simply confirming whether allowed redirects are configured or not; if it is configured, we should emit a warning.

msg = f'redirect {res_uri} - {redirection}'
logger.warning(msg, location=(result.docname, result.lineno))
else:
Expand Down Expand Up @@ -386,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
self.allowed_redirects = config.linkcheck_allowed_redirects or {}
self.retries: int = config.linkcheck_retries
self.rate_limit_timeout = config.linkcheck_rate_limit_timeout
self._allow_unauthorized = config.linkcheck_allow_unauthorized
Expand Down Expand Up @@ -748,20 +749,26 @@ def rewrite_github_anchor(app: Sphinx, uri: str) -> str | None:


def compile_linkcheck_allowed_redirects(app: Sphinx, config: Config) -> None:
"""Compile patterns in linkcheck_allowed_redirects to the regexp objects."""
linkcheck_allowed_redirects = app.config.linkcheck_allowed_redirects
for url, pattern in list(linkcheck_allowed_redirects.items()):
"""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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing an explanatory message here?

allowed_redirects = {}
for url, pattern in config.linkcheck_allowed_redirects.items():
try:
linkcheck_allowed_redirects[re.compile(url)] = re.compile(pattern)
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,
)
finally:
# Remove the original regexp-string
linkcheck_allowed_redirects.pop(url)
config.linkcheck_allowed_redirects = allowed_redirects


_sentinel_lar = object()


def setup(app: Sphinx) -> ExtensionMetadata:
Expand All @@ -772,7 +779,9 @@ def setup(app: Sphinx) -> ExtensionMetadata:
app.add_config_value(
'linkcheck_exclude_documents', [], '', types=frozenset({list, tuple})
)
app.add_config_value('linkcheck_allowed_redirects', {}, '', types=frozenset({dict}))
app.add_config_value(
'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}))
app.add_config_value('linkcheck_retries', 1, '', types=frozenset({int}))
Expand All @@ -799,7 +808,8 @@ def setup(app: Sphinx) -> ExtensionMetadata:

app.add_event('linkcheck-process-uri')

app.connect('config-inited', compile_linkcheck_allowed_redirects, priority=800)
# priority 900 to happen after ``check_confval_types()``
app.connect('config-inited', compile_linkcheck_allowed_redirects, priority=900)

# FIXME: Disable URL rewrite handler for github.com. temporarily.
# See: https://github.com./sphinx-doc/sphinx/issues/9435
Expand Down
31 changes: 31 additions & 0 deletions tests/test_builders/test_build_linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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
Expand All @@ -27,6 +28,7 @@
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
Expand All @@ -37,6 +39,7 @@

if TYPE_CHECKING:
from collections.abc import Callable, Iterable
from pathlib import Path
from typing import Any

from urllib3 import HTTPConnectionPool
Expand Down Expand Up @@ -752,6 +755,34 @@ def test_follows_redirects_on_GET(app, capsys):
assert app.warning.getvalue() == ''


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',
srcdir=tmp_path,
confoverrides={'linkcheck_allowed_redirects': {}},
)
assert strip_escape_sequences(app.warning.getvalue()) == ''


@pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-warn-redirects')
def test_linkcheck_allowed_redirects(app: SphinxTestApp) -> None:
with serve_application(app, make_redirect_handler(support_head=False)) as address:
Expand Down