From fff7d120f0e24869b7923b18c4e97b916215eed9 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 17 May 2023 15:07:02 +0100 Subject: [PATCH 1/6] tests: linkcheck: refactor to remove sharing of variables between the unit test and webserver handler threads --- tests/test_build_linkcheck.py | 114 ++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 32 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 260cf2c4214..428156bbb01 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -2,13 +2,13 @@ from __future__ import annotations -import base64 import http.server import json import re import textwrap import time import wsgiref.handlers +from base64 import b64encode from datetime import datetime from os import path from queue import Queue @@ -194,16 +194,42 @@ def do_GET(self): ) -def capture_headers_handler(records): - class HeadersDumperHandler(http.server.BaseHTTPRequestHandler): +def custom_handler(valid_credentials=None, success_criteria=lambda _: True): + """ + Returns an HTTP request handler that authenticates the client and then determines + an appropriate HTTP response code, based on caller-provided credentials and optional + success criteria, respectively. + """ + expected_token = None + if valid_credentials: + assert len(valid_credentials) == 2, "expected a pair of strings as credentials" + expected_token = b64encode(":".join(valid_credentials).encode()).decode("utf-8") + del valid_credentials + + class CustomHandler(http.server.BaseHTTPRequestHandler): + def authenticated(method): + def method_if_authenticated(self): + if expected_token is None: + return method(self) + elif self.headers["Authorization"] == f"Basic {expected_token}": + return method(self) + else: + self.send_response(403, "Forbidden") + self.end_headers() + + return method_if_authenticated + + @authenticated def do_HEAD(self): self.do_GET() + @authenticated def do_GET(self): - self.send_response(200, "OK") + response = (200, "OK") if success_criteria(self) else (400, "Bad Request") + self.send_response(*response) self.end_headers() - records.append(self.headers.as_string()) - return HeadersDumperHandler + + return CustomHandler @pytest.mark.sphinx( @@ -214,25 +240,26 @@ def do_GET(self): (r'.*local.*', ('user2', 'hunter2')), ]}) def test_auth_header_uses_first_match(app): - records = [] - with http_server(capture_headers_handler(records)): + with http_server(custom_handler(valid_credentials=("user1", "password"))): app.build() - stdout = "\n".join(records) - encoded_auth = base64.b64encode(b'user1:password').decode('ascii') - assert f"Authorization: Basic {encoded_auth}\n" in stdout + with open(app.outdir / "output.json", encoding="utf-8") as fp: + content = json.load(fp) + + assert content["status"] == "working" @pytest.mark.sphinx( 'linkcheck', testroot='linkcheck-localserver', freshenv=True, confoverrides={'linkcheck_auth': [(r'^$', ('user1', 'password'))]}) def test_auth_header_no_match(app): - records = [] - with http_server(capture_headers_handler(records)): + with http_server(custom_handler(valid_credentials=("user1", "password"))): app.build() - stdout = "\n".join(records) - assert "Authorization" not in stdout + with open(app.outdir / "output.json", encoding="utf-8") as fp: + content = json.load(fp) + + assert content["status"] == "broken" @pytest.mark.sphinx( @@ -246,14 +273,22 @@ def test_auth_header_no_match(app): }, }}) def test_linkcheck_request_headers(app): - records = [] - with http_server(capture_headers_handler(records)): + def check_headers(self): + if "X-Secret" in self.headers: + return False + if "sesami" in self.headers.as_string(): + return False + if self.headers["Accept"] != "text/html": + return False + return True + + with http_server(custom_handler(success_criteria=check_headers)): app.build() - stdout = "\n".join(records) - assert "Accept: text/html\n" in stdout - assert "X-Secret" not in stdout - assert "sesami" not in stdout + with open(app.outdir / "output.json", encoding="utf-8") as fp: + content = json.load(fp) + + assert content["status"] == "working" @pytest.mark.sphinx( @@ -263,14 +298,22 @@ def test_linkcheck_request_headers(app): "*": {"X-Secret": "open sesami"}, }}) def test_linkcheck_request_headers_no_slash(app): - records = [] - with http_server(capture_headers_handler(records)): + def check_headers(self): + if "X-Secret" in self.headers: + return False + if "sesami" in self.headers.as_string(): + return False + if self.headers["Accept"] != "application/json": + return False + return True + + with http_server(custom_handler(success_criteria=check_headers)): app.build() - stdout = "\n".join(records) - assert "Accept: application/json\n" in stdout - assert "X-Secret" not in stdout - assert "sesami" not in stdout + with open(app.outdir / "output.json", encoding="utf-8") as fp: + content = json.load(fp) + + assert content["status"] == "working" @pytest.mark.sphinx( @@ -280,13 +323,20 @@ def test_linkcheck_request_headers_no_slash(app): "*": {"X-Secret": "open sesami"}, }}) def test_linkcheck_request_headers_default(app): - records = [] - with http_server(capture_headers_handler(records)): + def check_headers(self): + if self.headers["X-Secret"] != "open sesami": + return False + if self.headers["Accept"] == "application/json": + return False + return True + + with http_server(custom_handler(success_criteria=check_headers)): app.build() - stdout = "\n".join(records) - assert "Accepts: application/json\n" not in stdout - assert "X-Secret: open sesami\n" in stdout + with open(app.outdir / "output.json", encoding="utf-8") as fp: + content = json.load(fp) + + assert content["status"] == "working" def make_redirect_handler(*, support_head): From 7099f3b1524352f9ef4a9d85fc5977fa3aa44e7b Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 17 May 2023 18:50:35 +0100 Subject: [PATCH 2/6] test_linkcheck_request_headers: cleanup: remove headers.as_string call --- tests/test_build_linkcheck.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 428156bbb01..77bb9fac1c6 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -276,8 +276,6 @@ def test_linkcheck_request_headers(app): def check_headers(self): if "X-Secret" in self.headers: return False - if "sesami" in self.headers.as_string(): - return False if self.headers["Accept"] != "text/html": return False return True From f1d09da4dd95c870df3a5431bd7e4aefb167e799 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 17 May 2023 19:17:06 +0100 Subject: [PATCH 3/6] test_linkcheck_request_headers: cleanup: remove (another) headers.as_string call --- tests/test_build_linkcheck.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 77bb9fac1c6..854f86f2633 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -299,8 +299,6 @@ def test_linkcheck_request_headers_no_slash(app): def check_headers(self): if "X-Secret" in self.headers: return False - if "sesami" in self.headers.as_string(): - return False if self.headers["Accept"] != "application/json": return False return True From 0c8bec25f64d64b51fe3f5b430070a69c656d1f2 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 22 May 2023 10:57:23 +0100 Subject: [PATCH 4/6] test_auth_header_no_match: add assertion to help confirm why the test link is considered broken --- tests/test_build_linkcheck.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 854f86f2633..1a667bc9130 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -259,6 +259,9 @@ def test_auth_header_no_match(app): with open(app.outdir / "output.json", encoding="utf-8") as fp: content = json.load(fp) + # TODO: should this test's webserver return HTTP 401 here? + # https://github.com/sphinx-doc/sphinx/issues/11433 + assert content["info"] == "403 Client Error: Forbidden for url: http://localhost:7777/" assert content["status"] == "broken" From 1ad8d9cc5ce2431e17daf934f5096881a497b822 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 6 Jun 2023 20:56:30 +0100 Subject: [PATCH 5/6] [no-op] Empty commit to re-run continuous integration From dadcac02a25420b6b38a87daddf1d79df57d091d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Thu, 20 Jul 2023 22:02:28 +0100 Subject: [PATCH 6/6] style --- tests/test_build_linkcheck.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index 1a667bc9130..41313aab4da 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -194,7 +194,7 @@ def do_GET(self): ) -def custom_handler(valid_credentials=None, success_criteria=lambda _: True): +def custom_handler(valid_credentials=(), success_criteria=lambda _: True): """ Returns an HTTP request handler that authenticates the client and then determines an appropriate HTTP response code, based on caller-provided credentials and optional @@ -225,8 +225,10 @@ def do_HEAD(self): @authenticated def do_GET(self): - response = (200, "OK") if success_criteria(self) else (400, "Bad Request") - self.send_response(*response) + if success_criteria(self): + self.send_response(200, "OK") + else: + self.send_response(400, "Bad Request") self.end_headers() return CustomHandler