From d6d6d2659ea5ab0b17d02c3653205c0ee6cc7b6b Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Sat, 8 Aug 2026 16:50:53 +0200 Subject: [PATCH] Honor user-set Content-Length request headers on HTTP/2 --- docs/topics/download-handlers.rst | 3 ++ scrapy/core/http2/stream.py | 18 ++------- .../test_downloader_handler_twisted_http11.py | 2 + .../test_downloader_handler_twisted_http2.py | 30 ++------------- tests/utils/bases/download_handlers_http.py | 37 +++++++++++++++++++ 5 files changed, 49 insertions(+), 41 deletions(-) diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index e0501c169..bfe5af6a8 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -254,6 +254,9 @@ Other limitations: - HTTPS proxies to HTTPS destinations are not supported. +- A ``Content-Length`` request header is sent in addition to the one built + from the request body, and most servers reject requests with two. + .. _httpx-handler: HttpxDownloadHandler diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index c6226bbca..3f048c1da 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -236,24 +236,12 @@ class Stream: (":path", path), ] - content_length = str(len(self._request.body)) - headers.append(("Content-Length", content_length)) + if b"Content-Length" not in self._request.headers: + headers.append(("Content-Length", str(len(self._request.body)))) - content_length_name = self._request.headers.normkey(b"Content-Length") for name, values in self._request.headers.items(): for value_bytes in values: - value = str(value_bytes, "utf-8") - if name == content_length_name: - if value != content_length: - logger.warning( - "Ignoring bad Content-Length header %r of request %r, " - "sending %r instead", - value, - self._request, - content_length, - ) - continue - headers.append((str(name, "utf-8"), value)) + headers.append((str(name, "utf-8"), str(value_bytes, "utf-8"))) return headers diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 32dfd7540..a949b9487 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -47,6 +47,8 @@ class HTTP11DownloadHandlerMixin: } } + handler_supports_custom_content_length = False + def test_not_configured_without_reactor() -> None: crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False}) diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 449f2d635..6a9917ebc 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -2,7 +2,6 @@ from __future__ import annotations -import logging import sys from typing import TYPE_CHECKING, Any @@ -121,33 +120,12 @@ class TestHttp2(H2DownloadHandlerMixin, TestHttpsBase): assert response.body == b"" @coroutine_test - async def test_custom_content_length_good(self, mockserver: MockServer) -> None: + async def test_custom_content_length_bad(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/contentlength", is_secure=self.is_secure)) - custom_content_length = str(len(request.body)) - request.headers["Content-Length"] = custom_content_length + request.headers["Content-Length"] = str(len(request.body) + 1) async with self.get_dh() as download_handler: - response = await download_handler.download_request(request) - assert response.text == custom_content_length - - @coroutine_test - async def test_custom_content_length_bad( - self, caplog: pytest.LogCaptureFixture, mockserver: MockServer - ) -> None: - request = Request(mockserver.url("/contentlength", is_secure=self.is_secure)) - actual_content_length = str(len(request.body)) - bad_content_length = str(len(request.body) + 1) - request.headers["Content-Length"] = bad_content_length - async with self.get_dh() as download_handler: - with caplog.at_level(logging.DEBUG): - response = await download_handler.download_request(request) - assert response.text == actual_content_length - assert ( - "scrapy.core.http2.stream", - logging.WARNING, - f"Ignoring bad Content-Length header " - f"{bad_content_length!r} of request {request}, sending " - f"{actual_content_length!r} instead", - ) in caplog.record_tuples + with pytest.raises(DownloadFailedError): + await download_handler.download_request(request) @coroutine_test async def test_data_loss_handling(self, mockserver: MockServer) -> None: diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index e44f9bcb8..cda90d42a 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -72,6 +72,9 @@ class TestHttpBase(ABC): # h2.connection.H2Connection.receive_data()), thus closing all streams that # were using it, and we handle this as a normal exception. handler_supports_http2_dataloss: bool = True + # whether the handler sends a request Content-Length header as is, instead + # of building its own (https://github.com/scrapy/scrapy/issues/4919) + handler_supports_custom_content_length: bool = True # 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]] = {} @@ -413,6 +416,40 @@ class TestHttpBase(ABC): assert len(contentlengths) == 1 assert contentlengths == [b"0"] + @coroutine_test + async def test_custom_content_length(self, mockserver: MockServer) -> None: + body = b"1" * 100 + request = Request( + mockserver.url("/echo", is_secure=self.is_secure), + method="POST", + body=body, + headers={"Content-Length": str(len(body))}, + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + if not self.handler_supports_custom_content_length: + # The server gets two Content-Length headers and rejects the + # request. + assert response.status == 400 + return + echo = json.loads(response.text) + assert Headers(echo["headers"]).getlist("Content-Length") == [b"100"] + assert echo["body"] == body.decode() + + @coroutine_test + async def test_custom_content_length_bodyless(self, mockserver: MockServer) -> None: + request = Request( + mockserver.url("/contentlength", is_secure=self.is_secure), + method="POST", + headers={"Content-Length": "0"}, + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + if not self.handler_supports_custom_content_length: + assert response.status == 400 + return + assert response.body == b"0" + @coroutine_test async def test_payload(self, mockserver: MockServer) -> None: body = b"1" * 100 # PayloadResource requires body length to be 100