Merge c7fcad1c2c into e28e56aa61

docs/topics/download-handlers.rst

@@ -253,6 +253,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

scrapy/core/_http2/stream.py

@@ -247,24 +247,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
 

tests/test_downloader_handler_twisted_http11.py

@@ -48,6 +48,8 @@ class HTTP11DownloadHandlerMixin:
             }
         }
 
+    handler_supports_custom_content_length = False
+
 
 def test_not_configured_without_reactor() -> None:
     crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False})

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
 
@@ -122,33 +121,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:

tests/utils/bases/download_handlers_http.py

@@ -75,6 +75,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
     # What the handler does with a bad response header line, e.g. one with no
     # colon in it:
     # "skip-bad": the bad line is skipped and the header lines that follow it
@@ -424,6 +427,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