Skip to content

linkcheck: disallow 'None' value for 'linkcheck_allowed_redirects' setting #13483

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------
Expand Down
7 changes: 4 additions & 3 deletions sphinx/builders/linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
39 changes: 30 additions & 9 deletions tests/test_builders/test_build_linkcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -715,13 +715,11 @@ 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')
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 -
Expand All @@ -738,13 +736,11 @@ 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')
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 -
Expand All @@ -755,6 +751,31 @@ 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_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:
Expand Down
Loading