From 9559cbee1e7344d9746b2e2164d10a9d6575f9d1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 29 Jun 2026 14:21:20 +0200 Subject: [PATCH 1/4] Solve timing issues with HTTP cache tests? (#7692) --- tests/test_downloadermiddleware_httpcache.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index 6c86d7adf..9d5d6874e 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -6,6 +6,7 @@ import tempfile import time from contextlib import contextmanager from typing import TYPE_CHECKING, Any +from unittest import mock import pytest @@ -47,7 +48,6 @@ class TestBase: settings = { "HTTPCACHE_ENABLED": True, "HTTPCACHE_DIR": self.tmpdir, - "HTTPCACHE_EXPIRATION_SECS": 1, "HTTPCACHE_IGNORE_HTTP_CODES": [], "HTTPCACHE_POLICY": self.policy_class, "HTTPCACHE_STORAGE": self.storage_class, @@ -94,7 +94,7 @@ class StorageTestMixin: """Mixin containing storage-specific test methods.""" def test_storage(self): - with self._storage() as (storage, crawler): + with self._storage(HTTPCACHE_EXPIRATION_SECS=1) as (storage, crawler): request2 = self.request.copy() assert storage.retrieve_response(crawler.spider, request2) is None @@ -103,15 +103,17 @@ class StorageTestMixin: assert isinstance(response2, HtmlResponse) # content-type header self.assertEqualResponse(self.response, response2) - time.sleep(2) # wait for cache to expire - assert storage.retrieve_response(crawler.spider, request2) is None + expired = time.time() + storage.expiration_secs + 1 + with mock.patch("scrapy.extensions.httpcache.time", return_value=expired): + assert storage.retrieve_response(crawler.spider, request2) is None def test_storage_never_expire(self): with self._storage(HTTPCACHE_EXPIRATION_SECS=0) as (storage, crawler): assert storage.retrieve_response(crawler.spider, self.request) is None storage.store_response(crawler.spider, self.request, self.response) - time.sleep(0.5) # give the chance to expire - assert storage.retrieve_response(crawler.spider, self.request) + future = time.time() + 10**6 + with mock.patch("scrapy.extensions.httpcache.time", return_value=future): + assert storage.retrieve_response(crawler.spider, self.request) def test_storage_no_content_type_header(self): """Test that the response body is used to get the right response class From 4b2b56f3848111d55151ccfefddfc6b3d84d1302 Mon Sep 17 00:00:00 2001 From: Javier <114426455+javidiazz@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:34:12 +0200 Subject: [PATCH 2/4] Update on Request objects (#7286) --- docs/topics/request-response.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 5e70a4f98..f2fcfb0b5 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -63,7 +63,7 @@ Request objects .. invisible-code-block: python - from scrapy.http import Request + from scrapy import Request 1. Using a dict: From 6591cb756c16bdb0ce85425071fea84a7cddaf53 Mon Sep 17 00:00:00 2001 From: tanishqtayade Date: Mon, 29 Jun 2026 22:07:06 +0530 Subject: [PATCH 3/4] Fix to LocalCache with limit=0 still stores items rather than disabling the cache (#7663) * Fix cell-var-from-loop in _send_catch_log_deferred Replace lambda capturing receiver by reference with a default argument to capture it by value, fixing a potential bug where all deferred callbacks could reference the last receiver in the loop instead of their respective receivers. Removes the pylint disable comment and TODO that were suppressing this issue. * chore: trigger CI rerun for mypy network error * Fix mypy error: pass receiver via addBoth args instead of lambda default * style: apply pre-commit ruff formatting * fix: LocalCache with limit=0 incorrectly stores items When limit=0 is passed to LocalCache (e.g. when DNSCACHE_ENABLED=False), the condition 'if self.limit' evaluates to False due to Python's truthiness rules, causing items to be stored despite the cache being disabled. This leads to an unbounded memory leak during long crawls when DNS caching is explicitly disabled. Fix changes the condition to 'if self.limit is not None' and adds an early return when limit=0 to correctly handle the disabled cache case. * test: add resolver-level regression tests for DNSCACHE_ENABLED=False Add two regression tests that verify DNS results are not stored in dnscache when DNSCACHE_ENABLED=False (cache_size=0): - test_caching_hostname_resolver_dnscache_disabled_rejects_storage: verifies _CachingResolutionReceiver does not write to dnscache when CachingHostnameResolver is initialized with cache_size=0 - test_caching_threaded_resolver_dnscache_disabled_rejects_storage: verifies dnscache rejects storage at the LocalCache level when limit=0 * test: drop misleading threaded resolver test it was writing directly to dnscache, not actually going through CachingThreadedResolver at all. the threaded resolver already has its own if dnscache.limit: guard so the bug doesnt affect it anyway. keeping only the hostname resolver test which covers the actual bug path through _CachingResolutionReceiver * test: clean up comments in resolver test * test: remove unnecessary comment --- scrapy/utils/datatypes.py | 4 +++- tests/test_resolver.py | 18 ++++++++++++++++++ tests/test_utils_datatypes.py | 10 ++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index 4e65c062e..dd0e062d0 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -152,7 +152,9 @@ class LocalCache(OrderedDict[_KT, _VT]): self.limit: int | None = limit def __setitem__(self, key: _KT, value: _VT) -> None: - if self.limit: + if self.limit is not None: + if self.limit == 0: + return while len(self) >= self.limit: self.popitem(last=False) super().__setitem__(key, value) diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 83fc8693b..7cca45ed1 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -53,3 +53,21 @@ def test_caching_hostname_resolver_no_addresses_not_cached(): resolver.resolveHostName(Mock(), "example.com") assert "example.com" not in dnscache + + +def test_caching_hostname_resolver_dnscache_disabled_rejects_storage(): + + def fake_resolve(receiver, *_): + receiver.resolutionBegan(Mock()) + receiver.addressResolved(Mock()) + receiver.resolutionComplete() + return receiver + + reactor = Mock() + reactor.nameResolver.resolveHostName.side_effect = fake_resolve + + resolver = CachingHostnameResolver(reactor, cache_size=0) + resolver.resolveHostName(Mock(), "example.com") + + assert "example.com" not in dnscache + assert len(dnscache) == 0 diff --git a/tests/test_utils_datatypes.py b/tests/test_utils_datatypes.py index ba6b82503..fe1f60c7d 100644 --- a/tests/test_utils_datatypes.py +++ b/tests/test_utils_datatypes.py @@ -309,6 +309,16 @@ class TestLocalCache: assert str(x) in cache assert cache[str(x)] == x + def test_cache_with_zero_limit(self): + cache = LocalCache(limit=0) + cache["a"] = 1 + cache["b"] = 2 + cache["c"] = 3 + assert len(cache) == 0 + assert "a" not in cache + assert "b" not in cache + assert "c" not in cache + class TestLocalWeakReferencedCache: def test_cache_with_limit(self): From 52147017b43baff1f9b4668e46ab11341aa8f400 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 29 Jun 2026 21:49:09 +0500 Subject: [PATCH 4/4] Cleanup cookie handling in request_to_curl() (#7684) * Adjust CookiesT. * Drop list of plain cookie dicts support from request_to_curl(). * Extract _decode_cookie(). * Unify logging. * Extract _to_verbose_cookies(). * Sync and type hint _cookie_to_set_cookie_value() in tests. * Add tests for bytes in Request.cookies. * Type hint assertCookieValEqual(). --- scrapy/downloadermiddlewares/cookies.py | 32 ++------- scrapy/http/request/__init__.py | 2 +- scrapy/utils/request.py | 73 +++++++++++++------ tests/test_downloadermiddleware_cookies.py | 81 +++++++++++++--------- tests/test_utils_request.py | 44 +++--------- 5 files changed, 116 insertions(+), 116 deletions(-) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index cd8c2abca..34d071fd5 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -12,6 +12,7 @@ from scrapy.http.cookies import CookieJar from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_unicode +from scrapy.utils.request import _decode_cookie, _to_verbose_cookies if TYPE_CHECKING: from collections.abc import Iterable, Sequence @@ -134,29 +135,10 @@ class CookiesMiddleware: Given a dict consisting of cookie components, return its string representation. Decode from bytes if necessary. """ - decoded = {} + decoded = _decode_cookie(cookie, request) + if decoded is None: + return None flags = set() - for key in ("name", "value", "path", "domain"): - value = cookie.get(key) - if value is None: - if key in {"name", "value"}: - msg = f"Invalid cookie found in request {request}: {cookie} ('{key}' is missing)" - logger.warning(msg) - return None - continue - if isinstance(value, (bool, float, int, str)): - decoded[key] = str(value) - else: - assert isinstance(value, bytes) - try: - decoded[key] = value.decode("utf8") - except UnicodeDecodeError: - logger.warning( - "Non UTF-8 encoded cookie found in request %s: %s", - request, - cookie, - ) - decoded[key] = value.decode("latin1", errors="replace") for flag in ("secure",): value = cookie.get(flag, _UNSET) if value is _UNSET or not value: @@ -177,11 +159,7 @@ class CookiesMiddleware: """ if not request.cookies: return () - cookies: Iterable[VerboseCookie] - if isinstance(request.cookies, dict): - cookies = tuple({"name": k, "value": v} for k, v in request.cookies.items()) - else: - cookies = request.cookies + cookies: Iterable[VerboseCookie] = _to_verbose_cookies(request.cookies) for cookie in cookies: cookie.setdefault("secure", urlparse_cached(request).scheme == "https") formatted = filter(None, (self._format_cookie(c, request) for c in cookies)) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 7db648a45..73c2e7dd4 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -51,7 +51,7 @@ class VerboseCookie(TypedDict): secure: NotRequired[bool] -CookiesT: TypeAlias = dict[str, str] | list[VerboseCookie] +CookiesT: TypeAlias = dict[str | bytes, str | bytes] | list[VerboseCookie] RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py index 4a85526c0..b2fd9a834 100644 --- a/scrapy/utils/request.py +++ b/scrapy/utils/request.py @@ -7,6 +7,7 @@ from __future__ import annotations import hashlib import json +import logging from typing import TYPE_CHECKING, Any, Protocol from urllib.parse import urlunparse from weakref import WeakKeyDictionary @@ -25,6 +26,9 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy.crawler import Crawler + from scrapy.http.request import CookiesT, VerboseCookie + +logger = logging.getLogger(__name__) _fingerprint_cache: WeakKeyDictionary[ @@ -179,17 +183,52 @@ def _get_method(obj: Any, name: Any) -> Any: raise ValueError(f"Method {name!r} not found in: {obj}") from None -def _cookie_value_to_unicode(value: str | bytes | float) -> str: - if isinstance(value, bytes): - return value.decode() - return str(value) +def _to_verbose_cookies(cookies: CookiesT) -> list[VerboseCookie]: + """Return a list of verbose cookies from ``request.cookies``. + + The list of dicts form is returned as is, the dict one is converted first. + """ + if isinstance(cookies, dict): + return [{"name": k, "value": v} for k, v in cookies.items()] + return cookies + + +def _decode_cookie(cookie: VerboseCookie, request: Request) -> dict[str, str] | None: + """Return a dict with non-flag verbose cookie values converted to strings. + + ``name``, ``value``, ``path``, ``domain`` are included, ``secure`` isn't. + """ + + decoded = {} + for key in ("name", "value", "path", "domain"): + value = cookie.get(key) + if value is None: + if key in {"name", "value"}: + logger.warning( + f"Invalid cookie found in request {request}:" + f" {cookie} ('{key}' is missing)" + ) + return None + continue + if isinstance(value, (bool, float, int, str)): + decoded[key] = str(value) + else: + assert isinstance(value, bytes) + try: + decoded[key] = value.decode("utf8") + except UnicodeDecodeError: + logger.warning( + f"Non UTF-8 encoded cookie found in request {request}: {cookie}", + ) + decoded[key] = value.decode("latin1", errors="replace") + return decoded def request_to_curl(request: Request) -> str: """ Converts a :class:`~scrapy.Request` object to a curl command. - :param :class:`~scrapy.Request`: Request object to be converted + :param request: Request object to be converted :return: string containing the curl command """ method = request.method @@ -201,22 +240,14 @@ def request_to_curl(request: Request) -> str: ) url = request.url - cookies = "" - if request.cookies: - if isinstance(request.cookies, dict): - cookie = "; ".join( - f"{_cookie_value_to_unicode(k)}={_cookie_value_to_unicode(v)}" - for k, v in request.cookies.items() - ) - cookies = f"--cookie '{cookie}'" - elif isinstance(request.cookies, list): - cookie = "; ".join( - f"{_cookie_value_to_unicode(c['name'])}={_cookie_value_to_unicode(c['value'])}" - if "name" in c and "value" in c - else f"{_cookie_value_to_unicode(next(iter(c.keys())))}={_cookie_value_to_unicode(next(iter(c.values())))}" - for c in request.cookies - ) - cookies = f"--cookie '{cookie}'" + + cookie_list: list[VerboseCookie] = _to_verbose_cookies(request.cookies) + pairs = [ + f"{decoded['name']}={decoded['value']}" + for c in cookie_list + if (decoded := _decode_cookie(c, request)) is not None + ] + cookies = f"--cookie '{'; '.join(pairs)}'" if pairs else "" curl_cmd = f"curl -X {method} {url} {data} {headers} {cookies}".strip() return " ".join(curl_cmd.split()) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index f79591020..b4419dc67 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -1,4 +1,5 @@ import logging +from collections.abc import Iterable import pytest from testfixtures import LogCapture @@ -8,30 +9,34 @@ from scrapy.downloadermiddlewares.defaultheaders import DefaultHeadersMiddleware from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response +from scrapy.http.request import CookiesT, VerboseCookie from scrapy.utils.python import to_bytes +from scrapy.utils.request import _to_verbose_cookies from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler UNSET = object() -def _cookie_to_set_cookie_value(cookie): +def _cookie_to_set_cookie_value(cookie: VerboseCookie) -> str | None: """Given a cookie defined as a dictionary with name and value keys, and optional path and domain keys, return the equivalent string that can be associated to a ``Set-Cookie`` header.""" decoded = {} for key in ("name", "value", "path", "domain"): - if cookie.get(key) is None: - if key in ("name", "value"): + value = cookie.get(key) + if value is None: + if key in {"name", "value"}: return None continue - if isinstance(cookie[key], (bool, float, int, str)): - decoded[key] = str(cookie[key]) + if isinstance(value, (bool, float, int, str)): + decoded[key] = str(value) else: + assert isinstance(value, bytes) try: - decoded[key] = cookie[key].decode("utf8") + decoded[key] = value.decode("utf8") except UnicodeDecodeError: - decoded[key] = cookie[key].decode("latin1", errors="replace") + decoded[key] = value.decode("latin1", errors="replace") cookie_str = f"{decoded.pop('name')}={decoded.pop('value')}" for key, value in decoded.items(): # path, domain @@ -39,24 +44,30 @@ def _cookie_to_set_cookie_value(cookie): return cookie_str -def _cookies_to_set_cookie_list(cookies): +def _cookies_to_set_cookie_list(cookies: CookiesT) -> Iterable[str]: """Given a group of cookie defined either as a dictionary or as a list of dictionaries (i.e. in a format supported by the cookies parameter of Request), return the equivalent list of strings that can be associated to a ``Set-Cookie`` header.""" if not cookies: return [] - if isinstance(cookies, dict): - cookies = ({"name": k, "value": v} for k, v in cookies.items()) - return filter(None, (_cookie_to_set_cookie_value(cookie) for cookie in cookies)) + return filter( + None, + ( + _cookie_to_set_cookie_value(cookie) + for cookie in _to_verbose_cookies(cookies) + ), + ) class TestCookiesMiddleware: - def assertCookieValEqual(self, first, second, msg=None): - def split_cookies(cookies): + @staticmethod + def assertCookieValEqual(first: bytes | str | None, second: bytes | str) -> None: + def split_cookies(cookies: bytes | str) -> list[bytes]: return sorted([s.strip() for s in to_bytes(cookies).split(b";")]) - assert split_cookies(first) == split_cookies(second), msg + assert first is not None + assert split_cookies(first) == split_cookies(second) def setup_method(self): crawler = get_crawler(DefaultSpider) @@ -372,21 +383,25 @@ class TestCookiesMiddleware: assert self.mw.process_request(req3) is None self.assertCookieValEqual(req3.headers["Cookie"], "a=new; c=d; e=f") - def test_request_cookies_encoding(self): - # 1) UTF8-encoded bytes - req1 = Request("http://example.org", cookies={"a": "á".encode()}) - assert self.mw.process_request(req1) is None - self.assertCookieValEqual(req1.headers["Cookie"], b"a=\xc3\xa1") - - # 2) Non UTF8-encoded bytes - req2 = Request("http://example.org", cookies={"a": "á".encode("latin1")}) - assert self.mw.process_request(req2) is None - self.assertCookieValEqual(req2.headers["Cookie"], b"a=\xc3\xa1") - - # 3) String - req3 = Request("http://example.org", cookies={"a": "á"}) - assert self.mw.process_request(req3) is None - self.assertCookieValEqual(req3.headers["Cookie"], b"a=\xc3\xa1") + @pytest.mark.parametrize( + "cookies", + [ + # UTF8-encoded bytes + {"a": "á".encode()}, + # non UTF8-encoded bytes + {"a": "á".encode("latin1")}, + # string + {"a": "á"}, + # key as bytes + {b"a": "á"}, + # key and value as bytes + {b"a": "á".encode()}, + ], + ) + def test_request_cookies_encoding(self, cookies: CookiesT) -> None: + req = Request("http://example.org", cookies=cookies) + assert self.mw.process_request(req) is None + self.assertCookieValEqual(req.headers["Cookie"], b"a=\xc3\xa1") @pytest.mark.xfail(reason="Cookie header is not currently being processed") def test_request_headers_cookie_encoding(self): @@ -410,7 +425,7 @@ class TestCookiesMiddleware: Invalid cookies are logged as warnings and discarded """ with LogCapture( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", propagate=False, level=logging.INFO, ) as lc: @@ -425,19 +440,19 @@ class TestCookiesMiddleware: assert self.mw.process_request(req3) is None lc.check( ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'value': 'bar', 'secure': False} ('name' is missing)", ), ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'name': 'foo', 'secure': False} ('value' is missing)", ), ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'name': 'foo', 'value': None, 'secure': False} ('value' is missing)", diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 63926e9d2..1642a932b 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -354,16 +354,18 @@ class TestCustomRequestFingerprinter: class TestRequestToCurl: - def _test_request(self, request_object, expected_curl_command): + def _test_request( + self, request_object: Request, expected_curl_command: str + ) -> None: curl_command = request_to_curl(request_object) assert curl_command == expected_curl_command - def test_get(self): + def test_get(self) -> None: request_object = Request("https://www.example.com") expected_curl_command = "curl -X GET https://www.example.com" self._test_request(request_object, expected_curl_command) - def test_post(self): + def test_post(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -374,7 +376,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_headers(self): + def test_headers(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -388,7 +390,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_dict(self): + def test_cookies_dict(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -401,7 +403,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_dict_bytes(self): + def test_cookies_dict_bytes(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -414,20 +416,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_list(self): - request_object = Request( - "https://www.httpbin.org/post", - method="POST", - cookies=[{"foo": "bar"}], - body=json.dumps({"foo": "bar"}), - ) - expected_curl_command = ( - "curl -X POST https://www.httpbin.org/post" - " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" - ) - self._test_request(request_object, expected_curl_command) - - def test_cookies_list_verbose(self): + def test_cookies_list_verbose(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -448,7 +437,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_list_verbose_non_string_value(self): + def test_cookies_list_verbose_non_string_value(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -468,16 +457,3 @@ class TestRequestToCurl: " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=1'" ) self._test_request(request_object, expected_curl_command) - - def test_cookies_list_bytes_nonstandard_key(self): - request_object = Request( - "https://www.httpbin.org/post", - method="POST", - cookies=[{b"foo": b"bar"}], - body=json.dumps({"foo": "bar"}), - ) - expected_curl_command = ( - "curl -X POST https://www.httpbin.org/post" - " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" - ) - self._test_request(request_object, expected_curl_command)