diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index e99257b6f..86fc071f8 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 json from typing import TYPE_CHECKING, Any import pytest @@ -11,9 +10,13 @@ from twisted.web.http import H2_ENABLED from scrapy import Spider from scrapy.crawler import Crawler -from scrapy.exceptions import NotConfigured, UnsupportedURLSchemeError +from scrapy.exceptions import ( + DownloadFailedError, + NotConfigured, + UnsupportedURLSchemeError, +) from scrapy.http import Request -from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.defer import maybe_deferred_to_future from tests.test_downloader_handlers_http_base import ( TestHttpProxyBase, TestHttps11Base, @@ -23,6 +26,7 @@ from tests.test_downloader_handlers_http_base import ( TestHttpsWrongHostnameBase, TestHttpWithCrawlerBase, ) +from tests.utils.decorators import coroutine_test if TYPE_CHECKING: from scrapy.core.downloader.handlers import DownloadHandlerProtocol @@ -58,27 +62,16 @@ def test_not_configured_without_reactor() -> None: class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): - HTTP2_DATALOSS_SKIP_REASON = "Content-Length mismatch raises InvalidBodyLengthError" + http2 = True + handler_supports_http2_dataloss = False - @deferred_f_from_coro_f + @coroutine_test async def test_protocol(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/host", is_secure=self.is_secure)) async with self.get_dh() as download_handler: response = await download_handler.download_request(request) assert response.protocol == "h2" - def test_download_cause_data_loss(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - - def test_download_cause_data_loss_double_warning(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - - def test_download_allow_data_loss(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - - def test_download_allow_data_loss_via_setting(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - def test_download_conn_failed(self) -> None: # type: ignore[override] # Unlike HTTP11DownloadHandler which raises it from download_request() # (without any special handling), here ConnectionRefusedError (raised in @@ -87,12 +80,6 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): # DOWNLOAD_TIMEOUT. pytest.skip("The handler doesn't properly reraise ConnectionRefusedError") - def test_download_conn_lost(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - - def test_download_conn_aborted(self) -> None: # type: ignore[override] - pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) - def test_download_dns_error(self) -> None: # type: ignore[override] # Unlike HTTP11DownloadHandler which raises it from download_request() # (without any special handling), here DNSLookupError (raised in @@ -101,7 +88,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): # DOWNLOAD_TIMEOUT. pytest.skip("The handler doesn't properly reraise DNSLookupError") - @deferred_f_from_coro_f + @coroutine_test async def test_concurrent_requests_same_domain( self, mockserver: MockServer ) -> None: @@ -116,7 +103,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): assert response2.headers["Content-Length"] == b"79" @pytest.mark.xfail(reason="https://github.com/python-hyper/h2/issues/1247") - @deferred_f_from_coro_f + @coroutine_test async def test_connect_request(self, mockserver: MockServer) -> None: request = Request( mockserver.url("/file", is_secure=self.is_secure), method="CONNECT" @@ -125,7 +112,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): response = await download_handler.download_request(request) assert response.body == b"" - @deferred_f_from_coro_f + @coroutine_test async def test_custom_content_length_good(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/contentlength", is_secure=self.is_secure)) custom_content_length = str(len(request.body)) @@ -134,7 +121,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): response = await download_handler.download_request(request) assert response.text == custom_content_length - @deferred_f_from_coro_f + @coroutine_test async def test_custom_content_length_bad(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/contentlength", is_secure=self.is_secure)) actual_content_length = str(len(request.body)) @@ -154,15 +141,12 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): ) ) - @deferred_f_from_coro_f - async def test_duplicate_header(self, mockserver: MockServer) -> None: - request = Request(mockserver.url("/echo", is_secure=self.is_secure)) - header, value1, value2 = "Custom-Header", "foo", "bar" - request.headers.appendlist(header, value1) - request.headers.appendlist(header, value2) + @coroutine_test + async def test_data_loss_handling(self, mockserver: MockServer) -> None: + request = Request(mockserver.url("/broken", is_secure=self.is_secure)) async with self.get_dh() as download_handler: - response = await download_handler.download_request(request) - assert json.loads(response.text)["headers"][header] == [value1, value2] + with pytest.raises(DownloadFailedError): + await download_handler.download_request(request) class TestHttps2WrongHostname(H2DownloadHandlerMixin, TestHttpsWrongHostnameBase): @@ -214,7 +198,7 @@ class TestHttps2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): is_secure = True expected_http_proxy_request_body = b"/" - @deferred_f_from_coro_f + @coroutine_test async def test_download_with_proxy_https_timeout( self, proxy_mockserver: ProxyEchoMockServer ) -> None: @@ -223,7 +207,7 @@ class TestHttps2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): super().test_download_with_proxy_https_timeout(proxy_mockserver) # type: ignore[arg-type] ) - @deferred_f_from_coro_f + @coroutine_test async def test_download_with_proxy_without_http_scheme( self, proxy_mockserver: ProxyEchoMockServer ) -> None: diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 59b6d5754..66edf9325 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -521,7 +521,14 @@ class TestHttpBase(ABC): class TestHttp11Base(TestHttpBase): - """HTTP 1.1 test case""" + http2: bool = False + # RFC 9113 ยง8.1.1 explicitly says that a Content-Length mismatch is a + # stream error (of type PROTOCOL_ERROR) so the client will send + # RST_STREAM. Some libraries do only this while e.g. h2 also closes the + # connection (see handling of ProtocolError in + # 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 @coroutine_test async def test_download_without_maxsize_limit(self, mockserver: MockServer) -> None: @@ -638,12 +645,11 @@ class TestHttp11Base(TestHttpBase): response = await download_handler.download_request(request) assert response.body == b"chunked content\n" - @pytest.mark.parametrize("url", ["broken", "broken-chunked"]) @coroutine_test - async def test_download_cause_data_loss( - self, url: str, mockserver: MockServer - ) -> None: - request = Request(mockserver.url(f"/{url}", is_secure=self.is_secure)) + async def test_download_cause_data_loss(self, mockserver: MockServer) -> None: + if self.http2 and not self.handler_supports_http2_dataloss: + pytest.skip("This handler doesn't support dataloss on HTTP/2") + request = Request(mockserver.url("/broken", is_secure=self.is_secure)) async with self.get_dh() as download_handler: with pytest.raises(ResponseDataLossError): await download_handler.download_request(request) @@ -652,6 +658,8 @@ class TestHttp11Base(TestHttpBase): async def test_download_cause_data_loss_double_warning( self, caplog: pytest.LogCaptureFixture, mockserver: MockServer ) -> None: + if self.http2 and not self.handler_supports_http2_dataloss: + pytest.skip("This handler doesn't support dataloss on HTTP/2") request = Request(mockserver.url("/broken", is_secure=self.is_secure)) async with self.get_dh() as download_handler: with pytest.raises(ResponseDataLossError): @@ -663,25 +671,43 @@ class TestHttp11Base(TestHttpBase): # no repeated warning assert "Got data loss" not in caplog.text - @pytest.mark.parametrize("url", ["broken", "broken-chunked"]) @coroutine_test - async def test_download_allow_data_loss( - self, url: str, mockserver: MockServer + async def test_download_allow_data_loss_broken( + self, mockserver: MockServer ) -> None: + if self.http2 and not self.handler_supports_http2_dataloss: + pytest.skip("This handler doesn't support dataloss on HTTP/2") request = Request( - mockserver.url(f"/{url}", is_secure=self.is_secure), + mockserver.url("/broken", is_secure=self.is_secure), meta={"download_fail_on_dataloss": False}, ) async with self.get_dh() as download_handler: response = await download_handler.download_request(request) assert response.flags == ["dataloss"] + assert response.text == "partial" + + @coroutine_test + async def test_download_allow_data_loss_broken_chunked( + self, mockserver: MockServer + ) -> None: + if self.http2: + pytest.skip("Chunked encoding is specific to HTTP/1.1") + request = Request( + mockserver.url("/broken-chunked", is_secure=self.is_secure), + meta={"download_fail_on_dataloss": False}, + ) + async with self.get_dh() as download_handler: + response = await download_handler.download_request(request) + assert response.flags == ["dataloss"] + assert response.text == "chunked content\n" - @pytest.mark.parametrize("url", ["broken", "broken-chunked"]) @coroutine_test async def test_download_allow_data_loss_via_setting( - self, url: str, mockserver: MockServer + self, mockserver: MockServer ) -> None: - request = Request(mockserver.url(f"/{url}", is_secure=self.is_secure)) + if self.http2 and not self.handler_supports_http2_dataloss: + pytest.skip("This handler doesn't support dataloss on HTTP/2") + request = Request(mockserver.url("/broken", is_secure=self.is_secure)) async with self.get_dh( {"DOWNLOAD_FAIL_ON_DATALOSS": False} ) as download_handler: @@ -708,6 +734,12 @@ class TestHttp11Base(TestHttpBase): @coroutine_test async def test_download_conn_aborted(self, mockserver: MockServer) -> None: # copy of TestCrawl.test_retry_conn_aborted() + if self.http2: + # it may be possible to write a separate resource that does something + # suitable on HTTP/2 without sending Content-Length + pytest.skip( + "On HTTP/2 this triggers a Content-Length mismatch error instead." + ) request = Request(mockserver.url("/drop?abort=1", is_secure=self.is_secure)) async with self.get_dh() as download_handler: with pytest.raises(DownloadFailedError):