From cbd39004b50791b44edcecaa977aef55a5a80617 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 5 May 2026 23:48:01 +0500 Subject: [PATCH] Run mitmproxy-based tests for every handler and improve them. --- conftest.py | 27 ++++ docs/topics/download-handlers.rst | 2 + tests/mockserver/mitm_proxy.py | 64 +++++++++ tests/mockserver/mitm_proxy_addon.py | 5 + tests/test_downloader_handler_httpx.py | 27 ++-- .../test_downloader_handler_twisted_http11.py | 27 ++-- .../test_downloader_handler_twisted_http2.py | 51 +++---- tests/test_downloader_handlers_http_base.py | 119 +++++++++++++++++ tests/test_proxy_connect.py | 125 ------------------ 9 files changed, 270 insertions(+), 177 deletions(-) create mode 100644 tests/mockserver/mitm_proxy.py create mode 100644 tests/mockserver/mitm_proxy_addon.py delete mode 100644 tests/test_proxy_connect.py diff --git a/conftest.py b/conftest.py index 4f8d32e1b..fcbf59426 100644 --- a/conftest.py +++ b/conftest.py @@ -11,6 +11,7 @@ from scrapy.utils.reactor import set_asyncio_event_loop_policy from scrapy.utils.reactorless import install_reactor_import_hook from tests.keys import generate_keys from tests.mockserver.http import MockServer +from tests.mockserver.mitm_proxy import MitmProxy if TYPE_CHECKING: from collections.abc import Generator @@ -72,6 +73,32 @@ def mockserver() -> Generator[MockServer]: yield mockserver +@pytest.fixture # function scope because it modifies os.environ +def mitm_proxy_server(monkeypatch: pytest.MonkeyPatch) -> Generator[MitmProxy]: + proxy = MitmProxy() + url = proxy.start() + monkeypatch.setenv("http_proxy", url) + monkeypatch.setenv("https_proxy", url) + + try: + yield proxy + finally: + proxy.stop() + + +@pytest.fixture # function scope because it modifies os.environ +def mitm_proxy_server_https(monkeypatch: pytest.MonkeyPatch) -> Generator[MitmProxy]: + proxy = MitmProxy() + url = proxy.start().replace("http://", "https://") + monkeypatch.setenv("http_proxy", url) + monkeypatch.setenv("https_proxy", url) + + try: + yield proxy + finally: + proxy.stop() + + @pytest.fixture(scope="session") def reactor_pytest(request) -> str: return request.config.getoption("--reactor") diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 96cf46c35..1e95864c6 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -206,6 +206,8 @@ If you want to use this handler you need to replace the default one for the Known limitations of the HTTP/2 implementation in this handler include: + - No support for proxies. + - No support for HTTP/2 Cleartext (h2c), since no major browser supports HTTP/2 unencrypted (refer `http2 faq`_). diff --git a/tests/mockserver/mitm_proxy.py b/tests/mockserver/mitm_proxy.py new file mode 100644 index 000000000..1ec5a79b5 --- /dev/null +++ b/tests/mockserver/mitm_proxy.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +import re +import sys +from pathlib import Path +from subprocess import PIPE, Popen +from urllib.parse import urlsplit, urlunsplit + + +class MitmProxy: + auth_user = "scrapy" + auth_pass = "scrapy" + + def start(self) -> str: + script = """ +import sys +from mitmproxy.tools.main import mitmdump +sys.argv[0] = "mitmdump" +sys.exit(mitmdump()) + """ + cert_path = Path(__file__).parent.parent.resolve() / "keys" + args = [ + "--listen-host", + "127.0.0.1", + "--listen-port", + "0", + "--proxyauth", + f"{self.auth_user}:{self.auth_pass}", + "--set", + f"confdir={cert_path}", + "--ssl-insecure", + "-s", + str(Path(__file__).with_name("mitm_proxy_addon.py")), + ] + self.proc: Popen[str] = Popen( + [ + sys.executable, + "-u", + "-c", + script, + *args, + ], + stdout=PIPE, + text=True, + ) + assert self.proc.stdout is not None + line = "" + for line in self.proc.stdout: + m = re.search(r"listening at (?:http://)?([^:]+:\d+)", line) + if m: + host_port = m.group(1) + return f"http://{self.auth_user}:{self.auth_pass}@{host_port}" + self.stop() + raise RuntimeError(f"Failed to parse mitmdump output: {line}") + + def stop(self) -> None: + self.proc.kill() + self.proc.communicate() + + +def wrong_credentials(proxy_url: str) -> str: + bad_auth_proxy = list(urlsplit(proxy_url)) + bad_auth_proxy[1] = bad_auth_proxy[1].replace("scrapy:scrapy@", "wrong:wronger@") + return urlunsplit(bad_auth_proxy) diff --git a/tests/mockserver/mitm_proxy_addon.py b/tests/mockserver/mitm_proxy_addon.py new file mode 100644 index 000000000..04ba2e4a1 --- /dev/null +++ b/tests/mockserver/mitm_proxy_addon.py @@ -0,0 +1,5 @@ +def response(flow) -> None: + # add custom headers to be able to check that the request went through the proxy + flow.response.headers["X-Via-Mitmproxy"] = "1" + if flow.client_conn.tls_established: + flow.response.headers["X-Via-Mitmproxy-TLS"] = "1" diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index a0e3fccb3..d4dfe8396 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -17,6 +17,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsInvalidDNSPatternBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, + TestMitmProxyBase, TestSimpleHttpsBase, ) from tests.utils.decorators import coroutine_test @@ -41,6 +42,15 @@ class HttpxDownloadHandlerMixin: return HttpxDownloadHandler + @property + def settings_dict(self) -> dict[str, Any] | None: + return { + "DOWNLOAD_HANDLERS": { + "http": "scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler", + "https": "scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler", + } + } + class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): handler_supports_bindaddress_meta = False @@ -108,15 +118,8 @@ class TestHttpsCustomCiphers(HttpxDownloadHandlerMixin, TestHttpsCustomCiphersBa pass -class TestHttpWithCrawler(TestHttpWithCrawlerBase): - @property - def settings_dict(self) -> dict[str, Any] | None: - return { - "DOWNLOAD_HANDLERS": { - "http": "scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler", - "https": "scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler", - } - } +class TestHttpWithCrawler(HttpxDownloadHandlerMixin, TestHttpWithCrawlerBase): + pass class TestHttpsWithCrawler(TestHttpWithCrawler): @@ -136,3 +139,9 @@ class TestHttpProxy(HttpxDownloadHandlerMixin, TestHttpProxyBase): @pytest.mark.skip(reason="Proxy support is not implemented yet") class TestHttpsProxy(HttpxDownloadHandlerMixin, TestHttpProxyBase): is_secure = True + + +@pytest.mark.skip(reason="Proxy support is not implemented yet") +@pytest.mark.requires_mitmproxy +class TestMitmProxy(HttpxDownloadHandlerMixin, TestMitmProxyBase): + pass diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 01fb26d85..ed742e55f 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -19,6 +19,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsInvalidDNSPatternBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, + TestMitmProxyBase, TestSimpleHttpsBase, ) @@ -34,6 +35,15 @@ class HTTP11DownloadHandlerMixin: def download_handler_cls(self) -> type[DownloadHandlerProtocol]: return HTTP11DownloadHandler + @property + def settings_dict(self) -> dict[str, Any] | None: + return { + "DOWNLOAD_HANDLERS": { + "http": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", + "https": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", + } + } + def test_not_configured_without_reactor() -> None: crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False}) @@ -71,15 +81,8 @@ class TestHttpsCustomCiphers(HTTP11DownloadHandlerMixin, TestHttpsCustomCiphersB pass -class TestHttpWithCrawler(TestHttpWithCrawlerBase): - @property - def settings_dict(self) -> dict[str, Any] | None: - return { - "DOWNLOAD_HANDLERS": { - "http": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", - "https": "scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler", - } - } +class TestHttpWithCrawler(HTTP11DownloadHandlerMixin, TestHttpWithCrawlerBase): + pass class TestHttpsWithCrawler(TestHttpWithCrawler): @@ -88,3 +91,9 @@ class TestHttpsWithCrawler(TestHttpWithCrawler): class TestHttpProxy(HTTP11DownloadHandlerMixin, TestHttpProxyBase): pass + + +@pytest.mark.requires_mitmproxy +class TestMitmProxy(HTTP11DownloadHandlerMixin, TestMitmProxyBase): + # not implemented + handler_supports_tls_in_tls = False diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 3273a264e..60b276166 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -10,13 +10,8 @@ from twisted.web.http import H2_ENABLED from scrapy import Spider from scrapy.crawler import Crawler -from scrapy.exceptions import ( - DownloadFailedError, - NotConfigured, - UnsupportedURLSchemeError, -) +from scrapy.exceptions import DownloadFailedError, NotConfigured from scrapy.http import Request -from scrapy.utils.defer import maybe_deferred_to_future from tests.test_downloader_handlers_http_base import ( TestHttpProxyBase, TestHttpsBase, @@ -25,13 +20,13 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsInvalidDNSPatternBase, TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, + TestMitmProxyBase, ) from tests.utils.decorators import coroutine_test if TYPE_CHECKING: from scrapy.core.downloader.handlers import DownloadHandlerProtocol from tests.mockserver.http import MockServer - from tests.mockserver.proxy_echo import ProxyEchoMockServer pytestmark = [ @@ -52,6 +47,15 @@ class H2DownloadHandlerMixin: return H2DownloadHandler + @property + def settings_dict(self) -> dict[str, Any] | None: + return { + "DOWNLOAD_HANDLERS": { + "http": None, + "https": "scrapy.core.downloader.handlers.http2.H2DownloadHandler", + } + } + def test_not_configured_without_reactor() -> None: from scrapy.core.downloader.handlers.http2 import H2DownloadHandler # noqa: PLC0415 @@ -167,16 +171,7 @@ class TestHttp2CustomCiphers(H2DownloadHandlerMixin, TestHttpsCustomCiphersBase) pass -class TestHttp2WithCrawler(TestHttpWithCrawlerBase): - @property - def settings_dict(self) -> dict[str, Any] | None: - return { - "DOWNLOAD_HANDLERS": { - "http": None, - "https": "scrapy.core.downloader.handlers.http2.H2DownloadHandler", - } - } - +class TestHttp2WithCrawler(H2DownloadHandlerMixin, TestHttpWithCrawlerBase): is_secure = True def test_bytes_received_stop_download_callback(self) -> None: # type: ignore[override] @@ -192,24 +187,12 @@ class TestHttp2WithCrawler(TestHttpWithCrawlerBase): pytest.skip("headers_received support is not implemented") +@pytest.mark.skip(reason="Proxy support is not implemented yet") class TestHttp2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): is_secure = True - expected_http_proxy_request_body = b"/" - @coroutine_test - async def test_download_with_proxy_https_timeout( - self, proxy_mockserver: ProxyEchoMockServer - ) -> None: - with pytest.raises(NotImplementedError): - await maybe_deferred_to_future( - super().test_download_with_proxy_https_timeout(proxy_mockserver) # type: ignore[arg-type] - ) - @coroutine_test - async def test_download_with_proxy_without_http_scheme( - self, proxy_mockserver: ProxyEchoMockServer - ) -> None: - with pytest.raises(UnsupportedURLSchemeError): - await maybe_deferred_to_future( - super().test_download_with_proxy_without_http_scheme(proxy_mockserver) # type: ignore[arg-type] - ) +@pytest.mark.skip(reason="Proxy support is not implemented yet") +@pytest.mark.requires_mitmproxy +class TestMitmProxy(H2DownloadHandlerMixin, TestMitmProxyBase): + pass diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 2a781dace..31239e1d8 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -4,6 +4,8 @@ from __future__ import annotations import gzip import json +import logging +import os import platform import re import sys @@ -36,6 +38,7 @@ from scrapy.utils.misc import build_from_crawler from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler from tests import NON_EXISTING_RESOLVABLE +from tests.mockserver.mitm_proxy import MitmProxy, wrong_credentials from tests.mockserver.proxy_echo import ProxyEchoMockServer from tests.mockserver.simple_https import SimpleMockServer from tests.spiders import ( @@ -43,6 +46,7 @@ from tests.spiders import ( BytesReceivedErrbackSpider, HeadersReceivedCallbackSpider, HeadersReceivedErrbackSpider, + SimpleSpider, SingleRequestSpider, ) from tests.utils.decorators import coroutine_test @@ -1143,3 +1147,118 @@ class TestHttpProxyBase(ABC): assert response.status == 200 assert response.url == request.url assert response.body == self.expected_http_proxy_request_body + + +class TestMitmProxyBase(ABC): + # whether the handler supports HTTPS proxies with HTTPS destinations + handler_supports_tls_in_tls: bool = True + + @property + @abstractmethod + def settings_dict(self) -> dict[str, Any] | None: + raise NotImplementedError + + @pytest.mark.parametrize( + "https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"] + ) + @coroutine_test + async def test_http_proxy( + self, + caplog: pytest.LogCaptureFixture, + mockserver: MockServer, + mitm_proxy_server: MitmProxy, + https_dest: bool, + ) -> None: + """HTTP proxy, HTTP or HTTPS destination.""" + crawler = get_crawler(SingleRequestSpider, self.settings_dict) + with caplog.at_level(logging.DEBUG): + await crawler.crawl_async( + seed=mockserver.url("/status?n=200", is_secure=https_dest) + ) + assert isinstance(crawler.spider, SingleRequestSpider) + self._assert_got_response_code(200, caplog.text) + self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest) + + @pytest.mark.parametrize( + "https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"] + ) + @coroutine_test + async def test_https_proxy( + self, + caplog: pytest.LogCaptureFixture, + mockserver: MockServer, + mitm_proxy_server_https: MitmProxy, + https_dest: bool, + ) -> None: + """HTTPS proxy, HTTP or HTTPS destination.""" + if https_dest and not self.handler_supports_tls_in_tls: + pytest.skip("HTTPS proxy to HTTPS destination is not supported") + crawler = get_crawler(SingleRequestSpider, self.settings_dict) + with caplog.at_level(logging.DEBUG): + await crawler.crawl_async( + seed=mockserver.url("/status?n=200", is_secure=https_dest) + ) + assert isinstance(crawler.spider, SingleRequestSpider) + self._assert_got_response_code(200, caplog.text) + self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest) + + @pytest.mark.parametrize( + "https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"] + ) + @coroutine_test + async def test_http_proxy_auth_error( + self, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + mockserver: MockServer, + mitm_proxy_server: MitmProxy, + https_dest: bool, + ) -> None: + """HTTP proxy, HTTP or HTTPS destination, wrong proxy creds.""" + envvar = "https_proxy" if https_dest else "http_proxy" + monkeypatch.setenv(envvar, wrong_credentials(os.environ[envvar])) + crawler = get_crawler(SimpleSpider, self.settings_dict) + with caplog.at_level(logging.DEBUG): + await crawler.crawl_async( + mockserver.url("/status?n=200", is_secure=https_dest) + ) + # The proxy returns a 407 error code but it does not reach the client; + # it just sees an exception. + self._assert_got_auth_exception(caplog.text) + + @pytest.mark.parametrize( + "https_dest", [False, True], ids=["HTTP dest", "HTTPS dest"] + ) + @coroutine_test + async def test_dont_leak_proxy_authorization_header( + self, + caplog: pytest.LogCaptureFixture, + mockserver: MockServer, + mitm_proxy_server: MitmProxy, + https_dest: bool, + ) -> None: + """HTTP proxy, HTTP or HTTPS destination. Check that the auth header + is not sent to the destination.""" + request = Request(mockserver.url("/echo", is_secure=https_dest)) + crawler = get_crawler(SingleRequestSpider, self.settings_dict) + with caplog.at_level(logging.DEBUG): + await crawler.crawl_async(seed=request) + assert isinstance(crawler.spider, SingleRequestSpider) + self._assert_got_response_code(200, caplog.text) + self._assert_headers(crawler.spider.meta["responses"][0].headers, https_dest) + echo = json.loads(crawler.spider.meta["responses"][0].text) + assert "Proxy-Authorization" not in echo["headers"] + + @staticmethod + def _assert_headers(headers: Headers, https_dest: bool) -> None: + assert b"X-Via-Mitmproxy" in headers + if https_dest: + assert b"X-Via-Mitmproxy-TLS" in headers + + @staticmethod + def _assert_got_response_code(code: int, log: str) -> None: + assert str(log).count(f"Crawled ({code})") == 1 + + @staticmethod + def _assert_got_auth_exception(log: str) -> None: + assert "Proxy Authentication Required" in log or "407" in log diff --git a/tests/test_proxy_connect.py b/tests/test_proxy_connect.py deleted file mode 100644 index c89b51ad3..000000000 --- a/tests/test_proxy_connect.py +++ /dev/null @@ -1,125 +0,0 @@ -import json -import os -import re -import sys -from pathlib import Path -from subprocess import PIPE, Popen -from urllib.parse import urlsplit, urlunsplit - -import pytest -from testfixtures import LogCapture - -from scrapy.http import Request -from scrapy.utils.test import get_crawler -from tests.mockserver.http import MockServer -from tests.spiders import SimpleSpider, SingleRequestSpider -from tests.utils.decorators import inline_callbacks_test - - -class MitmProxy: - auth_user = "scrapy" - auth_pass = "scrapy" - - def start(self) -> str: - script = """ -import sys -from mitmproxy.tools.main import mitmdump -sys.argv[0] = "mitmdump" -sys.exit(mitmdump()) - """ - cert_path = Path(__file__).parent.resolve() / "keys" - args = [ - "--listen-host", - "127.0.0.1", - "--listen-port", - "0", - "--proxyauth", - f"{self.auth_user}:{self.auth_pass}", - "--set", - f"confdir={cert_path}", - "--ssl-insecure", - ] - self.proc = Popen( - [ - sys.executable, - "-u", - "-c", - script, - *args, - ], - stdout=PIPE, - text=True, - ) - assert self.proc.stdout is not None - line = self.proc.stdout.readline() - m = re.search(r"listening at (?:http://)?([^:]+:\d+)", line) - if not m: - raise RuntimeError(f"Failed to parse mitmdump output: {line}") - host_port = m.group(1) - return f"http://{self.auth_user}:{self.auth_pass}@{host_port}" - - def stop(self) -> None: - self.proc.kill() - self.proc.communicate() - - -def _wrong_credentials(proxy_url: str) -> str: - bad_auth_proxy = list(urlsplit(proxy_url)) - bad_auth_proxy[1] = bad_auth_proxy[1].replace("scrapy:scrapy@", "wrong:wronger@") - return urlunsplit(bad_auth_proxy) - - -@pytest.mark.requires_mitmproxy -class TestProxyConnect: - @classmethod - def setup_class(cls): - cls.mockserver = MockServer() - cls.mockserver.__enter__() - - @classmethod - def teardown_class(cls): - cls.mockserver.__exit__(None, None, None) - - def setup_method(self): - self._oldenv = os.environ.copy() - self._proxy = MitmProxy() - proxy_url = self._proxy.start() - os.environ["https_proxy"] = proxy_url - os.environ["http_proxy"] = proxy_url - - def teardown_method(self): - self._proxy.stop() - os.environ = self._oldenv - - @inline_callbacks_test - def test_https_connect_tunnel(self): - crawler = get_crawler(SimpleSpider) - with LogCapture() as log: - yield crawler.crawl(self.mockserver.url("/status?n=200", is_secure=True)) - self._assert_got_response_code(200, log) - - @inline_callbacks_test - def test_https_tunnel_auth_error(self): - os.environ["https_proxy"] = _wrong_credentials(os.environ["https_proxy"]) - crawler = get_crawler(SimpleSpider) - with LogCapture() as log: - yield crawler.crawl(self.mockserver.url("/status?n=200", is_secure=True)) - # The proxy returns a 407 error code but it does not reach the client; - # he just sees a TunnelError. - self._assert_got_tunnel_error(log) - - @inline_callbacks_test - def test_https_tunnel_without_leak_proxy_authorization_header(self): - request = Request(self.mockserver.url("/echo", is_secure=True)) - crawler = get_crawler(SingleRequestSpider) - with LogCapture() as log: - yield crawler.crawl(seed=request) - self._assert_got_response_code(200, log) - echo = json.loads(crawler.spider.meta["responses"][0].text) - assert "Proxy-Authorization" not in echo["headers"] - - def _assert_got_response_code(self, code, log): - assert str(log).count(f"Crawled ({code})") == 1 - - def _assert_got_tunnel_error(self, log): - assert "TunnelError" in str(log)