From dd4549e6f9a92f4844b552aad49ab4431f73514f Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 24 Jun 2026 22:48:28 +0200 Subject: [PATCH] Improve test coverage for downloader middlewares (#7655) * Improve test coverage for downloader middlewares * Improve coverage further --- .../downloadermiddlewares/httpcompression.py | 90 ++++++++++--------- tests/test_downloadermiddleware_cookies.py | 14 +++ ...st_downloadermiddleware_downloadtimeout.py | 6 ++ tests/test_downloadermiddleware_httpcache.py | 1 + ...st_downloadermiddleware_httpcompression.py | 18 +++- tests/test_downloadermiddleware_httpproxy.py | 6 ++ tests/test_downloadermiddleware_offsite.py | 18 ++++ tests/test_downloadermiddleware_redirect.py | 17 ++++ ...wnloadermiddleware_redirect_metarefresh.py | 7 ++ tests/test_downloadermiddleware_stats.py | 14 ++- 10 files changed, 146 insertions(+), 45 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 414c3d8a3..2b1721ced 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -6,7 +6,7 @@ from logging import getLogger from typing import TYPE_CHECKING, Any from scrapy import Request, Spider, signals -from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes from scrapy.utils._compression import ( @@ -70,6 +70,12 @@ class HttpCompressionMiddleware: crawler: Crawler | None = None, ): if not crawler: + warnings.warn( + "Instantiating HttpCompressionMiddleware without a 'crawler' " + "argument is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) self.stats = stats self._max_size = 1073741824 self._warn_size = 33554432 @@ -108,49 +114,47 @@ class HttpCompressionMiddleware: ) -> Request | Response: if request.method == "HEAD": return response - if isinstance(response, Response): - content_encoding = response.headers.getlist("Content-Encoding") - if content_encoding: - max_size = request.meta.get("download_maxsize", self._max_size) - warn_size = request.meta.get("download_warnsize", self._warn_size) - try: - decoded_body, content_encoding = self._handle_encoding( - response.body, content_encoding, max_size - ) - except _DecompressionMaxSizeExceeded as e: - raise IgnoreRequest( - f"Ignored response {response} because its body " - f"({len(response.body)} B compressed, " - f"{e.decompressed_size} B decompressed so far) exceeded " - f"DOWNLOAD_MAXSIZE ({max_size} B) during decompression." - ) from e - if len(response.body) < warn_size <= len(decoded_body): - logger.warning( - f"{response} body size after decompression " - f"({len(decoded_body)} B) is larger than the " - f"download warning size ({warn_size} B)." - ) - if content_encoding: - self._warn_unknown_encoding(response, content_encoding) - response.headers["Content-Encoding"] = content_encoding - if self.stats: - self.stats.inc_value( - "httpcompression/response_bytes", - len(decoded_body), - ) - self.stats.inc_value("httpcompression/response_count") - respcls = responsetypes.from_args( - headers=response.headers, url=response.url, body=decoded_body + content_encoding = response.headers.getlist("Content-Encoding") + if content_encoding: + max_size = request.meta.get("download_maxsize", self._max_size) + warn_size = request.meta.get("download_warnsize", self._warn_size) + try: + decoded_body, content_encoding = self._handle_encoding( + response.body, content_encoding, max_size ) - kwargs: dict[str, Any] = {"body": decoded_body} - if issubclass(respcls, TextResponse): - # force recalculating the encoding until we make sure the - # responsetypes guessing is reliable - kwargs["encoding"] = None - response = response.replace(cls=respcls, **kwargs) - if not content_encoding: - del response.headers["Content-Encoding"] - + except _DecompressionMaxSizeExceeded as e: + raise IgnoreRequest( + f"Ignored response {response} because its body " + f"({len(response.body)} B compressed, " + f"{e.decompressed_size} B decompressed so far) exceeded " + f"DOWNLOAD_MAXSIZE ({max_size} B) during decompression." + ) from e + if len(response.body) < warn_size <= len(decoded_body): + logger.warning( + f"{response} body size after decompression " + f"({len(decoded_body)} B) is larger than the " + f"download warning size ({warn_size} B)." + ) + if content_encoding: + self._warn_unknown_encoding(response, content_encoding) + response.headers["Content-Encoding"] = content_encoding + if self.stats: + self.stats.inc_value( + "httpcompression/response_bytes", + len(decoded_body), + ) + self.stats.inc_value("httpcompression/response_count") + respcls = responsetypes.from_args( + headers=response.headers, url=response.url, body=decoded_body + ) + kwargs: dict[str, Any] = {"body": decoded_body} + if issubclass(respcls, TextResponse): + # force recalculating the encoding until we make sure the + # responsetypes guessing is reliable + kwargs["encoding"] = None + response = response.replace(cls=respcls, **kwargs) + if not content_encoding: + del response.headers["Content-Encoding"] return response def _handle_encoding( diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 225562644..f79591020 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -131,6 +131,20 @@ class TestCookiesMiddleware: ), ) + def test_debug_no_cookies(self): + crawler = get_crawler(settings_dict={"COOKIES_DEBUG": True}) + mw = CookiesMiddleware.from_crawler(crawler) + with LogCapture( + "scrapy.downloadermiddlewares.cookies", + propagate=False, + level=logging.DEBUG, + ) as log: + req = Request("http://scrapytest.org/") + res = Response("http://scrapytest.org/") # no Set-Cookie header + mw.process_response(req, res) + mw.process_request(req) # no cookies to send either + log.check() # no log output since cl is empty in both cases + def test_setting_disabled_cookies_debug(self): crawler = get_crawler(settings_dict={"COOKIES_DEBUG": False}) mw = CookiesMiddleware.from_crawler(crawler) diff --git a/tests/test_downloadermiddleware_downloadtimeout.py b/tests/test_downloadermiddleware_downloadtimeout.py index c744d259c..e6b17960e 100644 --- a/tests/test_downloadermiddleware_downloadtimeout.py +++ b/tests/test_downloadermiddleware_downloadtimeout.py @@ -35,3 +35,9 @@ class TestDownloadTimeoutMiddleware: req.meta["download_timeout"] = 1 assert mw.process_request(req) is None assert req.meta.get("download_timeout") == 1 + + def test_zero_download_timeout(self): + req, spider, mw = self.get_request_spider_mw({"DOWNLOAD_TIMEOUT": 0}) + mw.spider_opened(spider) + assert mw.process_request(req) is None + assert req.meta.get("download_timeout") is None diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index e5d726764..6c86d7adf 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -135,6 +135,7 @@ class PolicyTestMixin: def test_dont_cache(self): with self._middleware() as mw: self.request.meta["dont_cache"] = True + assert mw.process_request(self.request) is None mw.process_response(self.request, self.response) assert mw.storage.retrieve_response(mw.crawler.spider, self.request) is None diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 30caa094f..5c4085657 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -11,7 +11,7 @@ from scrapy.downloadermiddlewares.httpcompression import ( ACCEPTED_ENCODINGS, HttpCompressionMiddleware, ) -from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Request, Response from scrapy.responsetypes import responsetypes from scrapy.spiders import Spider @@ -124,6 +124,22 @@ class TestHttpCompression: HttpCompressionMiddleware, ) + def test_no_crawler_constructor(self): + with pytest.warns(ScrapyDeprecationWarning, match="HttpCompressionMiddleware"): + mw = HttpCompressionMiddleware() + buf = BytesIO() + with GzipFile(fileobj=buf, mode="wb") as f: + f.write(b"hello") + body = buf.getvalue() + request = Request("http://scrapytest.org") + response = Response( + "http://scrapytest.org", + body=body, + headers={"Content-Encoding": "gzip"}, + ) + newresponse = mw.process_response(request, response) + assert newresponse.body == b"hello" + def test_process_request(self): request = Request("http://scrapytest.org") assert "Accept-Encoding" not in request.headers diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py index a2d421e39..7ed848764 100644 --- a/tests/test_downloadermiddleware_httpproxy.py +++ b/tests/test_downloadermiddleware_httpproxy.py @@ -373,6 +373,12 @@ class TestHttpProxyMiddleware: assert "proxy" not in request.meta assert b"Proxy-Authorization" not in request.headers + def test_proxy_unparseable_url_clears_meta(self): + middleware = HttpProxyMiddleware() + request = Request("http://example.com", meta={"proxy": "//"}) + assert middleware.process_request(request) is None + assert request.meta["proxy"] is None + def test_proxy_authentication_header_disabled_proxy(self): middleware = HttpProxyMiddleware() request = Request( diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index edfb15d10..c0b8dc4dd 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -213,3 +213,21 @@ def test_request_scheduled_invalid_domains(): request = Request(f"https://{letter}.example") with pytest.raises(IgnoreRequest): mw.request_scheduled(request, crawler.spider) + + +def test_repeated_offsite_domain(): + crawler = get_crawler(Spider) + crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(crawler.spider) + req1 = Request("http://other.org/1") + req2 = Request("http://other.org/2") + with pytest.raises(IgnoreRequest): + mw.process_request(req1) + assert "other.org" in mw.domains_seen + assert crawler.stats.get_value("offsite/domains") == 1 + assert crawler.stats.get_value("offsite/filtered") == 1 + with pytest.raises(IgnoreRequest): + mw.process_request(req2) + assert crawler.stats.get_value("offsite/domains") == 1 # not incremented again + assert crawler.stats.get_value("offsite/filtered") == 2 diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 42a25cd5b..1da7bbf3e 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock import pytest from scrapy.downloadermiddlewares.redirect import RedirectMiddleware +from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.spidermiddlewares.referer import ( POLICY_NO_REFERRER, @@ -265,6 +266,16 @@ class TestRedirectMiddleware(Base.Test): assert isinstance(req2, Request) assert req2.url == "http://www.example.com/redirected#frag" + def test_redirect_target_has_fragment(self): + url = "http://www.example.com/302#original" + url2 = "http://www.example.com/redirected#target" + req = Request(url) + rsp = Response(url, headers={"Location": url2}, status=302) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == "http://www.example.com/redirected#target" + def test_redirect_302_head(self): url = "http://www.example.com/302" url2 = "http://www.example.com/redirected2" @@ -458,3 +469,9 @@ def test_warning_subclass(caplog): assert ( "(if defined in your code base) to override the handle_referer() method" ) in caplog.text + + +def test_not_configured(): + crawler = get_crawler(DefaultSpider, {"REDIRECT_ENABLED": False}) + with pytest.raises(NotConfigured): + RedirectMiddleware.from_crawler(crawler) diff --git a/tests/test_downloadermiddleware_redirect_metarefresh.py b/tests/test_downloadermiddleware_redirect_metarefresh.py index 416fbc2aa..d849cc8fb 100644 --- a/tests/test_downloadermiddleware_redirect_metarefresh.py +++ b/tests/test_downloadermiddleware_redirect_metarefresh.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock import pytest from scrapy.downloadermiddlewares.redirect import MetaRefreshMiddleware +from scrapy.exceptions import NotConfigured from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider from scrapy.utils.misc import build_from_crawler @@ -157,3 +158,9 @@ def test_warning_meta_refresh_middleware(caplog): "replace scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware " "with a subclass that overrides the handle_referer() method" ) in caplog.text + + +def test_not_configured(): + crawler = get_crawler(Spider, {"METAREFRESH_ENABLED": False}) + with pytest.raises(NotConfigured): + MetaRefreshMiddleware.from_crawler(crawler) diff --git a/tests/test_downloadermiddleware_stats.py b/tests/test_downloadermiddleware_stats.py index cf7b614c4..5609360a7 100644 --- a/tests/test_downloadermiddleware_stats.py +++ b/tests/test_downloadermiddleware_stats.py @@ -1,4 +1,7 @@ -from scrapy.downloadermiddlewares.stats import DownloaderStats +import pytest + +from scrapy.downloadermiddlewares.stats import DownloaderStats, get_header_size +from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.spiders import Spider from scrapy.utils.test import get_crawler @@ -39,5 +42,14 @@ class TestDownloaderStats: 1, ) + def test_from_crawler_not_configured(self): + crawler = get_crawler(Spider, {"DOWNLOADER_STATS": False}) + with pytest.raises(NotConfigured): + DownloaderStats.from_crawler(crawler) + def teardown_method(self): self.crawler.stats.close_spider() + + +def test_get_header_size_non_list_value(): + assert get_header_size({"Content-Type": "text/html"}) == 0