From 0540658e34fee6c4729eea3a7756e99aac073f10 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 29 Jul 2026 17:38:58 +0200 Subject: [PATCH 1/2] Limit response headers size like browsers --- docs/topics/download-handlers.rst | 59 ++++++-- docs/topics/settings.rst | 64 ++++++++ scrapy/core/downloader/handlers/_base_http.py | 4 + scrapy/core/downloader/handlers/_httpx.py | 22 +++ scrapy/core/downloader/handlers/http11.py | 137 +++++++++++++++++- scrapy/core/http2/protocol.py | 39 ++++- scrapy/core/http2/stream.py | 17 +++ scrapy/exceptions.py | 7 + scrapy/settings/default_settings.py | 7 + scrapy/utils/_download_handlers.py | 29 ++++ tests/mockserver/http.py | 2 + tests/mockserver/http_resources.py | 14 ++ tests/test_downloader_handler_httpx.py | 8 + .../test_downloader_handler_twisted_http11.py | 67 ++++++++- tests/utils/bases/download_handlers_http.py | 110 ++++++++++++++ 15 files changed, 569 insertions(+), 17 deletions(-) diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 34ab4f105..b0c5ab3fa 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -130,6 +130,8 @@ these exceptions. .. autoexception:: scrapy.exceptions.ResponseDataLossError +.. autoexception:: scrapy.exceptions.ResponseHeadersTooLargeError + .. autoexception:: scrapy.exceptions.UnsupportedURLSchemeError .. _download-handlers-ref: @@ -148,17 +150,46 @@ using different handlers. Here is a comparison of some features of the built-in HTTP handlers, see the individual handler docs for more differences: -================== ================= ===================== ==================== -Feature H2DownloadHandler HTTP11DownloadHandler HttpxDownloadHandler -================== ================= ===================== ==================== -Requires asyncio No No Yes -Requires a reactor Yes Yes No -HTTP/1.1 No Yes Yes -HTTP/2 Yes No Yes -TLS implementation ``cryptography`` ``cryptography`` Stdlib ``ssl`` -HTTP proxies No Yes Yes -SOCKS proxies No No Yes -================== ================= ===================== ==================== +.. list-table:: + :header-rows: 1 + + * - Feature + - H2DownloadHandler + - HTTP11DownloadHandler + - HttpxDownloadHandler + * - Requires asyncio + - No + - No + - Yes + * - Requires a reactor + - Yes + - Yes + - No + * - HTTP/1.1 + - No + - Yes + - Yes + * - HTTP/2 + - Yes + - No + - Yes + * - TLS implementation + - ``cryptography`` + - ``cryptography`` + - Stdlib ``ssl`` + * - HTTP proxies + - No + - Yes + - Yes + * - SOCKS proxies + - No + - No + - Yes + * - Response header size limit + - :setting:`DOWNLOAD_HEADERS_MAXSIZE` + - :setting:`DOWNLOAD_HEADERS_MAXSIZE` + - | HTTP/1.1: 100 KiB + | HTTP/2: 64 KiB You can find additional HTTP download handlers in the scrapy-download-handlers-incubator_ package. This package is made by the Scrapy @@ -329,6 +360,12 @@ Other limitations: limitations of ``httpx``) which may lead to higher resource usage when using proxy rotation. +- ``httpx`` does not allow configuring the response header size limit of its + HTTP client, so :setting:`DOWNLOAD_HEADERS_MAXSIZE` and + :setting:`DOWNLOAD_HEADERS_WARNSIZE` have no effect. Over HTTP/1.1 the + limit is also approximate, as it only applies while the response head is + still incomplete, so a complete head slightly above it is accepted. + .. setting:: HTTPX_HTTP2_ENABLED HTTPX_HTTP2_ENABLED diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 8068824e3..c6bfed94f 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1077,6 +1077,70 @@ The amount of time (in secs) that the downloader will wait before timing out. handler `, so it's not guaranteed to be supported by all 3rd-party handlers. +.. setting:: DOWNLOAD_HEADERS_MAXSIZE + +DOWNLOAD_HEADERS_MAXSIZE +------------------------ + +.. versionadded:: VERSION + +Default: ``393216`` (384 KiB) + +The maximum response header size (in bytes) allowed. Responses with bigger +headers are aborted and raise +:exc:`~scrapy.exceptions.ResponseHeadersTooLargeError`. + +This limit applies to the response head as a whole, i.e. to the status line +plus every header line, rather than to individual headers. That mirrors what +web browsers do; the default value matches the limit of Firefox_, the most +lenient of them. + +Use ``0`` to disable this limit. + +.. _Firefox: https://searchfox.org/mozilla-central/search?q=network.http.max_response_header_size + +.. note:: + + Over HTTP/2 this limit is applied as + ``SETTINGS_MAX_HEADER_LIST_SIZE``, which `RFC 9113 §6.5.2`_ defines in + terms of the uncompressed size of every header field plus an overhead of 32 + bytes each, rather than in terms of the bytes read from the connection. + Web browsers use the same value for both protocols regardless of that + difference, and so does Scrapy. + +.. _RFC 9113 §6.5.2: https://datatracker.ietf.org/doc/html/rfc9113#section-6.5.2 + +.. note:: + + Handling of this setting needs to be implemented inside the :ref:`download + handler `, so it's not guaranteed to be supported + by all handlers. See :ref:`download-handlers-ref`. + +.. setting:: DOWNLOAD_HEADERS_WARNSIZE + +DOWNLOAD_HEADERS_WARNSIZE +------------------------- + +.. versionadded:: VERSION + +Default: ``262144`` (256 KiB) + +If the size of the headers of a response exceeds this value, a warning will be +logged about it. The default value matches the limit of Chromium_, the least +lenient of the web browsers used as a reference for +:setting:`DOWNLOAD_HEADERS_MAXSIZE`, so that responses that some web browsers +would reject are reported even though Scrapy accepts them. + +Use ``0`` to disable this limit. + +.. _Chromium: https://source.chromium.org/chromium/chromium/src/+/main:net/http/http_stream_parser.h + +.. note:: + + Handling of this setting needs to be implemented inside the :ref:`download + handler `, so it's not guaranteed to be supported + by all handlers. See :ref:`download-handlers-ref`. + .. setting:: DOWNLOAD_MAXSIZE .. reqmeta:: download_maxsize diff --git a/scrapy/core/downloader/handlers/_base_http.py b/scrapy/core/downloader/handlers/_base_http.py index 83d130462..a6fd23237 100644 --- a/scrapy/core/downloader/handlers/_base_http.py +++ b/scrapy/core/downloader/handlers/_base_http.py @@ -16,6 +16,10 @@ class BaseHttpDownloadHandler(BaseDownloadHandler, ABC): super().__init__(crawler) self._default_maxsize: int = crawler.settings.getint("DOWNLOAD_MAXSIZE") self._default_warnsize: int = crawler.settings.getint("DOWNLOAD_WARNSIZE") + self._headers_maxsize: int = crawler.settings.getint("DOWNLOAD_HEADERS_MAXSIZE") + self._headers_warnsize: int = crawler.settings.getint( + "DOWNLOAD_HEADERS_WARNSIZE" + ) self._fail_on_dataloss: bool = crawler.settings.getbool( "DOWNLOAD_FAIL_ON_DATALOSS" ) diff --git a/scrapy/core/downloader/handlers/_httpx.py b/scrapy/core/downloader/handlers/_httpx.py index d5a4e9fcd..94c5b52a3 100644 --- a/scrapy/core/downloader/handlers/_httpx.py +++ b/scrapy/core/downloader/handlers/_httpx.py @@ -14,6 +14,7 @@ from scrapy.exceptions import ( DownloadFailedError, DownloadTimeoutError, NotConfigured, + ResponseHeadersTooLargeError, UnsupportedURLSchemeError, ) from scrapy.http import Headers @@ -70,6 +71,22 @@ else: pass +def _is_headers_too_large_exception(exc: BaseException) -> bool: + """Return whether *exc* was caused by response headers exceeding the limit + of the underlying HTTP client, which httpx does not let us configure and + reports as a generic protocol error.""" + for cause in _iter_exc_causes(exc): + # h11 reports it as a protocol error hinting at the 431 status, which + # it uses for nothing else. + if getattr(cause, "error_status_hint", None) == 431: + return True + # h2 reports it as a symptom of an HPACK bomb, which is what exceeding + # SETTINGS_MAX_HEADER_LIST_SIZE looks like from its point of view. + if HAS_HTTP2 and isinstance(cause, h2.exceptions.DenialOfServiceError): + return True + return False + + if TYPE_CHECKING: _Base = BaseStreamingDownloadHandler[httpx.Response] else: @@ -179,6 +196,11 @@ class HttpxDownloadHandler(_Base): except httpx.ProxyError as e: raise DownloadConnectionRefusedError(str(e)) from e except DOWNLOAD_FAILED_EXCEPTIONS as e: # pylint: disable=catching-non-exception + if _is_headers_too_large_exception(e): + raise ResponseHeadersTooLargeError( + f"Received response headers larger than the limit of the " + f"HTTP client while requesting {request.url}: {e}" + ) from e raise DownloadFailedError(str(e)) from e @staticmethod diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 17dca5acb..703fd48bd 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -5,6 +5,7 @@ from __future__ import annotations import ipaddress import logging import re +import sys from contextlib import suppress from functools import partial from io import BytesIO @@ -17,12 +18,14 @@ from twisted.internet.defer import Deferred, succeed from twisted.internet.endpoints import TCP4ClientEndpoint from twisted.internet.protocol import Factory, Protocol, connectionDone from twisted.python.failure import Failure +from twisted.web._newclient import HTTP11ClientProtocol, HTTPClientParser from twisted.web.client import ( URI, Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, + _HTTP11ClientFactory, ) from twisted.web.client import Response as TxResponse from twisted.web.http import PotentialDataLoss, _DataLoss @@ -37,12 +40,15 @@ from scrapy.exceptions import ( DownloadTimeoutError, NotConfigured, ResponseDataLossError, + ResponseHeadersTooLargeError, StopDownload, ) from scrapy.http import Headers, Response from scrapy.utils._download_handlers import ( check_stop_download, get_dataloss_msg, + get_headers_maxsize_msg, + get_headers_warnsize_msg, get_maxsize_msg, get_warnsize_msg, make_response, @@ -59,8 +65,11 @@ from scrapy.utils.url import add_http_if_no_scheme from ._base_http import BaseHttpDownloadHandler if TYPE_CHECKING: + from collections.abc import Callable + from twisted.internet.base import ReactorBase from twisted.internet.interfaces import IConsumer + from twisted.web._newclient import Request as TxClientRequest # typing.NotRequired requires Python 3.11 from typing_extensions import NotRequired @@ -82,6 +91,105 @@ class _ResultT(TypedDict): stop_download: NotRequired[StopDownload | None] +def _limit_response_headers( + parser: HTTPClientParser, maxsize: int, warnsize: int +) -> None: + """Make *parser* enforce *maxsize* and *warnsize* on the total size of the + response head, i.e. the status line plus the header lines plus their line + delimiters. + + Twisted limits the length of *individual* lines instead + (:attr:`~twisted.protocols.basic.LineReceiver.MAX_LENGTH`, 16 KiB), and + silently drops the connection when a line exceeds it, which makes the + resulting error impossible to tell apart from an unrelated disconnection. + Web browsers instead limit the total size of the response head, which is + what this function reproduces. See + https://github.com/twisted/twisted/issues/8570. + + Twisted builds its response parser without offering a way to configure it, + hence the instance-level overrides below. + """ + url = to_unicode(parser.request.absoluteURI or parser.request.uri) + # Keep the Twisted line length limit, which cannot report a meaningful + # error, from ever triggering before our own limit does. + parser.MAX_LENGTH = maxsize or sys.maxsize + delimiter_size = len(parser.delimiter) + line_received = parser.lineReceived + size = 0 + warned = False + + def lineReceived(line: bytes) -> None: + nonlocal size, warned + size += len(line) + delimiter_size + if maxsize and size > maxsize: + raise ResponseHeadersTooLargeError( + get_headers_maxsize_msg(size, maxsize, url) + ) + if warnsize and size > warnsize and not warned: + warned = True + logger.warning(get_headers_warnsize_msg(size, warnsize, url)) + line_received(line) # type: ignore[no-untyped-call] + + def lineLengthExceeded(line: bytes) -> None: + # A single line went over MAX_LENGTH, so lineReceived() never saw it. + # line holds the whole unparsed remainder of the buffer, so the + # reported size is a lower bound. + raise ResponseHeadersTooLargeError( + get_headers_maxsize_msg(size + len(line), maxsize, url) + ) + + parser.lineReceived = lineReceived # type: ignore[method-assign] + parser.lineLengthExceeded = lineLengthExceeded # type: ignore[method-assign] + + +class _ScrapyHTTP11ClientProtocol(HTTP11ClientProtocol): + """:class:`~twisted.web._newclient.HTTP11ClientProtocol` subclass that + limits the size of response headers.""" + + def __init__( + self, + quiescentCallback: Callable[[HTTP11ClientProtocol], None], + headers_maxsize: int, + headers_warnsize: int, + ): + super().__init__(quiescentCallback) # type: ignore[no-untyped-call] + self._headers_maxsize: int = headers_maxsize + self._headers_warnsize: int = headers_warnsize + + def request(self, request: TxClientRequest) -> Deferred[IResponse]: + 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: + _limit_response_headers( + self._parser, self._headers_maxsize, self._headers_warnsize + ) + return deferred + + +class _ScrapyHTTP11ClientFactory(_HTTP11ClientFactory): + """:class:`!twisted.web.client._HTTP11ClientFactory` subclass that builds + :class:`_ScrapyHTTP11ClientProtocol` instances.""" + + noisy = False + + def __init__( + self, + quiescentCallback: Callable[[HTTP11ClientProtocol], None], + metadata: str, + headers_maxsize: int, + headers_warnsize: int, + ): + super().__init__(quiescentCallback, metadata) # type: ignore[no-untyped-call] + self._headers_maxsize: int = headers_maxsize + self._headers_warnsize: int = headers_warnsize + + def buildProtocol(self, addr: Any) -> _ScrapyHTTP11ClientProtocol: + return _ScrapyHTTP11ClientProtocol( + self._quiescentCallback, self._headers_maxsize, self._headers_warnsize + ) + + class HTTP11DownloadHandler(BaseHttpDownloadHandler): def __init__(self, crawler: Crawler): if not crawler.settings.getbool("TWISTED_REACTOR_ENABLED"): @@ -95,7 +203,11 @@ class HTTP11DownloadHandler(BaseHttpDownloadHandler): self._pool.maxPersistentPerHost = crawler.settings.getint( "CONCURRENT_REQUESTS_PER_DOMAIN" ) - self._pool._factory.noisy = False + self._pool._factory = partial( # type: ignore[assignment] + _ScrapyHTTP11ClientFactory, + headers_maxsize=self._headers_maxsize, + headers_warnsize=self._headers_warnsize, + ) self._contextFactory: IPolicyForHTTPS = _load_context_factory_from_settings( crawler @@ -121,6 +233,7 @@ class HTTP11DownloadHandler(BaseHttpDownloadHandler): warnsize=getattr( self._crawler.spider, "download_warnsize", self._default_warnsize ), + headers_maxsize=self._headers_maxsize, fail_on_dataloss=self._fail_on_dataloss, crawler=self._crawler, tls_verbose_logging=self._tls_verbose_logging, @@ -185,6 +298,7 @@ class _TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): contextFactory: IPolicyForHTTPS, timeout: float = 30, bindAddress: tuple[str, int] | None = None, + headersMaxsize: int = 0, ): proxyHost, proxyPort, self._proxyAuthHeader = proxyConf super().__init__(reactor, proxyHost, proxyPort, timeout, bindAddress) @@ -193,6 +307,7 @@ class _TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): self._tunneledPort: int = port self._contextFactory: IPolicyForHTTPS = contextFactory self._connectBuffer: bytearray = bytearray() + self._headersMaxsize: int = headersMaxsize def requestTunnel(self, protocol: Protocol) -> Protocol: """Asks the proxy to open a tunnel.""" @@ -219,6 +334,20 @@ class _TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): # # see https://github.com/scrapy/scrapy/issues/2491 if b"\r\n\r\n" not in self._connectBuffer: + if self._headersMaxsize and len(self._connectBuffer) > self._headersMaxsize: + # A proxy that never ends its response head would otherwise + # make _connectBuffer grow without bound. + self._protocol.dataReceived = self._protocolDataReceived # type: ignore[method-assign] + self._protocol.transport.loseConnection() + self._tunnelReadyDeferred.errback( + ResponseHeadersTooLargeError( + get_headers_maxsize_msg( + len(self._connectBuffer), + self._headersMaxsize, + f"CONNECT {self._tunneledHost}:{self._tunneledPort}", + ) + ) + ) return self._protocol.dataReceived = self._protocolDataReceived # type: ignore[method-assign] respm = _TunnelingTCP4ClientEndpoint._responseMatcher.match(self._connectBuffer) @@ -298,10 +427,12 @@ class _TunnelingAgent(Agent): connectTimeout: float | None = None, bindAddress: tuple[str, int] | None = None, pool: HTTPConnectionPool | None = None, + headersMaxsize: int = 0, ): super().__init__(reactor, contextFactory, connectTimeout, bindAddress, pool) # type: ignore[no-untyped-call] self._proxyConf: tuple[str, int, bytes | None] = proxyConf self._contextFactory: IPolicyForHTTPS = contextFactory + self._headersMaxsize: int = headersMaxsize def _getEndpoint(self, uri: URI) -> _TunnelingTCP4ClientEndpoint: return _TunnelingTCP4ClientEndpoint( @@ -312,6 +443,7 @@ class _TunnelingAgent(Agent): contextFactory=self._contextFactory, timeout=self._endpointFactory._connectTimeout, bindAddress=self._endpointFactory._bindAddress, + headersMaxsize=self._headersMaxsize, ) def _requestWithEndpoint( @@ -391,6 +523,7 @@ class _ScrapyAgent: pool: HTTPConnectionPool | None = None, maxsize: int = 0, warnsize: int = 0, + headers_maxsize: int = 0, fail_on_dataloss: bool = True, crawler: Crawler, tls_verbose_logging: bool = False, @@ -401,6 +534,7 @@ class _ScrapyAgent: self._pool: HTTPConnectionPool | None = pool self._maxsize: int = maxsize self._warnsize: int = warnsize + self._headers_maxsize: int = headers_maxsize self._fail_on_dataloss: bool = fail_on_dataloss self._txresponse: TxResponse | None = None self._crawler: Crawler = crawler @@ -434,6 +568,7 @@ class _ScrapyAgent: connectTimeout=timeout, bindAddress=bindaddress, pool=self._pool, + headersMaxsize=self._headers_maxsize, ) return _ScrapyProxyAgent( reactor=reactor, diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index 7136e829e..a6bdd5ee5 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -20,7 +20,7 @@ from h2.events import ( UnknownFrameReceived, WindowUpdated, ) -from h2.exceptions import FrameTooLargeError, H2Error +from h2.exceptions import DenialOfServiceError, FrameTooLargeError, H2Error from twisted.internet.interfaces import ( IAddress, IHandshakeListener, @@ -32,8 +32,9 @@ from twisted.protocols.policies import TimeoutMixin from zope.interface import implementer from scrapy.core.http2.stream import Stream, StreamCloseReason -from scrapy.exceptions import DownloadTimeoutError +from scrapy.exceptions import DownloadTimeoutError, ResponseHeadersTooLargeError from scrapy.http import Request, Response +from scrapy.utils._download_handlers import get_headers_maxsize_msg from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.ssl import _log_ssl_conn_debug_info @@ -111,6 +112,24 @@ class H2ClientProtocol(Protocol, TimeoutMixin): config = H2Configuration(client_side=True, header_encoding="utf-8") self.conn = H2Connection(config=config) + # Unlike HTTP/1.1, where we count the bytes of the response head as + # they come from the wire, HTTP/2 uses the accounting that RFC 9113 + # §6.5.2 defines for SETTINGS_MAX_HEADER_LIST_SIZE: the uncompressed + # size of every header field plus 32 bytes of overhead each. Web + # browsers use the same limit for both protocols despite that + # difference, so we do the same. + self._headers_maxsize: int = settings.getint("DOWNLOAD_HEADERS_MAXSIZE") + # SETTINGS values are 32-bit, so the maximum stands for "no limit". + max_header_list_size = self._headers_maxsize or 2**32 - 1 + # local_settings is what we advertise to the remote peer, while the + # decoder is what enforces the limit on the headers we do receive. + self.conn.local_settings.max_header_list_size = max_header_list_size + # Setting a value only queues it; acknowledging it here, before the + # connection is initiated, makes the initial SETTINGS frame carry it. + self.conn.local_settings.acknowledge() + self.conn.decoder.max_header_list_size = max_header_list_size + self.headers_warnsize: int = settings.getint("DOWNLOAD_HEADERS_WARNSIZE") + # ID of the next request stream # Following the convention - 'Streams initiated by a client MUST # use odd-numbered stream identifiers' (RFC 7540 - Section 5.1.1) @@ -306,6 +325,22 @@ class H2ClientProtocol(Protocol, TimeoutMixin): events = self.conn.receive_data(data) self._handle_events(events) except H2Error as e: + if isinstance(e, DenialOfServiceError): + # h2 only raises this when the remote peer disregards the + # SETTINGS_MAX_HEADER_LIST_SIZE that we advertised. + self._lose_connection_with_error( + [ + ResponseHeadersTooLargeError( + get_headers_maxsize_msg( + None, + self._headers_maxsize, + self.metadata["uri"].toBytes().decode(), + ) + ) + ] + ) + return + if isinstance(e, FrameTooLargeError): # hyper-h2 does not drop the connection in this scenario, we # need to abort the connection manually. diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index c6226bbca..4ca2f4702 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -15,6 +15,7 @@ from twisted.web.client import ResponseFailed from scrapy.exceptions import DownloadCancelledError from scrapy.http.headers import Headers from scrapy.utils._download_handlers import ( + get_headers_warnsize_msg, get_maxsize_msg, get_warnsize_msg, make_response, @@ -369,6 +370,22 @@ class Stream: else: self._response["headers"].appendlist(name, value) + # DOWNLOAD_HEADERS_MAXSIZE is enforced by the HPACK decoder, which has + # no equivalent hook for a warning, so the warning happens here instead, + # using the same accounting that RFC 9113 §6.5.2 defines. Headers are + # already decoded at this point, so character counts stand in for the + # octet counts of the specification, which only differ for non-ASCII + # values. + headers_warnsize = self._protocol.headers_warnsize + if headers_warnsize: + headers_size = sum(len(name) + len(value) + 32 for name, value in headers) + if headers_size > headers_warnsize: + logger.warning( + get_headers_warnsize_msg( + headers_size, headers_warnsize, self._request.url + ) + ) + # Check if we exceed the allowed max data size which can be received expected_size = int(self._response["headers"].get(b"Content-Length", -1)) if self._download_maxsize and expected_size > self._download_maxsize: diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index ccaccddf2..95b0fe236 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -130,6 +130,13 @@ class ResponseDataLossError(Exception): """Indicates that Scrapy couldn't get a complete response.""" +class ResponseHeadersTooLargeError(DownloadFailedError): + """Indicates that the response headers exceeded + :setting:`DOWNLOAD_HEADERS_MAXSIZE`, or the equivalent limit of the + underlying HTTP client for :ref:`download handlers + ` that do not support that setting.""" + + class UnsupportedURLSchemeError(Exception): """Indicates that the URL scheme is not supported.""" diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 993f2436d..8c03be010 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -70,6 +70,8 @@ __all__ = [ "DOWNLOAD_FAIL_ON_DATALOSS", "DOWNLOAD_HANDLERS", "DOWNLOAD_HANDLERS_BASE", + "DOWNLOAD_HEADERS_MAXSIZE", + "DOWNLOAD_HEADERS_WARNSIZE", "DOWNLOAD_MAXSIZE", "DOWNLOAD_SLOTS", "DOWNLOAD_TIMEOUT", @@ -291,6 +293,11 @@ DOWNLOAD_HANDLERS_BASE = { "ftp": "scrapy.core.downloader.handlers.ftp.FTPDownloadHandler", } +# Firefox, the most lenient of the reference web browsers, allows 384k. +DOWNLOAD_HEADERS_MAXSIZE = 384 * 1024 # 384k +# Chromium, the least lenient of the reference web browsers, allows 256k. +DOWNLOAD_HEADERS_WARNSIZE = 256 * 1024 # 256k + DOWNLOAD_MAXSIZE = 1024 * 1024 * 1024 # 1024m DOWNLOAD_WARNSIZE = 32 * 1024 * 1024 # 32m diff --git a/scrapy/utils/_download_handlers.py b/scrapy/utils/_download_handlers.py index 23b715ac6..945d7d58d 100644 --- a/scrapy/utils/_download_handlers.py +++ b/scrapy/utils/_download_handlers.py @@ -21,6 +21,7 @@ from scrapy.exceptions import ( DownloadConnectionRefusedError, DownloadFailedError, DownloadTimeoutError, + ResponseHeadersTooLargeError, StopDownload, UnsupportedURLSchemeError, ) @@ -62,6 +63,13 @@ def wrap_twisted_exceptions() -> Iterator[None]: except DNSLookupError as e: raise CannotResolveHostError(str(e)) from e except ResponseFailed as e: + # Twisted reports parsing errors as the underlying reason of a generic + # failure; report the ones we raise ourselves as themselves. Reasons may + # be either Failure objects or plain exceptions. + for reason in e.reasons: + value = reason.value if isinstance(reason, Failure) else reason + if isinstance(value, ResponseHeadersTooLargeError): + raise value from e raise DownloadFailedError(str(e)) from e except TxTimeoutError as e: raise DownloadTimeoutError(str(e)) from e @@ -137,6 +145,27 @@ def get_warnsize_msg(size: int, limit: int, request: Request, *, expected: bool) ) +def get_headers_maxsize_msg(size: int | None, limit: int, url: str) -> str: + # size is None when the underlying HTTP client only reports that the limit + # was exceeded, and not by how much. + prefix = ( + "Received response headers" + if size is None + else f"Received {size} bytes of response headers" + ) + return ( + f"{prefix}, which is larger than the download headers max size " + f"({limit}), while requesting {url}." + ) + + +def get_headers_warnsize_msg(size: int, limit: int, url: str) -> str: + return ( + f"Received {size} bytes of response headers, which is larger than the " + f"download headers warn size ({limit}), while requesting {url}." + ) + + def get_dataloss_msg(url: str) -> str: return ( f"Got data loss in {url}. If you want to process broken " diff --git a/tests/mockserver/http.py b/tests/mockserver/http.py index 7ad873c02..307822243 100644 --- a/tests/mockserver/http.py +++ b/tests/mockserver/http.py @@ -26,6 +26,7 @@ from .http_resources import ( ForeverTakingResource, HostHeaderResource, LargeChunkedFileResource, + LargeHeadersResource, NoMetaRefreshRedirect, Partial, PayloadResource, @@ -83,6 +84,7 @@ class Root(resource.Resource): self.putChild(b"largechunkedfile", LargeChunkedFileResource()) self.putChild(b"compress", Compress()) self.putChild(b"duplicate-header", DuplicateHeaderResource()) + self.putChild(b"large-headers", LargeHeadersResource()) self.putChild(b"response-headers", ResponseHeadersResource()) self.putChild(b"set-cookie", SetCookie()) self.putChild(b"uri", UriResource()) diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index 98ac6cf6a..6b507fe75 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -326,6 +326,20 @@ class DuplicateHeaderResource(resource.Resource): return b"" +class LargeHeadersResource(resource.Resource): + """Return a response with ``count`` headers whose values are ``size`` bytes + long each.""" + + def render(self, request): + size = getarg(request, b"size", 100, type_=int) + count = getarg(request, b"count", 1, type_=int) + for index in range(count): + request.responseHeaders.setRawHeaders( + f"X-Large-{index}".encode(), [b"a" * size] + ) + return b"" + + class UriResource(resource.Resource): """Return the full uri that was requested""" diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index 5ceb93382..015c75c6a 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -43,6 +43,12 @@ if find_spec("httpx2") is None and find_spec("httpx") is None: class HttpxDownloadHandlerMixin: + # httpx does not expose the response head size limit of its HTTP client, so + # DOWNLOAD_HEADERS_MAXSIZE and DOWNLOAD_HEADERS_WARNSIZE do not apply. Over + # HTTP/1.1 the limit is httpcore's MAX_INCOMPLETE_EVENT_SIZE (100 KiB), + # which h11 only enforces once the head exceeds it and is still incomplete. + headers_maxsize: int | None = 100 * 1024 + @property def download_handler_cls(self) -> type[DownloadHandlerProtocol]: return HttpxDownloadHandler @@ -91,6 +97,8 @@ class TestHttps(HttpxDownloadHandlerMixin, TestHttpsBase): class TestHttp2(TestHttps): http2 = True handler_supports_http2_dataloss = False + # Over HTTP/2 the limit is instead h2's DEFAULT_MAX_HEADER_LIST_SIZE. + headers_maxsize = 64 * 1024 default_handler_settings: ClassVar[dict[str, Any]] = { "HTTPX_HTTP2_ENABLED": True, diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 32dfd7540..e29e4a277 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -3,14 +3,18 @@ from __future__ import annotations import sys -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast +from unittest.mock import Mock import pytest from scrapy import Spider -from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler +from scrapy.core.downloader.handlers.http11 import ( + HTTP11DownloadHandler, + _TunnelingTCP4ClientEndpoint, +) from scrapy.crawler import Crawler -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import NotConfigured, ResponseHeadersTooLargeError from tests.utils.bases.download_handlers_http import ( TestHttpBase, TestHttpProxyBase, @@ -27,6 +31,8 @@ from tests.utils.bases.download_handlers_http import ( ) if TYPE_CHECKING: + from twisted.python.failure import Failure + from scrapy.core.downloader.handlers import DownloadHandlerProtocol @@ -117,3 +123,58 @@ class TestRealWebsite(HTTP11DownloadHandlerMixin, TestRealWebsiteBase): @property def platform_cert_store_works(self) -> bool: return sys.platform != "win32" + + +class TestTunnelingHeadersMaxsize: + """Tests for the DOWNLOAD_HEADERS_MAXSIZE limit that + ``_TunnelingTCP4ClientEndpoint`` applies to the response head of the proxy, + which no HTTP client parses for us.""" + + def _get_endpoint(self, headers_maxsize: int) -> _TunnelingTCP4ClientEndpoint: + from twisted.internet import reactor + + endpoint = _TunnelingTCP4ClientEndpoint( + reactor=cast("Any", reactor), + host="example.com", + port=443, + proxyConf=("proxy.example.com", 8080, None), + contextFactory=cast("Any", None), + headersMaxsize=headers_maxsize, + ) + endpoint._protocol = cast("Any", Mock()) + endpoint._protocolDataReceived = Mock() + return endpoint + + def test_over_maxsize(self) -> None: + endpoint = self._get_endpoint(1024) + failures: list[Failure] = [] + endpoint._tunnelReadyDeferred.addErrback(failures.append) + + # A proxy response head that never ends. + for _ in range(3): + endpoint.processProxyResponse(b"a" * 512) + + assert len(failures) == 1 + assert failures[0].check(ResponseHeadersTooLargeError) + assert "1024" in str(failures[0].value) + assert "CONNECT example.com:443" in str(failures[0].value) + endpoint._protocol.transport.loseConnection.assert_called_once() # type: ignore[union-attr] + + def test_under_maxsize(self) -> None: + endpoint = self._get_endpoint(1024) + failures: list[Failure] = [] + endpoint._tunnelReadyDeferred.addErrback(failures.append) + + endpoint.processProxyResponse(b"a" * 512) + + assert not failures + + def test_maxsize_disabled(self) -> None: + endpoint = self._get_endpoint(0) + failures: list[Failure] = [] + endpoint._tunnelReadyDeferred.addErrback(failures.append) + + for _ in range(3): + endpoint.processProxyResponse(b"a" * 512) + + assert not failures diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index e44f9bcb8..8ecc586dd 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -26,6 +26,7 @@ from scrapy.exceptions import ( DownloadFailedError, DownloadTimeoutError, ResponseDataLossError, + ResponseHeadersTooLargeError, ScrapyDeprecationWarning, StopDownload, UnsupportedURLSchemeError, @@ -60,6 +61,11 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer +# Value that the tests set DOWNLOAD_HEADERS_MAXSIZE to for handlers that honor +# it, so that they do not depend on its default value and stay reasonably fast. +CONFIGURED_HEADERS_MAXSIZE = 64 * 1024 + + class TestHttpBase(ABC): is_secure: bool = False http2: bool = False @@ -75,6 +81,15 @@ class TestHttpBase(ABC): # default headers added by the underlying library that cannot be suppressed always_present_req_headers: ClassVar[frozenset[str]] = frozenset() default_handler_settings: ClassVar[dict[str, Any]] = {} + # Limit that the underlying HTTP client applies to the size of a single + # response header line, on top of headers_maxsize. 0 means that only + # headers_maxsize applies, which is what web browsers do. + header_line_maxsize: int = 0 + # Limit that applies to the size of the response head as a whole: None if + # the handler honors DOWNLOAD_HEADERS_MAXSIZE and DOWNLOAD_HEADERS_WARNSIZE, + # 0 if its HTTP client applies no limit at all, or else the hard-coded limit + # of that client. + headers_maxsize: int | None = None @property @abstractmethod @@ -461,6 +476,101 @@ class TestHttpBase(ABC): response = await download_handler.download_request(request) assert response.headers.getlist(b"Set-Cookie") == [b"a=b", b"c=d"] + @property + def _effective_headers_maxsize(self) -> int: + """Limit that the tests should expect the handler to apply, 0 for + none.""" + if self.headers_maxsize is None: + return CONFIGURED_HEADERS_MAXSIZE + return self.headers_maxsize + + @property + def _headers_maxsize_settings(self) -> dict[str, Any]: + if self.headers_maxsize is None: + return {"DOWNLOAD_HEADERS_MAXSIZE": CONFIGURED_HEADERS_MAXSIZE} + return {} + + @coroutine_test + async def test_get_long_header(self, mockserver: MockServer) -> None: + """A single header larger than the 16 KiB line length limit that + Twisted applies by default is still read. See + https://github.com/scrapy/scrapy/issues/355.""" + size = 32 * 1024 + if self.header_line_maxsize: + size = min(size, self.header_line_maxsize) + request = Request( + mockserver.url(f"/large-headers?size={size}", is_secure=self.is_secure) + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + assert response.headers[b"X-Large-0"] == b"a" * size + + @coroutine_test + async def test_get_headers_over_maxsize_single_header( + self, mockserver: MockServer + ) -> None: + if not self._effective_headers_maxsize: + pytest.skip(f"{type(self).__name__} does not limit response head size") + # Twice the limit, because some HTTP clients only enforce it once the + # unparsed head exceeds it by a whole read buffer. + size = self._effective_headers_maxsize * 2 + request = Request( + mockserver.url(f"/large-headers?size={size}", is_secure=self.is_secure) + ) + async with self.get_dh(self._headers_maxsize_settings) as download_handler: + with pytest.raises(ResponseHeadersTooLargeError): + await download_handler.download_request(request) + + @coroutine_test + async def test_get_headers_over_maxsize_many_headers( + self, mockserver: MockServer + ) -> None: + if not self._effective_headers_maxsize: + pytest.skip(f"{type(self).__name__} does not limit response head size") + size = 8 * 1024 + count = self._effective_headers_maxsize * 2 // size + request = Request( + mockserver.url( + f"/large-headers?size={size}&count={count}", is_secure=self.is_secure + ) + ) + async with self.get_dh(self._headers_maxsize_settings) as download_handler: + with pytest.raises(ResponseHeadersTooLargeError): + await download_handler.download_request(request) + + @coroutine_test + async def test_get_headers_maxsize_disabled(self, mockserver: MockServer) -> None: + if self.headers_maxsize is not None: + pytest.skip( + f"{type(self).__name__} does not support DOWNLOAD_HEADERS_MAXSIZE" + ) + size = self._effective_headers_maxsize * 2 + request = Request( + mockserver.url(f"/large-headers?size={size}", is_secure=self.is_secure) + ) + async with self.get_dh({"DOWNLOAD_HEADERS_MAXSIZE": 0}) as download_handler: + response = await download_handler.download_request(request) + assert response.headers[b"X-Large-0"] == b"a" * size + + @coroutine_test + async def test_get_headers_over_warnsize( + 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": size // 2} + 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" in caplog.text + @coroutine_test async def test_download_is_not_automatically_gzip_decoded( self, mockserver: MockServer From f04cc08587edc577c6ce2152acbacb2394fa79ae Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 29 Jul 2026 18:24:37 +0200 Subject: [PATCH 2/2] 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