mirror of https://github.com/scrapy/scrapy.git
Clarify dataloss/http2 tests. (#7466)
This commit is contained in:
parent
528745b059
commit
988afe1454
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue