From 75f05d4e80b82705f54b2fe6b2948c463c7eb36b Mon Sep 17 00:00:00 2001 From: "Shashank S. Khasare" Date: Mon, 22 Jun 2026 12:20:11 +0530 Subject: [PATCH 01/18] Add test coverage for scrapy.utils.decorators (#7645) --- tests/test_utils_decorators.py | 107 +++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/test_utils_decorators.py diff --git a/tests/test_utils_decorators.py b/tests/test_utils_decorators.py new file mode 100644 index 000000000..9743e1a50 --- /dev/null +++ b/tests/test_utils_decorators.py @@ -0,0 +1,107 @@ +from __future__ import annotations + +import warnings + +import pytest +from twisted.internet.defer import Deferred + +from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.decorators import _warn_spider_arg, deprecated, inthread +from scrapy.utils.defer import maybe_deferred_to_future +from tests.utils.decorators import coroutine_test + + +class TestDeprecated: + def test_warns_and_still_calls(self): + @deprecated() + def add(a, b): + return a + b + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Call to deprecated function add\." + ): + result = add(2, 3) + + assert result == 5 + + def test_use_instead_in_message(self): + @deprecated(use_instead="other_function") + def old(): + return None + + with pytest.warns( + ScrapyDeprecationWarning, + match=r"Call to deprecated function old\. Use other_function instead\.", + ): + old() + + def test_applied_without_parentheses(self): + @deprecated + def square(x): + return x * x + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Call to deprecated function square\." + ) as record: + result = square(4) + + assert result == 16 + # No "Use ... instead." part when applied directly to the function. + assert "instead" not in str(record[0].message) + + +class TestInthread: + @coroutine_test + async def test_returns_deferred_with_result(self): + @inthread + def multiply(a, b): + return a * b + + deferred = multiply(6, 7) + assert isinstance(deferred, Deferred) + assert await maybe_deferred_to_future(deferred) == 42 + + +class TestWarnSpiderArg: + def test_sync_warns_with_spider_arg(self): + @_warn_spider_arg + def parse(response, spider=None): + return response + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Passing a 'spider' argument" + ): + assert parse("response", spider="spider") == "response" + + def test_sync_no_warning_without_spider_arg(self): + @_warn_spider_arg + def parse(response, spider=None): + return response + + with warnings.catch_warnings(): + warnings.simplefilter("error", category=ScrapyDeprecationWarning) + assert parse("response") == "response" + + @coroutine_test + async def test_async_warns_with_spider_arg(self): + @_warn_spider_arg + async def parse(response, spider=None): + return response + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Passing a 'spider' argument" + ): + assert await parse("response", spider="spider") == "response" + + @coroutine_test + async def test_asyncgen_warns_with_spider_arg(self): + @_warn_spider_arg + async def parse(response, spider=None): + yield response + + with pytest.warns( + ScrapyDeprecationWarning, match=r"Passing a 'spider' argument" + ): + results = [item async for item in parse("response", spider="spider")] + + assert results == ["response"] From b6596de317acb16c1a0de0a77496640f7542364b Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 22 Jun 2026 17:10:10 +0200 Subject: [PATCH 02/18] Do not ignore CrawlerProcess settings (#7647) * Do not ignore CrawlerProcess settings * Update test_crawlerrunner_accepts_crawler --- scrapy/crawler.py | 8 ++++++-- tests/test_crawl.py | 4 ++-- tests/test_crawler.py | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 8e3ae7879..9828fe6f5 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -363,9 +363,12 @@ class CrawlerRunnerBase(ABC): """ Return a :class:`~scrapy.crawler.Crawler` object. - * If ``crawler_or_spidercls`` is a Crawler, it is returned as-is. + * If ``crawler_or_spidercls`` is a Crawler, the runner's settings are + merged into it as defaults: for each setting, the runner's value + is applied only if the Crawler does not already have that setting at + an equal or higher priority. The Crawler is then returned. * If ``crawler_or_spidercls`` is a Spider subclass, a new Crawler - is constructed for it. + is constructed for it using this runner's settings. * If ``crawler_or_spidercls`` is a string, this function finds a spider with this name in a Scrapy project (using spider loader), then creates a Crawler instance for it. @@ -376,6 +379,7 @@ class CrawlerRunnerBase(ABC): "it must be a spider class (or a Crawler object)" ) if isinstance(crawler_or_spidercls, Crawler): + crawler_or_spidercls.settings.update(self.settings) return crawler_or_spidercls return self._create_crawler(crawler_or_spidercls) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 66b15bfdd..be6c80429 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -14,7 +14,7 @@ from twisted.internet.ssl import Certificate from twisted.python.failure import Failure from scrapy import Spider, signals -from scrapy.crawler import AsyncCrawlerRunner, CrawlerRunner +from scrapy.crawler import AsyncCrawlerRunner, Crawler, CrawlerRunner from scrapy.exceptions import CloseSpider, ScrapyDeprecationWarning, StopDownload from scrapy.http import Request from scrapy.http.response import Response @@ -432,7 +432,7 @@ with multiples lines async def test_crawlerrunner_accepts_crawler( self, caplog: pytest.LogCaptureFixture, mockserver: MockServer ) -> None: - crawler = get_crawler(SimpleSpider) + crawler = Crawler(SimpleSpider, get_reactor_settings()) runner = CrawlerRunner() with caplog.at_level(logging.DEBUG): await maybe_deferred_to_future( diff --git a/tests/test_crawler.py b/tests/test_crawler.py index c646aea4b..0cddfd0ed 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -651,6 +651,42 @@ class TestAsyncCrawlerProcess(TestBaseCrawler): self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") +@pytest.mark.parametrize("runner_cls", [AsyncCrawlerRunner, CrawlerRunner]) +def test_runner_settings_applied_to_crawler_instance( + runner_cls: type[CrawlerRunnerBase], +) -> None: + runner = runner_cls({"FOO": "runner"}) + crawler = Crawler(DefaultSpider) + result = runner.create_crawler(crawler) + assert result is crawler + assert result.settings["FOO"] == "runner" + + +@pytest.mark.parametrize("runner_cls", [AsyncCrawlerRunner, CrawlerRunner]) +def test_spider_custom_settings_override_runner( + runner_cls: type[CrawlerRunnerBase], +) -> None: + class MySpider(DefaultSpider): + custom_settings = {"FOO": "spider"} + + runner = runner_cls({"FOO": "runner"}) + crawler = Crawler(MySpider) + runner.create_crawler(crawler) + assert crawler.settings["FOO"] == "spider" + + +def test_create_crawler_instance_consistent_with_spider_class() -> None: + runner = AsyncCrawlerRunner({"FOO": "runner"}) + + crawler_from_class = runner.create_crawler(DefaultSpider) + + pre_built = Crawler(DefaultSpider) + runner.create_crawler(pre_built) + + assert crawler_from_class.settings["FOO"] == "runner" + assert pre_built.settings["FOO"] == "runner" + + class ExceptionSpider(scrapy.Spider): name = "exception" From 7499d17e281cd214a70538ea92c05b14d8459dde Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 22 Jun 2026 17:10:38 +0200 Subject: [PATCH 03/18] Improve test coverage for responsetypes.py (#7646) * Improve test coverage for responsetypes.py * Solve typing issues --- scrapy/responsetypes.py | 7 +------ tests/test_responsetypes.py | 3 +++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/scrapy/responsetypes.py b/scrapy/responsetypes.py index cd62f02af..29e9b6bb7 100644 --- a/scrapy/responsetypes.py +++ b/scrapy/responsetypes.py @@ -40,18 +40,13 @@ class ResponseTypes: self.classes: dict[str, type[Response]] = {} self.mimetypes: MimeTypes = MimeTypes() mimedata = get_data("scrapy", "mime.types") - if not mimedata: - raise ValueError( - "The mime.types file is not found in the Scrapy installation" - ) + assert mimedata is not None self.mimetypes.readfp(StringIO(mimedata.decode("utf8"))) for mimetype, cls in self.CLASSES.items(): self.classes[mimetype] = load_object(cls) def from_mimetype(self, mimetype: str) -> type[Response]: """Return the most appropriate Response class for the given mimetype""" - if mimetype is None: - return Response if mimetype in self.classes: return self.classes[mimetype] basetype = f"{mimetype.split('/', maxsplit=1)[0]}/*" diff --git a/tests/test_responsetypes.py b/tests/test_responsetypes.py index 5b04c7436..42ab29267 100644 --- a/tests/test_responsetypes.py +++ b/tests/test_responsetypes.py @@ -40,6 +40,9 @@ class TestResponseTypes: retcls = responsetypes.from_content_disposition(source) assert retcls is cls, f"{source} ==> {retcls} != {cls}" + def test_from_content_disposition_no_filename(self): + assert responsetypes.from_content_disposition(b"attachment") is Response + def test_from_content_type(self): mappings = [ ("text/html; charset=UTF-8", HtmlResponse), From f605defefc5cb8e7a969fba7da66c2087868c104 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 22 Jun 2026 17:12:16 +0200 Subject: [PATCH 04/18] Document scrapy-lint, remove start_url check (#7627) * Document scrapy-lint, remove start_url check * Remove the offsite URL check in favor of scrapy-lint --- .pre-commit-config.yaml | 2 +- docs/conf.py | 1 + docs/requirements.in | 2 +- docs/requirements.txt | 2 +- docs/topics/practices.rst | 8 ++++++++ scrapy/downloadermiddlewares/offsite.py | 18 +----------------- scrapy/spiders/__init__.py | 6 ------ tests/test_downloadermiddleware_offsite.py | 10 ++-------- tests/test_spider_crawl.py | 15 --------------- tox.ini | 2 +- 10 files changed, 16 insertions(+), 50 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6b9ef3c04..754bd6f9c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,6 +27,6 @@ repos: hooks: - id: sphinx-lint - repo: https://github.com/scrapy/sphinx-scrapy - rev: 0.8.6 + rev: 0.8.8 hooks: - id: sphinx-scrapy diff --git a/docs/conf.py b/docs/conf.py index 99d5df7da..b950c4ee2 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -158,6 +158,7 @@ scrapy_intersphinx_enable = [ "itemloaders", "parsel", "pytest", + "scrapy-lint", "sphinx", "tox", "twisted", diff --git a/docs/requirements.in b/docs/requirements.in index 140791641..a1f3a7468 100644 --- a/docs/requirements.in +++ b/docs/requirements.in @@ -5,4 +5,4 @@ sphinx sphinx-notfound-page sphinx-rtd-theme sphinx-rtd-dark-mode -sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.6 +sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.8 diff --git a/docs/requirements.txt b/docs/requirements.txt index 9c93dacd0..a5cbad302 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -153,7 +153,7 @@ sphinx-rtd-theme==3.1.0 # via # -r docs/requirements.in # sphinx-rtd-dark-mode -sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@b1d55db4d16a5425fc68576d63519bbfe26dd9c0 +sphinx-scrapy @ git+https://github.com/scrapy/sphinx-scrapy.git@c0b2ac815afc3cb8857d575cecb5d55c05e6b737 # via -r docs/requirements.in sphinx-sitemap==2.9.0 # via sphinx-scrapy diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index b3c58d6d3..8a04f7ada 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -440,6 +440,14 @@ Here are some tips to keep in mind when dealing with these kinds of sites: If you are still unable to prevent your bot getting banned, consider contacting `commercial support`_. +.. _static-analysis: + +Static analysis +=============== + +Consider using :doc:`scrapy-lint `, a linter for Scrapy +projects that detects common mistakes and anti-patterns. + .. _Tor project: https://www.torproject.org/ .. _commercial support: https://www.scrapy.org/companies .. _ProxyMesh: https://proxymesh.com/ diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index 10f19bacc..e03dc040e 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -2,7 +2,6 @@ from __future__ import annotations import logging import re -import warnings from typing import TYPE_CHECKING from scrapy import Request, Spider, signals @@ -75,25 +74,10 @@ class OffsiteMiddleware: allowed_domains = getattr(spider, "allowed_domains", None) if not allowed_domains: return re.compile("") # allow all by default - url_pattern = re.compile(r"^https?://.*$") - port_pattern = re.compile(r":\d+$") domains = [] for domain in allowed_domains: if domain is None: continue - if url_pattern.match(domain): - message = ( - "allowed_domains accepts only domains, not URLs. " - f"Ignoring URL entry {domain} in allowed_domains." - ) - warnings.warn(message, stacklevel=2) - elif port_pattern.search(domain): - message = ( - "allowed_domains accepts only domains without ports. " - f"Ignoring entry {domain} in allowed_domains." - ) - warnings.warn(message, stacklevel=2) - else: - domains.append(re.escape(domain)) + domains.append(re.escape(domain)) regex = rf"^(.*\.)?({'|'.join(domains)})$" return re.compile(regex) diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 6e8c67adb..de527d04f 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -125,12 +125,6 @@ class Spider(object_ref): .. seealso:: :ref:`start-requests` """ - if not self.start_urls and hasattr(self, "start_url"): - raise AttributeError( - "Crawling could not start: 'start_urls' not found " - "or empty (but found 'start_url' attribute instead, " - "did you miss an 's'?)" - ) for url in self.start_urls: yield Request(url, dont_filter=True) diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index dc0a31a76..edfb15d10 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -1,5 +1,3 @@ -import warnings - import pytest from scrapy import Request, Spider @@ -120,9 +118,7 @@ def test_process_request_invalid_domains(): allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] crawler.spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) mw = OffsiteMiddleware.from_crawler(crawler) - with warnings.catch_warnings(): - warnings.simplefilter("ignore", UserWarning) - mw.spider_opened(crawler.spider) + mw.spider_opened(crawler.spider) request = Request("https://a.example") assert mw.process_request(request) is None for letter in ("b", "c"): @@ -210,9 +206,7 @@ def test_request_scheduled_invalid_domains(): allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] crawler.spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) mw = OffsiteMiddleware.from_crawler(crawler) - with warnings.catch_warnings(): - warnings.simplefilter("ignore", UserWarning) - mw.spider_opened(crawler.spider) + mw.spider_opened(crawler.spider) request = Request("https://a.example") assert mw.request_scheduled(request, crawler.spider) is None for letter in ("b", "c"): diff --git a/tests/test_spider_crawl.py b/tests/test_spider_crawl.py index 2eeae110b..63010e195 100644 --- a/tests/test_spider_crawl.py +++ b/tests/test_spider_crawl.py @@ -2,10 +2,8 @@ from __future__ import annotations import re import warnings -from logging import ERROR import pytest -from testfixtures import LogCapture from w3lib.url import safe_url_string from scrapy.exceptions import ScrapyDeprecationWarning @@ -14,7 +12,6 @@ from scrapy.linkextractors import LinkExtractor from scrapy.spiders import CrawlSpider, Rule, Spider from scrapy.utils.test import get_crawler from tests.test_spider import TestSpider -from tests.utils.decorators import inline_callbacks_test class TestCrawlSpider(TestSpider): @@ -247,18 +244,6 @@ class TestCrawlSpider(TestSpider): assert hasattr(spider, "_follow_links") assert not spider._follow_links - @inline_callbacks_test - def test_start_url(self): - class TestSpider(self.spider_class): - name = "test" - start_url = "https://www.example.com" - - crawler = get_crawler(TestSpider) - with LogCapture("scrapy.core.engine", propagate=False, level=ERROR) as log: - yield crawler.crawl() - assert "Error while reading start items and requests" in str(log) - assert "did you miss an 's'?" in str(log) - def test_parse_response_use(self): class _CrawlSpider(CrawlSpider): name = "test" diff --git a/tox.ini b/tox.ini index 44e240e03..2fa7b455d 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,7 @@ [tox] requires = - sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.6 + sphinx-scrapy[tox] @ git+https://github.com/scrapy/sphinx-scrapy.git@0.8.8 envlist = pre-commit pylint From f5a62a293f82bb3a7d4be1357ae8392d61f56021 Mon Sep 17 00:00:00 2001 From: tanishqtayade Date: Tue, 23 Jun 2026 12:34:01 +0530 Subject: [PATCH 05/18] Fix cell-var-from-loop bug in _send_catch_log_deferred (#7649) --- scrapy/utils/signal.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/scrapy/utils/signal.py b/scrapy/utils/signal.py index ee391a49e..919f67240 100644 --- a/scrapy/utils/signal.py +++ b/scrapy/utils/signal.py @@ -125,13 +125,11 @@ def _send_catch_log_deferred( **named, ) d.addErrback(logerror, receiver) - # TODO https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/cell-var-from-loop.html + d2: Deferred[tuple[TypingAny, TypingAny]] = d.addBoth( - lambda result: ( - receiver, # pylint: disable=cell-var-from-loop # noqa: B023 - result, - ) + lambda result, recv: (recv, result), receiver ) + dfds.append(d2) results = yield DeferredList(dfds) From fb3455304d9e9f5332a97edbe2ea0a77a874e974 Mon Sep 17 00:00:00 2001 From: smellslikeml Date: Tue, 23 Jun 2026 00:04:19 -0700 Subject: [PATCH 06/18] Add content-based image filtering example (#4954) --- docs/topics/media-pipeline.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index b542c6c05..c67d2d627 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -774,4 +774,28 @@ To enable your custom media pipeline component you must add its class import pat ITEM_PIPELINES = {"myproject.pipelines.MyImagesPipeline": 300} +Content-based image filtering pipeline +-------------------------------------- + +This example overrides ``get_images()`` to filter images using a classifier, +such as a TensorFlow_ model. Override ``is_valid_image()`` with your +classification logic: + +.. code-block:: python + + from scrapy.pipelines.images import ImagesPipeline, ImageException + + + class ImageClassifierPipeline(ImagesPipeline): + def is_valid_image(self, image): + raise NotImplementedError + + def get_images(self, response, request, info, *, item=None): + for path, image, buf in super().get_images(response, request, info, item=item): + if not self.is_valid_image(image): + raise ImageException("Image does not match criteria") + yield path, image, buf + + .. _MD5 hash: https://en.wikipedia.org/wiki/MD5 +.. _TensorFlow: https://tensorflow.org From b78ab3d6c8715d488ac657e0d09d0cf7bc6abc42 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 23 Jun 2026 11:47:21 +0200 Subject: [PATCH 07/18] Improve test coverage for settings/ (#7654) --- tests/test_settings/__init__.py | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index 908bb417c..3282b0591 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -539,6 +539,56 @@ class TestBaseSettings: msg = caplog.records[0].message assert "tests.test_settings.Component1" in msg + def test_getdictorlist(self): + settings = BaseSettings() + + # No value and no default → {} + assert settings.getdictorlist("MISSING") == {} + + # String: valid JSON dict + settings.set("S_DICT_STR", '{"key": "val"}') + assert settings.getdictorlist("S_DICT_STR") == {"key": "val"} + + # String: valid JSON list + settings.set("S_LIST_STR", '["a", "b"]') + assert settings.getdictorlist("S_LIST_STR") == ["a", "b"] + + # String: invalid JSON → comma-split fallback + settings.set("S_CSV", "a,b,c") + assert settings.getdictorlist("S_CSV") == ["a", "b", "c"] + + # String: valid JSON but not dict or list → ValueError caught → comma-split + settings.set("S_JSON_NUMBER", "123") + assert settings.getdictorlist("S_JSON_NUMBER") == ["123"] + + # Tuple → list + settings.set("S_TUPLE", ("x", "y")) + assert settings.getdictorlist("S_TUPLE") == ["x", "y"] + + # Unsupported type → raises ValueError + settings.set("S_INT", 42) + with pytest.raises(ValueError, match="must be a dict, list, tuple, or string"): + settings.getdictorlist("S_INT") + + # Dict value → deepcopy returned + settings.set("S_DICT", {"key": "val"}) + assert settings.getdictorlist("S_DICT") == {"key": "val"} + + # List value → deepcopy returned + settings.set("S_LIST", ["a", "b"]) + assert settings.getdictorlist("S_LIST") == ["a", "b"] + + def test_repr_pretty_(self): + settings = BaseSettings({"key": "value"}) + mock_p = mock.Mock() + + settings._repr_pretty_(mock_p, cycle=False) + assert mock_p.text.call_count == 1 + + mock_p.reset_mock() + settings._repr_pretty_(mock_p, cycle=True) + mock_p.text.assert_called_once_with(repr(settings)) + def test_getwithbase_invalid_setting_name(self): settings = BaseSettings() with pytest.raises( @@ -710,6 +760,15 @@ def test_remove_from_list(before, name, item, after): assert settings.getpriority(name) == expected_settings.getpriority(name) +def test_deprecated_dns_resolver_setting(): + settings = Settings() + with pytest.warns( + ScrapyDeprecationWarning, + match="The DNS_RESOLVER setting is deprecated", + ): + settings.get("DNS_RESOLVER") + + def test_deprecated_concurrent_requests_per_ip_setting(): settings = Settings({"CONCURRENT_REQUESTS_PER_IP": 1}) with pytest.warns( From dd4549e6f9a92f4844b552aad49ab4431f73514f Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 24 Jun 2026 22:48:28 +0200 Subject: [PATCH 08/18] 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 From 65e8954a0689b65ba8ae187a3045e8d4ed00b84f Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 25 Jun 2026 12:35:25 +0200 Subject: [PATCH 09/18] Improve test coverage for spider middlewares (#7664) --- scrapy/spidermiddlewares/referer.py | 6 ++++ tests/test_spidermiddleware_base.py | 5 +++ tests/test_spidermiddleware_depth.py | 25 ++++++++++++++ tests/test_spidermiddleware_referer.py | 43 +++++++++++++++++++++++- tests/test_spidermiddleware_start.py | 12 ++++++- tests/test_spidermiddleware_urllength.py | 8 +++++ 6 files changed, 97 insertions(+), 2 deletions(-) diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index 2c9452174..264f685c1 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -308,6 +308,12 @@ class RefererMiddleware(BaseSpiderMiddleware): # Reference: https://www.w3.org/TR/referrer-policy/#referrer-policy-empty-string self.policies[""] = NoReferrerWhenDowngradePolicy if settings is None: + warn( + "Instantiating RefererMiddleware without a 'settings' argument is " + "deprecated.", + ScrapyDeprecationWarning, + stacklevel=2, + ) return setting_policies = settings.getdict("REFERRER_POLICIES") for policy_name, policy_class_import_path in setting_policies.items(): diff --git a/tests/test_spidermiddleware_base.py b/tests/test_spidermiddleware_base.py index 70326f3f1..4de57fda9 100644 --- a/tests/test_spidermiddleware_base.py +++ b/tests/test_spidermiddleware_base.py @@ -56,6 +56,11 @@ async def test_processed_request(crawler: Crawler) -> None: spider_output = [test_req1, {"foo": "bar"}, test_req2, test_req3] for processed in [ list(mw.process_spider_output(Response("data:,"), spider_output)), + await collect_asyncgen( + mw.process_spider_output_async( + Response("data:,"), as_async_generator(spider_output) + ) + ), await collect_asyncgen(mw.process_start(as_async_generator(spider_output))), ]: assert len(processed) == 3 diff --git a/tests/test_spidermiddleware_depth.py b/tests/test_spidermiddleware_depth.py index 00e547305..307ed2440 100644 --- a/tests/test_spidermiddleware_depth.py +++ b/tests/test_spidermiddleware_depth.py @@ -7,6 +7,7 @@ import pytest from scrapy.http import Request, Response from scrapy.spidermiddlewares.depth import DepthMiddleware from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler if TYPE_CHECKING: @@ -55,3 +56,27 @@ def test_process_spider_output(mw: DepthMiddleware, stats: StatsCollector) -> No rdm = stats.get_value("request_depth_max") assert rdm == 1 + + +def test_priority_and_non_verbose_stats() -> None: + crawler = get_crawler( + Spider, + {"DEPTH_LIMIT": 0, "DEPTH_STATS_VERBOSE": False, "DEPTH_PRIORITY": 10}, + ) + assert crawler.stats is not None + crawler.stats.open_spider() + try: + mw = build_from_crawler(DepthMiddleware, crawler) + resp = Response("http://toscrape.com") + resp.request = Request("http://toscrape.com") + resp.request.meta["depth"] = 2 + out = list(mw.process_spider_output(resp, [Request("http://toscrape.com")])) + assert len(out) == 1 + # priority is decremented by depth * DEPTH_PRIORITY + assert out[0].priority == -30 + assert out[0].meta["depth"] == 3 + # non-verbose stats don't track per-depth counts but still track the max + assert crawler.stats.get_value("request_depth_count/3") is None + assert crawler.stats.get_value("request_depth_max") == 3 + finally: + crawler.stats.close_spider() diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index b3a434c3f..32e83f990 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -6,7 +6,7 @@ from urllib.parse import urlparse import pytest -from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.spidermiddlewares.referer import ( @@ -1017,6 +1017,14 @@ class TestPolicyMethodResponseParamRename: response=self.response, resp_or_url=self.response, request=self.request ) + def test_missing_response(self): + with pytest.raises(TypeError, match="Missing required argument: 'response'"): + self.mw.policy(request=self.request) + + def test_missing_request(self): + with pytest.raises(TypeError, match="Missing required argument: 'request'"): + self.mw.policy(response=self.response) + @coroutine_test async def test_response_policy_only_supports_policy_names(): @@ -1115,3 +1123,36 @@ async def test_referer_policies_setting(): ] assert len(output) == 1 assert output[0].headers == {b"Referer": [b"https://python.org/"]} + + +class TestReferrerPolicyHelpers: + def test_origin_referrer_local_scheme(self): + # A local scheme yields no referrer. + assert UnsafeUrlPolicy().origin_referrer("data:,foo") is None + + def test_strip_url_empty(self): + assert UnsafeUrlPolicy().strip_url("") is None + + def test_potentially_trustworthy_data_scheme(self): + assert UnsafeUrlPolicy().potentially_trustworthy("data:,foo") is False + + +def test_default_policy(): + crawler = get_crawler() + mw = build_from_crawler(RefererMiddleware, crawler) + assert mw.default_policy is DefaultReferrerPolicy + + +def test_no_settings_constructor(): + with pytest.warns( + ScrapyDeprecationWarning, + match="Instantiating RefererMiddleware without a 'settings' argument", + ): + mw = RefererMiddleware() + assert mw.default_policy is DefaultReferrerPolicy + + +def test_not_configured_when_disabled(): + crawler = get_crawler(settings_dict={"REFERER_ENABLED": False}) + with pytest.raises(NotConfigured): + build_from_crawler(RefererMiddleware, crawler) diff --git a/tests/test_spidermiddleware_start.py b/tests/test_spidermiddleware_start.py index 26397d37c..def3a3df3 100644 --- a/tests/test_spidermiddleware_start.py +++ b/tests/test_spidermiddleware_start.py @@ -1,4 +1,4 @@ -from scrapy.http import Request +from scrapy.http import Request, Response from scrapy.spidermiddlewares.start import StartSpiderMiddleware from scrapy.spiders import Spider from scrapy.utils.misc import build_from_crawler @@ -23,3 +23,13 @@ class TestMiddleware: async for request in mw.process_start(start()) ] assert result == [True, True, False, "foo"] + + def test_spider_output_not_marked(self): + # Requests from a non-None response (spider output) are not flagged. + crawler = get_crawler(Spider) + mw = build_from_crawler(StartSpiderMiddleware, crawler) + response = Response("data:,") + request = Request("data:,1") + out = list(mw.process_spider_output(response, [request])) + assert out == [request] + assert "is_start_request" not in request.meta diff --git a/tests/test_spidermiddleware_urllength.py b/tests/test_spidermiddleware_urllength.py index 750ae3b07..1ed3a5637 100644 --- a/tests/test_spidermiddleware_urllength.py +++ b/tests/test_spidermiddleware_urllength.py @@ -5,9 +5,11 @@ from typing import TYPE_CHECKING import pytest +from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.spidermiddlewares.urllength import UrlLengthMiddleware from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler if TYPE_CHECKING: @@ -46,6 +48,12 @@ def test_middleware_works(mw: UrlLengthMiddleware) -> None: assert process_spider_output(mw) == [short_url_req] +def test_not_configured_without_limit() -> None: + crawler = get_crawler(Spider, {"URLLENGTH_LIMIT": 0}) + with pytest.raises(NotConfigured): + build_from_crawler(UrlLengthMiddleware, crawler) + + def test_logging( stats: StatsCollector, mw: UrlLengthMiddleware, caplog: pytest.LogCaptureFixture ) -> None: From c690eac770a9fb803cdf0003ac3dc73e409146dc Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 25 Jun 2026 13:34:37 +0200 Subject: [PATCH 10/18] =?UTF-8?q?Document=20=E2=80=9Clogging=20settings?= =?UTF-8?q?=E2=80=9D=20as=20special=20settings=20(#7668)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/practices.rst | 8 ++--- docs/topics/settings.rst | 64 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 8a04f7ada..e7faf48c4 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -347,10 +347,10 @@ finishes before starting the next one: install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") react(deferred_f_from_coro_f(crawl)) -.. note:: When running multiple spiders in the same process, :ref:`reactor - settings ` should not have a different value per spider. - Also, :ref:`pre-crawler settings ` cannot be defined - per spider. +.. note:: When running multiple spiders in the same process, :ref:`logging + settings ` and :ref:`reactor settings ` + should not have a different value per spider, and :ref:`pre-crawler + settings ` cannot be defined per spider. .. seealso:: :ref:`run-from-script`. diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 455985a56..c452f88d1 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -361,6 +361,31 @@ All of these settings, except for :setting:`ASYNCIO_EVENT_LOOP`, are only used when the Twisted reactor is used, i.e. when :setting:`TWISTED_REACTOR_ENABLED` is ``True``. +.. _logging-settings: + +Logging settings +---------------- + +**Logging settings** are settings that configure the global root logging +handler installed by :func:`~scrapy.utils.log.configure_logging`. + +These settings can be defined from a spider. However, because only 1 root +logging handler is active per process, these settings cannot use a different +value per spider when :ref:`running multiple spiders in the same process +`. + +These settings are: + +- :setting:`LOG_DATEFORMAT` +- :setting:`LOG_ENABLED` +- :setting:`LOG_ENCODING` +- :setting:`LOG_FILE` +- :setting:`LOG_FILE_APPEND` +- :setting:`LOG_FORMAT` +- :setting:`LOG_LEVEL` +- :setting:`LOG_SHORT_NAMES` +- :setting:`LOG_STDOUT` + .. _topics-settings-ref: Built-in settings reference @@ -481,6 +506,8 @@ Note that the event loop class must inherit from :class:`asyncio.AbstractEventLo :func:`asyncio.set_event_loop`, which will set the specified event loop as the current loop for the current OS thread. +.. note:: This is a :ref:`reactor setting `. + .. setting:: BOT_NAME BOT_NAME @@ -662,6 +689,8 @@ Whether to enable DNS in-memory cache. :setting:`TWISTED_REACTOR_ENABLED` is ``False``, and may have no effect either when :setting:`DNS_RESOLVER` is set to a different resolver. +.. note:: This is a :ref:`reactor setting `. + .. setting:: DNSCACHE_SIZE DNSCACHE_SIZE @@ -671,6 +700,8 @@ Default: ``10000`` DNS in-memory cache size, see :setting:`DNSCACHE_ENABLED`. +.. note:: This is a :ref:`reactor setting `. + .. setting:: TWISTED_DNS_RESOLVER TWISTED_DNS_RESOLVER @@ -688,6 +719,8 @@ take the :setting:`DNS_TIMEOUT` setting into account. .. note:: This setting has no effect when :setting:`TWISTED_REACTOR_ENABLED` is ``False``. +.. note:: This is a :ref:`reactor setting `. + .. setting:: DNS_TIMEOUT DNS_TIMEOUT @@ -703,6 +736,8 @@ Timeout for processing of DNS queries in seconds. Float is supported. :setting:`TWISTED_REACTOR_ENABLED` is ``False``, and may have no effect either when :setting:`DNS_RESOLVER` is set to a different resolver. +.. note:: This is a :ref:`reactor setting `. + .. setting:: DOWNLOADER DOWNLOADER @@ -1442,6 +1477,8 @@ Default: ``True`` Whether to enable logging. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_ENCODING LOG_ENCODING @@ -1451,6 +1488,8 @@ Default: ``'utf-8'`` The encoding to use for logging. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_FILE LOG_FILE @@ -1460,6 +1499,8 @@ Default: ``None`` File name to use for logging output. If ``None``, standard error will be used. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_FILE_APPEND LOG_FILE_APPEND @@ -1470,6 +1511,8 @@ Default: ``True`` If ``False``, the log file specified with :setting:`LOG_FILE` will be overwritten (discarding the output from previous runs, if any). +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_FORMAT LOG_FORMAT @@ -1481,6 +1524,8 @@ String for formatting log messages. Refer to the :ref:`Python logging documentation ` for the whole list of available placeholders. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_DATEFORMAT LOG_DATEFORMAT @@ -1493,6 +1538,8 @@ in :setting:`LOG_FORMAT`. Refer to the :ref:`Python datetime documentation ` for the whole list of available directives. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_FORMATTER LOG_FORMATTER @@ -1512,6 +1559,8 @@ Default: ``'DEBUG'`` Minimum level to log. Available levels are: CRITICAL, ERROR, WARNING, INFO, DEBUG. For more info see :ref:`topics-logging`. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_STDOUT LOG_STDOUT @@ -1523,6 +1572,8 @@ If ``True``, all standard output (and error) of your process will be redirected to the log. For example if you ``print('hello')`` it will appear in the Scrapy log. +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_SHORT_NAMES LOG_SHORT_NAMES @@ -1533,6 +1584,8 @@ Default: ``False`` If ``True``, the logs will just contain the root path. If it is set to ``False`` then it displays the component responsible for the log output +.. note:: This is a :ref:`logging setting `. + .. setting:: LOG_VERSIONS LOG_VERSIONS @@ -1695,6 +1748,8 @@ multi-purpose thread pool used by various Scrapy components. Threaded DNS Resolver, BlockingFeedStorage, S3FilesStore just to name a few. Increase this value if you're experiencing problems with insufficient blocking IO. +.. note:: This is a :ref:`reactor setting `. + .. setting:: REDIRECT_PRIORITY_ADJUST REDIRECT_PRIORITY_ADJUST @@ -1921,6 +1976,8 @@ Default: ``'scrapy.spiderloader.SpiderLoader'`` The class that will be used for loading spiders, which must implement the :ref:`topics-api-spiderloader`. +.. note:: This is a :ref:`pre-crawler setting `. + .. setting:: SPIDER_LOADER_WARN_ONLY SPIDER_LOADER_WARN_ONLY @@ -1933,6 +1990,8 @@ it will fail loudly if there is any ``ImportError`` or ``SyntaxError`` exception But you can choose to silence this exception and turn it into a simple warning by setting ``SPIDER_LOADER_WARN_ONLY = True``. +.. note:: This is a :ref:`pre-crawler setting `. + .. setting:: SPIDER_MIDDLEWARES SPIDER_MIDDLEWARES @@ -1978,6 +2037,8 @@ Example: SPIDER_MODULES = ["mybot.spiders_prod", "mybot.spiders_dev"] +.. note:: This is a :ref:`pre-crawler setting `. + .. setting:: STATS_CLASS STATS_CLASS @@ -2049,7 +2110,7 @@ stopped) will not apply. This mode is currently experimental and may not be suitable for production use. It may also not be supported by 3rd-party code. See :ref:`asyncio-without-reactor` for more information about this mode. -.. note:: This setting can't be set :ref:`per-spider `. +.. note:: This is a :ref:`pre-crawler setting `. .. versionadded:: 2.15.0 @@ -2156,6 +2217,7 @@ current platform. For additional information, see :doc:`core/howto/choosing-reactor`. +.. note:: This is a :ref:`reactor setting `. .. setting:: URLLENGTH_LIMIT From 74e6b61071c9fee372da4342604cd35510d03718 Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 25 Jun 2026 13:44:51 +0200 Subject: [PATCH 11/18] Make DOWNLOADER_CLIENT_TLS_CIPHERS=None enable Twisted defaults (#7665) * Make DOWNLOADER_CLIENT_TLS_CIPHERS=None enable Twisted defaults * Remove pointless comment * Remove unnecessary mypy comments --- docs/topics/settings.rst | 3 ++ scrapy/core/downloader/contextfactory.py | 11 +++-- scrapy/core/downloader/tls.py | 12 +++--- tests/test_core_downloader.py | 52 +++++++++++++++++++++++- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c452f88d1..b92733ede 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -763,6 +763,9 @@ necessary to access certain HTTPS websites: for example, you may need to use ``'DEFAULT:!DH'`` for a website with weak DH parameters or enable a specific cipher that is not included in ``DEFAULT`` if a website requires it. +Set this setting to ``None`` to use the default ciphers of the underlying TLS +implementation. + .. _OpenSSL cipher list format: https://docs.openssl.org/master/man1/openssl-ciphers/#cipher-list-format .. note:: diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 40f09ffb2..c1796c723 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -17,7 +17,6 @@ from zope.interface.verify import verifyObject from scrapy.core.downloader.tls import ( _TWISTED_VERSION_MAP, - DEFAULT_CIPHERS, _openssl_methods, _ScrapyClientTLSOptions, _ScrapyClientTLSOptions26, @@ -68,11 +67,11 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): self.tls_min_version: TLSVersion | None = tls_min_version self.tls_max_version: TLSVersion | None = tls_max_version self.tls_verbose_logging: bool = tls_verbose_logging # unused - self.tls_ciphers: AcceptableCiphers - if tls_ciphers: - self.tls_ciphers = AcceptableCiphers.fromOpenSSLCipherString(tls_ciphers) - else: - self.tls_ciphers = DEFAULT_CIPHERS + self.tls_ciphers: AcceptableCiphers | None = ( + AcceptableCiphers.fromOpenSSLCipherString(tls_ciphers) + if tls_ciphers + else None + ) self._verify_certificates = verify_certificates @classmethod diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 3b903b39d..c5cf1156d 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -43,6 +43,13 @@ _openssl_methods: dict[str, int] = { def __getattr__(name: str) -> Any: + if name == "DEFAULT_CIPHERS": + warnings.warn( + "scrapy.core.downloader.tls.DEFAULT_CIPHERS is deprecated.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + return AcceptableCiphers.fromOpenSSLCipherString("DEFAULT") deprecated = { "METHOD_TLS": "TLS", "METHOD_TLSv10": "TLSv1.0", @@ -177,8 +184,3 @@ class _ScrapyClientTLSOptions26(ClientTLSOptions): return True return verifyCallback - - -DEFAULT_CIPHERS: AcceptableCiphers = AcceptableCiphers.fromOpenSSLCipherString( - "DEFAULT" -) diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index e348bfb7f..fdd5edc27 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -8,7 +8,7 @@ import pytest from pytest_twisted import async_yield_fixture from twisted.internet.protocol import Factory from twisted.internet.protocol import Protocol as TxProtocol -from twisted.internet.ssl import optionsForClientTLS +from twisted.internet.ssl import AcceptableCiphers, optionsForClientTLS from twisted.protocols.tls import TLSMemoryBIOFactory, TLSMemoryBIOProtocol from twisted.web import server, static from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody @@ -180,6 +180,49 @@ class TestContextFactory(TestContextFactoryBase): assert options & 0x4 # OP_LEGACY_SERVER_CONNECT +class TestContextFactoryCiphers(TestContextFactoryBase): + async def _assert_factory_works( + self, server_url: str, client_context_factory: _ScrapyClientContextFactory + ) -> None: + s = "0123456789" * 10 + body = await self.get_page( + server_url + "payload", client_context_factory, body=s + ) + assert body == to_bytes(s) + + def test_default(self) -> None: + """The default 'DEFAULT' value is passed to Twisted as is.""" + crawler = get_crawler() + factory = build_from_crawler(_ScrapyClientContextFactory, crawler) + assert factory.tls_ciphers is not None + # OpenSSLAcceptableCiphers has no __eq__, so compare the parsed ciphers. + assert ( + factory.tls_ciphers._ciphers + == AcceptableCiphers.fromOpenSSLCipherString("DEFAULT")._ciphers + ) + assert factory._get_cert_options_kwargs()["acceptableCiphers"] is not None + + def test_custom(self) -> None: + crawler = get_crawler( + settings_dict={"DOWNLOADER_CLIENT_TLS_CIPHERS": "CAMELLIA256-SHA"} + ) + factory = build_from_crawler(_ScrapyClientContextFactory, crawler) + assert factory.tls_ciphers is not None + assert ( + factory.tls_ciphers._ciphers + == AcceptableCiphers.fromOpenSSLCipherString("CAMELLIA256-SHA")._ciphers + ) + + @coroutine_test + async def test_none(self, server_url: str) -> None: + """A None value enables the Twisted default ciphers.""" + crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_CIPHERS": None}) + factory = build_from_crawler(_ScrapyClientContextFactory, crawler) + assert factory.tls_ciphers is None + assert factory._get_cert_options_kwargs()["acceptableCiphers"] is None + await self._assert_factory_works(server_url, factory) + + class TestContextFactoryTLSMethod(TestContextFactoryBase): async def _assert_factory_works( self, server_url: str, client_context_factory: _ScrapyClientContextFactory @@ -278,3 +321,10 @@ def test_deprecated_tls_module_names() -> None: match="scrapy.core.downloader.tls.openssl_methods is deprecated", ): assert isinstance(tls.openssl_methods, dict) + with pytest.warns( + ScrapyDeprecationWarning, + match="scrapy.core.downloader.tls.DEFAULT_CIPHERS is deprecated", + ): + assert tls.DEFAULT_CIPHERS._ciphers == ( + AcceptableCiphers.fromOpenSSLCipherString("DEFAULT")._ciphers + ) From d8d7de2339b3aeabdc144fa7d0d11cae3ecfd1bb Mon Sep 17 00:00:00 2001 From: Sriniketh24 Date: Fri, 26 Jun 2026 07:56:59 +0530 Subject: [PATCH 12/18] Fix FTPDownloadHandler not closing FTP connection after download (#7667) --- scrapy/core/downloader/handlers/ftp.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 6258067c1..9064aa53a 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -119,7 +119,10 @@ class FTPDownloadHandler(BaseDownloadHandler): httpcode = self.CODE_MAPPING.get(ftpcode, self.CODE_MAPPING["default"]) return Response(url=request.url, status=httpcode, body=message.encode()) raise - protocol.close() + finally: + protocol.close() + assert client.transport + client.transport.loseConnection() headers = {"local filename": protocol.filename or b"", "size": protocol.size} body = protocol.filename or protocol.body.read() respcls = responsetypes.from_args(url=request.url, body=body) From 0007676e8d5210f8d0d05ffb12610ca780a3c5ea Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 09:21:12 +0200 Subject: [PATCH 13/18] Improve test coverage for http/ (#7672) * Improve test coverage for http/ * Use isinstance() instead of type() --- tests/test_http_cookies.py | 53 +++++++++++++++++++++++++++++++- tests/test_http_headers.py | 1 + tests/test_http_request.py | 10 ++++++ tests/test_http_request_form.py | 3 ++ tests/test_http_response.py | 5 +++ tests/test_http_response_text.py | 7 +++++ 6 files changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/test_http_cookies.py b/tests/test_http_cookies.py index 660b76d08..ce9296764 100644 --- a/tests/test_http_cookies.py +++ b/tests/test_http_cookies.py @@ -1,8 +1,59 @@ +from http.cookiejar import DefaultCookiePolicy + from scrapy.http import Request, Response -from scrapy.http.cookies import WrappedRequest, WrappedResponse +from scrapy.http.cookies import CookieJar, WrappedRequest, WrappedResponse from scrapy.utils.httpobj import urlparse_cached +class TestCookieJar: + def setup_method(self): + self.jar = CookieJar() + self.request = Request("http://example.com/") + self.response = Response( + "http://example.com/", + headers={"Set-Cookie": "name=value; Domain=example.com; Path=/"}, + ) + + def test_extract_cookies(self): + assert len(self.jar) == 0 + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + cookie = next(iter(self.jar)) + assert cookie.name == "name" + assert cookie.value == "value" + assert ".example.com" in self.jar._cookies + + def test_make_cookies_and_set_cookie(self): + cookies = self.jar.make_cookies(self.response, self.request) + assert len(cookies) == 1 + jar = CookieJar() + for cookie in cookies: + jar.set_cookie(cookie) + assert len(jar) == 1 + + def test_clear(self): + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + self.jar.clear() + assert len(self.jar) == 0 + + def test_clear_session_cookies(self): + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + self.jar.clear_session_cookies() + assert len(self.jar) == 0 + + def test_set_policy(self): + policy = DefaultCookiePolicy() + self.jar.set_policy(policy) + assert self.jar.jar._policy is policy + + def test_check_expired_frequency(self): + jar = CookieJar(check_expired_frequency=1) + jar.add_cookie_header(self.request) + assert jar.processed == 1 + + class TestWrappedRequest: def setup_method(self): self.request = Request( diff --git a/tests/test_http_headers.py b/tests/test_http_headers.py index 243aa6afe..aff3562e3 100644 --- a/tests/test_http_headers.py +++ b/tests/test_http_headers.py @@ -140,6 +140,7 @@ class TestHeaders: h1["foo"] = "bar" h1["foo"] = None h1.setdefault("foo", "bar") + assert h1["foo"] is None assert h1.get("foo") is None assert h1.getlist("foo") == [] diff --git a/tests/test_http_request.py b/tests/test_http_request.py index e22689d49..1941b826f 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -24,6 +24,10 @@ class TestRequest: with pytest.raises(TypeError): self.request_class(123) + # priority argument must be an integer + with pytest.raises(TypeError, match="Request priority not an integer"): + self.request_class("http://www.example.com", priority="1") + r = self.request_class("http://www.example.com") assert isinstance(r.url, str) assert r.url == "http://www.example.com" @@ -274,6 +278,12 @@ class TestRequest: assert r4.meta == {} assert r4.dont_filter is False + # the cls argument allows changing the resulting class + custom_request_cls = type("CustomRequest", (self.request_class,), {}) + r5 = r1.replace(cls=custom_request_cls) + assert isinstance(r5, custom_request_cls) + assert r5.url == r1.url + def test_method_always_str(self): r = self.request_class("http://www.example.com", method="POST") assert isinstance(r.method, str) diff --git a/tests/test_http_request_form.py b/tests/test_http_request_form.py index a4f87d50d..c667e48a1 100644 --- a/tests/test_http_request_form.py +++ b/tests/test_http_request_form.py @@ -294,6 +294,9 @@ class TestFormRequest(TestRequest): assert request.method == "GET" request = FormRequest.from_response(response, method="POST") assert request.method == "POST" + # an explicit method=None skips form-method normalization + request = FormRequest.from_response(response, method=None) + assert request.method == "NONE" def test_from_response_override_url(self): response = _buildresponse( diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 079c547c7..a8ea4920e 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -254,6 +254,11 @@ class TestResponse: with pytest.raises(ValueError, match="url can't be None"): r.follow(None) + def test_follow_None_encoding(self): + r = self.response_class("http://example.com") + with pytest.raises(ValueError, match="encoding can't be None"): + r.follow("foo", encoding=None) + @pytest.mark.xfail( not W3LIB_STRIPS_URLS, reason="https://github.com/scrapy/w3lib/pull/207", diff --git a/tests/test_http_response_text.py b/tests/test_http_response_text.py index 4b3fa2302..507fd1864 100644 --- a/tests/test_http_response_text.py +++ b/tests/test_http_response_text.py @@ -14,6 +14,13 @@ from tests.test_http_response import TestResponse class TestTextResponse(TestResponse): response_class = TextResponse + def test_follow_None_encoding(self): + # unlike the base Response, TextResponse.follow() falls back to the + # response encoding when encoding is None instead of raising + r = self.response_class("http://example.com", body=b"hello", encoding="cp1252") + req = r.follow("foo", encoding=None) + assert req.encoding == "cp1252" + def test_replace(self): super().test_replace() r1 = self.response_class( From 0ccddb4f61b27dd493fd65329d039b88a3333e7b Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 11:47:30 +0200 Subject: [PATCH 14/18] Improve test coverage for contracts (#7677) --- tests/test_contracts.py | 180 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/tests/test_contracts.py b/tests/test_contracts.py index e80945b93..698044de7 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -133,6 +133,29 @@ class DemoSpider(Spider): """ return DemoItem(url=response.url) + def returns_request_range_fail(self, response): + """method which returns fewer requests than the expected range + @url http://scrapy.org + @returns requests 2 3 + """ + return Request("http://scrapy.org", callback=self.returns_item) + + def yields_item_and_request(self, response): + """yields one item and one request + @url http://scrapy.org + @returns items 1 1 + @scrapes name url + """ + yield DemoItem(name="test", url=response.url) + yield Request("http://scrapy.org", callback=self.returns_item) + + async def returns_async_gen(self, response): + """async generator callback + @url http://scrapy.org + @returns items 1 1 + """ + yield DemoItem(url=response.url) + def returns_dict_fail(self, response): """method which returns item @url http://scrapy.org @@ -437,6 +460,48 @@ class TestContractsManager: request.callback(response) self.should_error() + def test_returns_invalid_argument_count(self): + spider = DemoSpider() + with pytest.raises(ValueError, match="expected 1, 2 or 3, got 0"): + ReturnsContract(spider.returns_item) + with pytest.raises(ValueError, match="expected 1, 2 or 3, got 4"): + ReturnsContract(spider.returns_item, "items", "1", "2", "3") + + def test_returns_default_bounds(self): + spider = DemoSpider() + contract = ReturnsContract(spider.returns_item, "items") + assert contract.min_bound == 1 + assert contract.max_bound == float("inf") + + def test_returns_range_fail(self): + spider = DemoSpider() + response = ResponseMock() + + request = self.conman.from_method( + spider.returns_request_range_fail, self.results + ) + request.callback(response) + self.should_fail() + assert "expected 2..3" in self.results.failures[-1][-1] + + def test_returns_and_scrapes_ignore_other_types(self): + spider = DemoSpider() + response = ResponseMock() + + # @returns and @scrapes only count matching output objects and skip + # the request that is also yielded. + request = self.conman.from_method(spider.yields_item_and_request, self.results) + request.callback(response) + self.should_succeed() + + def test_testcase_str(self): + spider = DemoSpider() + contract = UrlContract(spider.returns_request, "http://scrapy.org") + assert ( + str(contract.testcase_pre) + == "[demo_spider] returns_request (@url pre-hook)" + ) + def test_scrapes(self): spider = DemoSpider() response = ResponseMock() @@ -570,6 +635,41 @@ class CustomFailContractPostProcess(Contract): raise KeyboardInterrupt("Post-process exception") +class PreProcessSuccessContract(Contract): + name = "pre_success" + + def pre_process(self, response): + return + + +class PreProcessAssertionFailContract(Contract): + name = "pre_assertion_fail" + + def pre_process(self, response): + raise AssertionError("pre-process assertion") + + +class PreProcessErrorContract(Contract): + name = "pre_error" + + def pre_process(self, response): + raise ValueError("pre-process error") + + +class PostProcessSuccessContract(Contract): + name = "post_success" + + def post_process(self, output): + return + + +class PostProcessErrorContract(Contract): + name = "post_error" + + def post_process(self, output): + raise ValueError("post-process error") + + class TestCustomContractPrePostProcess: def setup_method(self): self.results = TextTestResult(stream=None, descriptions=False, verbosity=0) @@ -601,3 +701,83 @@ class TestCustomContractPrePostProcess: assert not self.results.failures assert not self.results.errors + + def test_pre_hook_success(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert not self.results.failures + assert not self.results.errors + + def test_pre_hook_assertion_failure(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessAssertionFailContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.failures + assert not self.results.errors + + def test_pre_hook_error(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessErrorContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.errors + + def test_pre_hook_async_callback(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_request_async) + request = Request("http://scrapy.org", callback=spider.returns_request_async) + contract.add_pre_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_pre_hook_async_generator(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_async_gen) + request = Request("http://scrapy.org", callback=spider.returns_async_gen) + contract.add_pre_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_post_hook_async_generator(self): + spider = DemoSpider() + response = ResponseMock() + contract = PostProcessSuccessContract(spider.returns_async_gen) + request = Request("http://scrapy.org", callback=spider.returns_async_gen) + contract.add_post_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_post_hook_error(self): + spider = DemoSpider() + response = ResponseMock() + contract = PostProcessErrorContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_post_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.errors From 7ab404c72546a9f0ef5bf4a6ee54512d3c1d6851 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 11:58:37 +0200 Subject: [PATCH 15/18] Add a documentation page about security (#7678) --- docs/index.rst | 5 + docs/topics/download-handlers.rst | 4 + docs/topics/downloader-middleware.rst | 2 + docs/topics/security.rst | 207 ++++++++++++++++++++++++++ docs/topics/settings.rst | 16 ++ docs/topics/spider-middleware.rst | 2 + docs/topics/telnetconsole.rst | 4 + 7 files changed, 240 insertions(+) create mode 100644 docs/topics/security.rst diff --git a/docs/index.rst b/docs/index.rst index a46a2ad9f..8e8624a22 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -151,6 +151,7 @@ Solving specific problems topics/debug topics/contracts topics/practices + topics/security topics/broad-crawls topics/developer-tools topics/dynamic-content @@ -175,6 +176,10 @@ Solving specific problems :doc:`topics/practices` Get familiar with some Scrapy common practices. +:doc:`topics/security` + Understand the security implications of Scrapy defaults and how to harden + them. + :doc:`topics/broad-crawls` Tune Scrapy for crawling a lot domains in parallel. diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 888bfaf08..c901e01e4 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -39,6 +39,10 @@ for additional schemes and to replace or disable default ones: "sftp": "my.download_handlers.SftpHandler", } +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources`, for the security implications of the + default ``http``, ``ftp``, ``file`` and ``data`` handlers. + Replacing HTTP(S) download handlers ----------------------------------- diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 0c1af5276..5649453b1 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -379,6 +379,8 @@ that this risks leaking credentials to unrelated domains. This setting must be explicitly configured whenever :setting:`HTTPAUTH_USER` or :setting:`HTTPAUTH_PASS` is set. +.. seealso:: :ref:`security-credential-leakage` + .. _Basic access authentication: https://en.wikipedia.org/wiki/Basic_access_authentication diff --git a/docs/topics/security.rst b/docs/topics/security.rst new file mode 100644 index 000000000..2ca270045 --- /dev/null +++ b/docs/topics/security.rst @@ -0,0 +1,207 @@ +.. _security: + +======== +Security +======== + +Scrapy defaults are optimized for web scraping, not for the security posture +that you might expect from software that handles untrusted input or runs in a +shared or exposed environment. Some common security practices are unnecessary +for many scraping use cases, and a few can even prevent valid ones (for +example, sites that you must scrape may use misconfigured TLS certificates or +serve content over unencrypted protocols). + +This page highlights the Scrapy defaults that have security implications, so +that you can make an informed decision about whether to keep them, and explains +how to harden them along with the trade-offs involved. + +.. note:: + + None of the options below are silver bullets. Which of them make sense + depends on your threat model: whether the URLs you crawl come from trusted + sources, whether the machine running Scrapy is exposed to a network you do + not control, whether the data you handle is sensitive, and so on. + +.. _security-untrusted-responses: + +Treat responses as untrusted input +================================== + +Regardless of any setting, remember that response data comes from servers you +do not control, even when you trust the site you are crawling, as responses may +be tampered with in transit or the server itself may be compromised. + +Never pass response data to functions that can execute code or otherwise act on +their input in an unsafe way, such as :func:`eval`, :func:`exec`, or +:func:`pickle.loads`, and be careful when writing response data to paths +derived from the response itself. + +TLS connections +=============== + +.. _security-certificate-verification: + +Certificate verification +------------------------ + +By default Scrapy does **not** verify the TLS certificate of HTTPS servers, as +controlled by the :setting:`DOWNLOAD_VERIFY_CERTIFICATES` setting (default: +``False``). + +This default favors reach over security: many sites that are otherwise fine to +scrape have expired, self-signed, or otherwise invalid certificates, and +verifying certificates would make requests to them fail. + +If the integrity of the connection matters to you (for example, to detect +man-in-the-middle attacks), set: + +.. code-block:: python + + DOWNLOAD_VERIFY_CERTIFICATES = True + +* **Pro:** requests to servers with invalid or untrusted certificates fail + instead of silently succeeding, protecting you from some man-in-the-middle + attacks. + +* **Con:** you can no longer scrape sites with misconfigured certificates + without re-disabling verification for them. + +.. _security-tls-protocols-ciphers: + +Protocol versions and ciphers +----------------------------- + +You can restrict the TLS protocol versions that Scrapy accepts through the +:setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` +settings, e.g. to reject obsolete protocol versions. + +By default Scrapy uses the OpenSSL ``DEFAULT`` cipher list +(:setting:`DOWNLOADER_CLIENT_TLS_CIPHERS`), which favors compatibility and still +allows some older, weaker ciphers. Set it to ``None`` to instead use the curated +cipher list of the underlying TLS implementation (Twisted), which excludes weak +ciphers: + +.. code-block:: python + + DOWNLOADER_CLIENT_TLS_CIPHERS = None + +* **Pro:** connections that would negotiate a weak cipher fail instead of + succeeding. + +* **Con:** you can no longer connect to servers that only support the excluded + ciphers. + +.. _security-unencrypted-protocols: + +Unencrypted protocols +===================== + +By default Scrapy enables download handlers for unencrypted protocols, namely +``http://`` and ``ftp://`` (see :setting:`DOWNLOAD_HANDLERS_BASE`). Data sent +and received over these protocols, including any credentials, travels in plain +text and can be read or modified by anyone on the network path. + +If you only crawl over encrypted protocols, you can disable the unencrypted +ones so that no request can accidentally be sent unencrypted: + +.. code-block:: python + + DOWNLOAD_HANDLERS = { + "http": None, + "ftp": None, + } + +* **Pro:** a misconfigured or maliciously-redirected request cannot leak data + over an unencrypted connection, as such requests fail instead. + +* **Con:** you can no longer crawl resources that are only available over those + protocols. + +Note that disabling the ``http`` handler also prevents plain-HTTP requests that +result from following an ``http://`` redirect or link, which is often the point +of disabling it. + +.. _security-local-resources: + +Local and non-network resources +=============================== + +By default Scrapy enables download handlers for the ``file://`` and ``data:`` +schemes (see :setting:`DOWNLOAD_HANDLERS_BASE`). The ``file://`` handler reads +arbitrary files from the local filesystem, limited only by the permissions of +the process running Scrapy. + +This is convenient (for example, to parse a local HTML file), but it is a risk +if any of the URLs you schedule come from an untrusted source: a crafted +``file:///etc/passwd`` URL could read local files. + +If you do not need them, disable these handlers: + +.. code-block:: python + + DOWNLOAD_HANDLERS = { + "file": None, + "data": None, + } + +* **Pro:** crawled URLs cannot be used to read local files or inline data. + +* **Con:** you can no longer fetch ``file://`` or ``data:`` URLs. + +More generally, if you crawl URLs from untrusted sources, consider validating +their schemes (and, where applicable, their hosts) before scheduling requests, +to avoid server-side request forgery (SSRF) and similar issues. + +.. _security-telnet: + +Telnet console +============== + +Scrapy enables the :ref:`telnet console ` by default +(:setting:`TELNETCONSOLE_ENABLED`). The telnet console is a Python shell +running inside the Scrapy process, so anyone who can connect to it can run +arbitrary code in that process. + +By default the console binds to ``127.0.0.1`` (:setting:`TELNETCONSOLE_HOST`) +and is protected by a username (:setting:`TELNETCONSOLE_USERNAME`, default +``scrapy``) and an automatically generated password +(:setting:`TELNETCONSOLE_PASSWORD`), so it is only reachable from the local +machine. + +.. warning:: + + Telnet does not provide any transport-layer security, so the + username/password authentication does not protect the credentials or the + session from anyone able to observe the traffic. Never expose the telnet + console over an untrusted network by changing :setting:`TELNETCONSOLE_HOST` + to a non-local address. + +If you do not use the telnet console, disable it entirely: + +.. code-block:: python + + TELNETCONSOLE_ENABLED = False + +* **Pro:** removes a local code-execution surface and one less listening port. + +* **Con:** you can no longer :ref:`inspect and control a running crawler + ` through it. + +.. _security-credential-leakage: + +Credential leakage across domains +================================= + +Some Scrapy features attach credentials or other sensitive headers to requests, +and a crawl that spans multiple domains can leak them to unintended hosts: + +* HTTP authentication credentials set through + :class:`~scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware` are only + sent to the domain set in :setting:`HTTPAUTH_DOMAIN`. Leave this set to the + intended domain rather than ``None`` so that credentials are not sent to + every domain you crawl. + +* The ``Referer`` header may disclose the URLs you crawl to other sites. The + default :setting:`REFERRER_POLICY` already avoids sending the referrer from + HTTPS to HTTP, but you can tighten it further (for example, to + ``same-origin`` or ``no-referrer``) if needed. diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index b92733ede..11251a92c 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -774,6 +774,8 @@ implementation. handler `, so it's not guaranteed to be supported by all 3rd-party handlers. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOAD_TLS_MAX_VERSION DOWNLOAD_TLS_MAX_VERSION @@ -807,6 +809,8 @@ modern environments. by all 3rd-party handlers. Additionally, the set of supported TLS versions depends on the TLS implementation being used by the handler. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOAD_TLS_MIN_VERSION DOWNLOAD_TLS_MIN_VERSION @@ -819,6 +823,8 @@ be used by Scrapy. See :setting:`DOWNLOAD_TLS_MAX_VERSION` for the details and limitations. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING @@ -979,6 +985,9 @@ enabled in your project. See :setting:`DOWNLOAD_HANDLERS_BASE` for example format. +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources` + .. setting:: DOWNLOAD_HANDLERS_BASE DOWNLOAD_HANDLERS_BASE @@ -1026,6 +1035,9 @@ handler (without replacement), place this in your ``settings.py``: "ftp": None, } +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources` + .. setting:: DOWNLOAD_SLOTS @@ -1184,6 +1196,8 @@ when making a request and abort the request if the verification fails. certificate problems are logged when this setting is set to ``False``) depends on its implementation. +.. seealso:: :ref:`security-certificate-verification` + .. setting:: DUPEFILTER_CLASS DUPEFILTER_CLASS @@ -2074,6 +2088,8 @@ Default: ``True`` (``False`` when :setting:`TWISTED_REACTOR_ENABLED` is ``False` A boolean which specifies if the :ref:`telnet console ` will be enabled (provided its extension is also enabled). +.. seealso:: :ref:`security-telnet` + .. setting:: TEMPLATES_DIR TEMPLATES_DIR diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 99bbdf292..e2cd0f986 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -355,6 +355,8 @@ Default: ``'scrapy.spidermiddlewares.referer.DefaultReferrerPolicy'`` using the special ``"referrer_policy"`` :ref:`Request.meta ` key, with the same acceptable values as for the ``REFERRER_POLICY`` setting. +.. seealso:: :ref:`security-credential-leakage` + Acceptable values for REFERRER_POLICY ************************************* diff --git a/docs/topics/telnetconsole.rst b/docs/topics/telnetconsole.rst index e274edc9b..a30258112 100644 --- a/docs/topics/telnetconsole.rst +++ b/docs/topics/telnetconsole.rst @@ -29,6 +29,8 @@ disable it if you want. For more information about the extension itself see .. note:: This feature is not supported when :setting:`TWISTED_REACTOR_ENABLED` is ``False``. +.. seealso:: :ref:`security-telnet` + .. highlight:: none How to access the telnet console @@ -190,6 +192,8 @@ Default: ``'127.0.0.1'`` The interface the telnet console should listen on +.. seealso:: :ref:`security-telnet` + .. setting:: TELNETCONSOLE_USERNAME From 1b940a75ac20401b1e944a65c8901569c06e091b Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 13:18:22 +0200 Subject: [PATCH 16/18] Improve the item pipeline docs (#7676) --- docs/topics/item-pipeline.rst | 111 ++++++++++++++++++ .../project/module/pipelines.py.tmpl | 2 +- 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 9a53f88fb..f1fa463d8 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -57,6 +57,8 @@ Any of these methods may be defined as a coroutine function (``async def``). Item pipeline example ===================== +.. _price-pipeline-example: + Price validation and dropping items with no prices -------------------------------------------------- @@ -246,6 +248,8 @@ returns multiples items with the same id: return item +.. _activating-item-pipeline: + Activating an Item Pipeline component ===================================== @@ -262,3 +266,110 @@ To activate an Item Pipeline component you must add its class to the The integer values you assign to classes in this setting determine the order in which they run: items go through from lower valued to higher valued classes. It's customary to define these numbers in the 0-1000 range. + +A complete example +================== + +The examples above show item pipeline components on their own. In a project, a +pipeline is one of four pieces that work together: the :ref:`item +` your spider produces, the :ref:`spider ` that +yields it, the pipeline that processes it, and the :setting:`ITEM_PIPELINES` +setting that enables the pipeline. + +The following example wires those pieces together to validate the price of +books scraped from `books.toscrape.com`_, reusing the ``PricePipeline`` from +:ref:`price-pipeline-example` above. + +Define the item in ``myproject/items.py``: + +.. code-block:: python + + from dataclasses import dataclass + + + @dataclass + class BookItem: + title: str + price: float + +Yield instances of that item from your spider, e.g. in +``myproject/spiders/books.py``: + +.. skip: next +.. code-block:: python + + import scrapy + + from myproject.items import BookItem + + + class BooksSpider(scrapy.Spider): + name = "books" + start_urls = ["https://books.toscrape.com/"] + + def parse(self, response): + for book in response.css("article.product_pod"): + yield BookItem( + title=book.css("h3 a::attr(title)").get(), + price=float(book.css("p.price_color::text").re_first(r"[\d.]+")), + ) + +Put the ``PricePipeline`` shown earlier in ``myproject/pipelines.py``, and +enable it in ``myproject/settings.py``: + +.. code-block:: python + + ITEM_PIPELINES = { + "myproject.pipelines.PricePipeline": 300, + } + +With these pieces in place, every ``BookItem`` that ``BooksSpider`` yields +passes through ``PricePipeline`` before it reaches the :ref:`feed exports +` or any other output. + +.. _books.toscrape.com: https://books.toscrape.com/ + + +Common pitfalls +=============== + +The pipeline does not run +------------------------- + +A pipeline component only runs if its class is listed in the +:setting:`ITEM_PIPELINES` setting, normally in your project's +:file:`settings.py` file (see :ref:`activating-item-pipeline`). Adding it to +the spider or elsewhere has no effect. + +To confirm that Scrapy loaded your pipeline, look for a line like this near the +start of the crawl log:: + + [scrapy.middleware] INFO: Enabled item pipelines: + ['myproject.pipelines.PricePipeline'] + +If your pipeline is missing from that list, check that its import path matches +the :setting:`ITEM_PIPELINES` entry, and that the setting is not being +overridden, for example by :attr:`~scrapy.Spider.custom_settings` or by a +redefinition of :setting:`ITEM_PIPELINES` in :file:`settings.py`. + +The item is not returned +------------------------ + +:meth:`process_item` must return the item (or raise +:exc:`~scrapy.exceptions.DropItem`). A common mistake is to modify the item but +forget to return it: + +.. code-block:: python + + def process_item(self, item): + ItemAdapter(item)["price"] *= 1.15 + # Bug: returns None, so the next component gets None instead of the item. + +Return the item so that the next component, and the rest of Scrapy, can keep +processing it: + +.. code-block:: python + + def process_item(self, item): + ItemAdapter(item)["price"] *= 1.15 + return item diff --git a/scrapy/templates/project/module/pipelines.py.tmpl b/scrapy/templates/project/module/pipelines.py.tmpl index e845f43e9..a6494b391 100644 --- a/scrapy/templates/project/module/pipelines.py.tmpl +++ b/scrapy/templates/project/module/pipelines.py.tmpl @@ -9,5 +9,5 @@ from itemadapter import ItemAdapter class ${ProjectName}Pipeline: - def process_item(self, item, spider): + def process_item(self, item): return item From edc353c975d2c7fdef480bcaad27c4ff6602682d Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 16:40:36 +0200 Subject: [PATCH 17/18] Improve test coverage for shell/ (#7680) * Improve test coverage for shell/ * Address test issues * Make the shell config test more reliable on Windows --- tests/test_command_shell.py | 196 +++++++++++++++++++++++++++++++++++- 1 file changed, 195 insertions(+), 1 deletion(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 1585835cc..1c109a333 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,19 +1,28 @@ from __future__ import annotations +import importlib.util import os +import signal import sys from io import BytesIO from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, MagicMock, patch import pytest from pexpect.popen_spawn import PopenSpawn +from scrapy import Spider +from scrapy.http import Request, Response +from scrapy.shell import Shell, inspect_response from scrapy.utils.reactor import _asyncio_reactor_path +from scrapy.utils.test import get_crawler from tests import NON_EXISTING_RESOLVABLE, tests_datadir from tests.utils.cmdline import proc +from tests.utils.decorators import coroutine_test if TYPE_CHECKING: + from scrapy.crawler import Crawler from tests.mockserver.http import MockServer @@ -139,6 +148,19 @@ class TestShellCommand: ) assert ret == 0, err + def test_shelp(self) -> None: + ret, out, _ = proc("shell", "-c", "shelp()") + assert ret == 0, out + assert "Available Scrapy objects" in out + + def test_fetch_request_with_callbacks(self, mockserver: MockServer) -> None: + url = mockserver.url("/text") + code = ( + f"fetch(scrapy.Request('{url}', callback=lambda r: r, errback=lambda f: f))" + ) + ret, out, _ = proc("shell", "-c", code) + assert ret == 0, out + class TestInteractiveShell: def test_fetch(self, mockserver: MockServer) -> None: @@ -161,3 +183,175 @@ class TestInteractiveShell: p.wait() # type: ignore[no-untyped-call] logfile.seek(0) assert "Traceback" not in logfile.read().decode() + + @staticmethod + def _isolate_config(env: dict[str, str], config_home: Path) -> None: + """Point every scrapy.cfg location (see + :func:`scrapy.utils.conf.get_sources`) at ``config_home``. + + ``XDG_CONFIG_HOME`` is read by Scrapy on all platforms, while + ``~/.scrapy.cfg`` goes through :func:`os.path.expanduser`, which uses + ``HOME`` on POSIX and ``USERPROFILE`` on Windows. The working directory + stays at the repository root (no scrapy.cfg) so subprocess coverage data + is still collected there. + """ + env.pop("SCRAPY_PYTHON_SHELL", None) + env["HOME"] = str(config_home) + env["USERPROFILE"] = str(config_home) + env["XDG_CONFIG_HOME"] = str(config_home) + + def _run_interactive_shell(self, env: dict[str, str]) -> str: + args = (sys.executable, "-m", "scrapy.cmdline", "shell") + logfile = BytesIO() + p = PopenSpawn(args, env=env, timeout=5) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + p.sendeof() + p.wait() # type: ignore[no-untyped-call] + logfile.seek(0) + return logfile.read().decode() + + @pytest.mark.skipif( + importlib.util.find_spec("IPython") is None, + reason="Without IPython installed, shell=python and the default both " + "select the standard Python shell, so the setting has no observable effect.", + ) + def test_shell_from_cfg(self, tmp_path: Path) -> None: + config_home = tmp_path / "config" + config_home.mkdir() + (config_home / "scrapy.cfg").write_text("[settings]\nshell = python\n") + env = os.environ.copy() + self._isolate_config(env, config_home) + args = (sys.executable, "-m", "scrapy.cmdline", "shell") + logfile = BytesIO() + p = PopenSpawn(args, env=env, timeout=10) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + # The standard Python shell never imports IPython, whereas the IPython + # shell (the default when installed) does; this confirms the configured + # shell=python was honored, regardless of platform-specific prompts. + p.sendline("import sys; print('IPYMODULE', 'IPython' in sys.modules)") + p.expect_exact("IPYMODULE False") + p.sendeof() + p.wait() # type: ignore[no-untyped-call] + logfile.seek(0) + assert "Traceback" not in logfile.read().decode() + + def test_shell_default_shells(self, tmp_path: Path) -> None: + config_home = tmp_path / "config" + config_home.mkdir() + env = os.environ.copy() + self._isolate_config(env, config_home) + assert "Traceback" not in self._run_interactive_shell(env) + + +@pytest.fixture +def restore_sigint(): + """Shell.start() installs SIG_IGN as the SIGINT handler; restore it.""" + handler = signal.getsignal(signal.SIGINT) + try: + yield + finally: + signal.signal(signal.SIGINT, handler) + + +def _no_reactor_crawler(monkeypatch: pytest.MonkeyPatch) -> Crawler: + """Return a crawler that reports ``TWISTED_REACTOR_ENABLED=False``. + + A genuine no-reactor crawler cannot be built while a Twisted reactor is + installed (as it is during the test run), so we build a normal crawler and + make its settings report the reactor as disabled, which is all the shell + code looks at. + """ + crawler = get_crawler() + real_getbool = crawler.settings.getbool + + def fake_getbool(name: str, *args: Any, **kwargs: Any) -> bool: + if name == "TWISTED_REACTOR_ENABLED": + return False + return real_getbool(name, *args, **kwargs) + + monkeypatch.setattr(crawler.settings, "getbool", fake_getbool) + return crawler + + +@pytest.mark.requires_reactor +class TestShell: + """Tests for :class:`~scrapy.shell.Shell` paths with no ``scrapy shell`` + command-line route: those reached through + :func:`scrapy.shell.inspect_response` (called from spider callbacks) or only + through direct API use, hence not covered by the subprocess tests above. + """ + + def test_populate_vars_fetch_not_available(self) -> None: + shell = Shell(get_crawler()) + shell._inthread = False + shell.populate_vars() + assert "fetch" not in shell.vars + + def test_get_help_fetch_not_available(self) -> None: + shell = Shell(get_crawler()) + shell._inthread = False + shell.populate_vars() + help_text = shell.get_help() + assert "fetch(url" not in help_text + assert "shelp()" in help_text + + def test_start_with_request(self, restore_sigint: None) -> None: + shell = Shell(get_crawler(), code="1") + shell.fetch = MagicMock() # type: ignore[method-assign] + request = Request("data:,") + shell.start(request=request) + shell.fetch.assert_called_once_with(request, None) + + def test_start_with_response( + self, restore_sigint: None, capsys: pytest.CaptureFixture[str] + ) -> None: + shell = Shell(get_crawler(), code="response.url") + request = Request("data:,") + response = Response("data:,", request=request) + shell.start(response=response) + assert "data:," in capsys.readouterr().out + assert shell.vars["response"] is response + assert shell.vars["request"] is request + + @patch("scrapy.shell.start_python_console") + def test_inspect_response( + self, mock_console: MagicMock, restore_sigint: None + ) -> None: + crawler = get_crawler() + spider = crawler._create_spider() + response = Response("data:,", request=Request("data:,")) + sigint_handler = signal.getsignal(signal.SIGINT) + inspect_response(response, spider) + mock_console.assert_called_once() + assert signal.getsignal(signal.SIGINT) is sigint_handler + + @coroutine_test + async def test_open_spider_explicit_spider(self) -> None: + crawler = get_crawler() + crawler.engine = MagicMock() + crawler.engine.open_spider_async = AsyncMock() + shell = Shell(crawler) + spider = Spider("test") + await shell._open_spider(spider) + assert shell.spider is spider + assert crawler.spider is spider + crawler.engine.open_spider_async.assert_called_once_with(close_if_idle=False) + + +@pytest.mark.only_asyncio +class TestShellNoReactor: + @coroutine_test + @patch("scrapy.shell.start_python_console") + async def test_inspect_response_no_reactor( + self, + mock_console: MagicMock, + restore_sigint: None, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + crawler = _no_reactor_crawler(monkeypatch) + spider = crawler._create_spider() + response = Response("data:,", request=Request("data:,")) + inspect_response(response, spider) + mock_console.assert_called_once() From cf5607f8bca45fa7c86b053b599a457542641ec1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 17:35:46 +0200 Subject: [PATCH 18/18] Undeprecated the basic FormRequest API (#7671) --- docs/topics/request-response.rst | 5 +++ scrapy/http/__init__.py | 17 +-------- scrapy/http/request/form.py | 63 ++++++++++++++++++++++++++++---- tests/test_http_request_form.py | 28 +++++++++++++- 4 files changed, 89 insertions(+), 24 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 700238fe9..5e70a4f98 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -911,6 +911,11 @@ Request subclasses Here is the list of built-in :class:`~scrapy.Request` subclasses. You can also subclass it to implement your own custom functionality. +FormRequest +----------- + +.. autoclass:: scrapy.FormRequest + JsonRequest ----------- diff --git a/scrapy/http/__init__.py b/scrapy/http/__init__.py index e20e894ae..0e5c2b53b 100644 --- a/scrapy/http/__init__.py +++ b/scrapy/http/__init__.py @@ -5,11 +5,9 @@ Use this module (instead of the more specific ones) when importing Headers, Request and Response outside this module. """ -from warnings import catch_warnings, filterwarnings - -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http.headers import Headers from scrapy.http.request import Request +from scrapy.http.request.form import FormRequest from scrapy.http.request.json_request import JsonRequest from scrapy.http.request.rpc import XmlRpcRequest from scrapy.http.response import Response @@ -17,19 +15,6 @@ from scrapy.http.response.html import HtmlResponse from scrapy.http.response.json import JsonResponse from scrapy.http.response.text import TextResponse from scrapy.http.response.xml import XmlResponse -from scrapy.utils.deprecate import create_deprecated_class - -with catch_warnings(): - filterwarnings("ignore", category=ScrapyDeprecationWarning) - - from scrapy.http.request.form import FormRequest as _FormRequest - - FormRequest = create_deprecated_class( - name="FormRequest", - new_class=_FormRequest, - subclass_warn_message="{cls} inherits from deprecated class {old}, use the form2request library instead.", - instance_warn_message="{cls} is deprecated, use the form2request library instead.", - ) __all__ = [ "FormRequest", diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index 4da595b22..f1a8dbf3b 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -32,19 +32,61 @@ if TYPE_CHECKING: from scrapy.http.response.text import TextResponse -warn( - "The entire scrapy.http.request.form module is deprecated. Use the " - "form2request library instead.", - ScrapyDeprecationWarning, - stacklevel=2, -) - FormdataVType: TypeAlias = str | Iterable[str] FormdataKVType: TypeAlias = tuple[str, FormdataVType] FormdataType: TypeAlias = dict[str, FormdataVType] | list[FormdataKVType] | None class FormRequest(Request): + """A :class:`~scrapy.Request` subclass with a ``formdata`` parameter that + url-encodes the given data and assigns it to the request, which makes it + convenient to send arbitrary form data via HTTP POST or GET without an HTML + ``
`` element to parse. + + .. note:: To build a request from an HTML ```` element found in a + response, use :doc:`form2request ` instead. See + :ref:`form`. + + The remaining arguments are the same as for the :class:`~scrapy.Request` + class and are not documented here. + + :param formdata: a dictionary (or iterable of (key, value) tuples) + containing HTML form data which will be url-encoded. If + :attr:`~scrapy.Request.method` is not given and ``formdata`` is + provided, the method is set to ``"POST"`` and the data is assigned to + the request body; if the method is ``"GET"``, the data is added to the + URL query string instead. + :type formdata: dict or collections.abc.Iterable + + To send data via HTTP POST, simulating an HTML form submission, return a + :class:`~scrapy.FormRequest` object from your spider: + + .. skip: next + .. code-block:: python + + return [ + FormRequest( + url="http://www.example.com/post/action", + formdata={"name": "John Doe", "age": "27"}, + callback=self.after_post, + ) + ] + + To send the data in the URL query string instead, use the ``GET`` method: + + .. skip: next + .. code-block:: python + + return [ + FormRequest( + url="http://www.example.com/search", + method="GET", + formdata={"q": "keyword", "page": "1"}, + callback=self.parse_results, + ) + ] + """ + __slots__ = () valid_form_methods: ClassVar[list[str]] = ["GET", "POST"] @@ -84,6 +126,13 @@ class FormRequest(Request): formcss: str | None = None, **kwargs: Any, ) -> Self: + warn( + "FormRequest.from_response() is deprecated. Use the form2request " + "library instead.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + kwargs.setdefault("encoding", response.encoding) if formcss is not None: diff --git a/tests/test_http_request_form.py b/tests/test_http_request_form.py index c667e48a1..af86b35c0 100644 --- a/tests/test_http_request_form.py +++ b/tests/test_http_request_form.py @@ -1,10 +1,12 @@ from __future__ import annotations import re +import warnings from urllib.parse import parse_qs, unquote_to_bytes import pytest +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import FormRequest, HtmlResponse from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_unicode @@ -26,15 +28,39 @@ def _qs(req, encoding="utf-8", to_unicode=False): return parse_qs(uqs, True) +# FormRequest.from_response() is deprecated in favor of form2request, so the +# many tests below that exercise it ignore the resulting deprecation warning. @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class TestFormRequest(TestRequest): - request_class = FormRequest # type: ignore[assignment] + request_class = FormRequest def assertQueryEqual(self, first, second, msg=None): first = to_unicode(first).split("&") second = to_unicode(second).split("&") assert sorted(first) == sorted(second), msg + def test_init_not_deprecated(self): + # Building a request directly from form data is not deprecated. + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) + self.request_class( + "http://www.example.com", formdata={"a": "1"}, method="POST" + ) + self.request_class( + "http://www.example.com", method="GET", formdata={"a": "1"} + ) + + def test_from_response_deprecated(self): + response = _buildresponse( + """ + + """ + ) + with pytest.warns( + ScrapyDeprecationWarning, match=r"FormRequest\.from_response\(\)" + ): + self.request_class.from_response(response) + def test_empty_formdata(self): r1 = self.request_class("http://www.example.com", formdata={}) assert r1.body == b""