From f04cc08587edc577c6ce2152acbacb2394fa79ae Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 29 Jul 2026 18:24:37 +0200 Subject: [PATCH] Improve test coverage --- scrapy/core/downloader/handlers/http11.py | 13 +++-- .../test_downloader_handler_twisted_http11.py | 53 +++++++++++++++++++ tests/utils/bases/download_handlers_http.py | 19 +++++++ 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 703fd48bd..8c1676e9d 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -157,12 +157,17 @@ class _ScrapyHTTP11ClientProtocol(HTTP11ClientProtocol): self._headers_warnsize: int = headers_warnsize def request(self, request: TxClientRequest) -> Deferred[IResponse]: + previous_parser = self._parser deferred = super().request(request) - # super() builds the response parser, and a new one is built for every - # request, so the size counter resets as intended on pooled connections. - if self._parser is not None: + # super() builds a new response parser for every request it accepts, so + # the size counter resets as intended on pooled connections. It can also + # refuse a request, e.g. on a pooled connection that just died, in which + # case it leaves the parser of the previous request in place, and + # limiting that one again would reset its counter mid-response. + parser = self._parser + if parser is not None and parser is not previous_parser: _limit_response_headers( - self._parser, self._headers_maxsize, self._headers_warnsize + parser, self._headers_maxsize, self._headers_warnsize ) return deferred diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index e29e4a277..8b9f027b0 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -7,10 +7,15 @@ from typing import TYPE_CHECKING, Any, cast from unittest.mock import Mock import pytest +from twisted.internet.testing import StringTransport +from twisted.web._newclient import Request as TxClientRequest +from twisted.web._newclient import RequestNotSent +from twisted.web.http_headers import Headers as TxHeaders from scrapy import Spider from scrapy.core.downloader.handlers.http11 import ( HTTP11DownloadHandler, + _ScrapyHTTP11ClientProtocol, _TunnelingTCP4ClientEndpoint, ) from scrapy.crawler import Crawler @@ -178,3 +183,51 @@ class TestTunnelingHeadersMaxsize: endpoint.processProxyResponse(b"a" * 512) assert not failures + + +class TestScrapyHTTP11ClientProtocol: + """Tests for the response header size limiting that + ``_ScrapyHTTP11ClientProtocol`` installs on the response parser that + Twisted builds for each request.""" + + @staticmethod + def _get_protocol() -> _ScrapyHTTP11ClientProtocol: + protocol = _ScrapyHTTP11ClientProtocol(lambda _: None, 64 * 1024, 32 * 1024) + protocol.makeConnection(StringTransport()) # type: ignore[no-untyped-call] + return protocol + + @staticmethod + def _get_request() -> TxClientRequest: + return TxClientRequest( + b"GET", b"/", TxHeaders({b"host": [b"example.com"]}), None + ) + + def test_parser_is_limited(self) -> None: + protocol = self._get_protocol() + protocol.request(self._get_request()) + assert protocol._parser is not None + assert protocol._parser.MAX_LENGTH == 64 * 1024 + # _limit_response_headers() overrides these on the instance. + assert "lineReceived" in vars(protocol._parser) + assert "lineLengthExceeded" in vars(protocol._parser) + + def test_refused_request_keeps_previous_parser_limits(self) -> None: + """A request that Twisted refuses leaves the parser of the previous + request untouched, so that its size counter is not reset while its + response is still being read.""" + protocol = self._get_protocol() + protocol.request(self._get_request()) + parser = protocol._parser + assert parser is not None + line_received = vars(parser)["lineReceived"] + + # A second request over the same connection is refused, as the first one + # is still in progress. + deferred = protocol.request(self._get_request()) + failures: list[Failure] = [] + deferred.addErrback(failures.append) + + assert len(failures) == 1 + assert failures[0].check(RequestNotSent) + assert protocol._parser is parser + assert vars(parser)["lineReceived"] is line_received diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index 8ecc586dd..2af7748fb 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -571,6 +571,25 @@ class TestHttpBase(ABC): assert response.headers[b"X-Large-0"] == b"a" * size assert "download headers warn size" in caplog.text + @coroutine_test + async def test_get_headers_warnsize_disabled( + self, mockserver: MockServer, caplog: pytest.LogCaptureFixture + ) -> None: + if self.headers_maxsize is not None: + pytest.skip( + f"{type(self).__name__} does not support DOWNLOAD_HEADERS_WARNSIZE" + ) + size = 32 * 1024 + request = Request( + mockserver.url(f"/large-headers?size={size}", is_secure=self.is_secure) + ) + settings = {"DOWNLOAD_HEADERS_WARNSIZE": 0} + with caplog.at_level(logging.WARNING): + async with self.get_dh(settings) as download_handler: + response = await download_handler.download_request(request) + assert response.headers[b"X-Large-0"] == b"a" * size + assert "download headers warn size" not in caplog.text + @coroutine_test async def test_download_is_not_automatically_gzip_decoded( self, mockserver: MockServer