Improve test coverage for downloader middlewares (#7655)

* Improve test coverage for downloader middlewares

* Improve coverage further

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(

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)

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

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
 

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

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(

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

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)

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)

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