From eabb149f4bf7737eb469319c7ce2c98ed0a6c32d Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 1 Apr 2026 15:18:43 +0500 Subject: [PATCH] Deprecate DOWNLOADER_CLIENTCONTEXTFACTORY, add DOWNLOAD_VERIFY_CERTIFICATES (#7379) * Deprecate DOWNLOADER_CLIENTCONTEXTFACTORY. * Deprecate BrowserLikeContextFactory. * Add DOWNLOAD_VERIFY_CERTIFICATES. * Cleanup, add acceptableCiphers to the verifying path. --- docs/news.rst | 2 +- docs/topics/components.rst | 2 - docs/topics/settings.rst | 73 ++++++++------------- scrapy/core/downloader/contextfactory.py | 61 +++++++++++++---- scrapy/core/downloader/handlers/http10.py | 18 +++-- scrapy/settings/default_settings.py | 10 +-- scrapy/utils/ssl.py | 10 ++- tests/test_downloader_handler_httpx.py | 4 ++ tests/test_downloader_handlers_http_base.py | 33 ++++++++++ 9 files changed, 141 insertions(+), 72 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 6bff95518..fd96b0b9b 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -5369,7 +5369,7 @@ Backward-incompatible changes consistency with similar classes (:issue:`3929`, :issue:`3982`) * If you are using a custom context factory - (:setting:`DOWNLOADER_CLIENTCONTEXTFACTORY`), its ``__init__`` method must + (``DOWNLOADER_CLIENTCONTEXTFACTORY``), its ``__init__`` method must accept two new parameters: ``tls_verbose_logging`` and ``tls_ciphers`` (:issue:`2111`, :issue:`3392`, :issue:`3442`, :issue:`3450`) diff --git a/docs/topics/components.rst b/docs/topics/components.rst index 56f8c6498..d8a604ea7 100644 --- a/docs/topics/components.rst +++ b/docs/topics/components.rst @@ -15,8 +15,6 @@ That includes the classes that you may assign to the following settings: - :setting:`DOWNLOAD_HANDLERS` -- :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY` - - :setting:`DOWNLOADER_MIDDLEWARES` - :setting:`DUPEFILTER_CLASS` diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index c004c253e..13735b670 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -691,35 +691,6 @@ Default: ``'scrapy.core.downloader.Downloader'`` The downloader to use for crawling. -.. setting:: DOWNLOADER_CLIENTCONTEXTFACTORY - -DOWNLOADER_CLIENTCONTEXTFACTORY -------------------------------- - -Default: ``'scrapy.core.downloader.contextfactory.ScrapyClientContextFactory'`` - -Represents the classpath to the ContextFactory to use. - -Here, "ContextFactory" is a Twisted term for SSL/TLS contexts, defining -the TLS/SSL protocol version to use, whether to do certificate verification, -or even enable client-side authentication (and various other things). - -.. note:: - - Scrapy default context factory **does NOT perform remote server - certificate verification**. This is usually fine for web scraping. - - If you do need remote server certificate verification enabled, - Scrapy also has another context factory class that you can set, - ``'scrapy.core.downloader.contextfactory.BrowserLikeContextFactory'``, - which uses the platform's certificates to validate remote endpoints. - -.. note:: - - This setting is specific to the built-in Twisted-based download handlers: - :class:`scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler` and - :class:`scrapy.core.downloader.handlers.http2.H2DownloadHandler`. - .. setting:: DOWNLOADER_CLIENT_TLS_CIPHERS DOWNLOADER_CLIENT_TLS_CIPHERS @@ -742,12 +713,7 @@ specific cipher that is not included in ``DEFAULT`` if a website requires it. Handling of this setting needs to be implemented inside the :ref:`download handler `, so it's not guaranteed to be supported - by all 3rd-party handlers. Moreover, for the built-in Twisted-based - download handlers - (:class:`scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler` and - :class:`scrapy.core.downloader.handlers.http2.H2DownloadHandler`) it needs - to be implemented in the :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY` class. - It's currently unsupported by + by all 3rd-party handlers. It's currently unsupported by :class:`~scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler`. .. setting:: DOWNLOADER_CLIENT_TLS_METHOD @@ -774,12 +740,7 @@ This setting must be one of these string values: Handling of this setting needs to be implemented inside the :ref:`download handler `, so it's not guaranteed to be supported - by all 3rd-party handlers. Moreover, for the built-in Twisted-based - download handlers - (:class:`scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler` and - :class:`scrapy.core.downloader.handlers.http2.H2DownloadHandler`) it needs - to be implemented in the :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY` class. - It's currently unsupported by + by all 3rd-party handlers. It's currently unsupported by :class:`~scrapy.core.downloader.handlers._httpx.HttpxDownloadHandler`. .. setting:: DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING @@ -798,11 +759,7 @@ the TLS-related libraries. Handling of this setting needs to be implemented inside the :ref:`download handler `, so it's not guaranteed to be supported - by all 3rd-party handlers. Moreover, for the built-in Twisted-based - download handlers - (:class:`scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler` and - :class:`scrapy.core.downloader.handlers.http2.H2DownloadHandler`) it needs - to be implemented in the :setting:`DOWNLOADER_CLIENTCONTEXTFACTORY` class. + by all 3rd-party handlers. .. setting:: DOWNLOADER_MIDDLEWARES @@ -1120,6 +1077,30 @@ Optionally, this can be set per-request basis by using the requests that use the same connection; hence, a ``ResponseFailed([InvalidBodyLengthError])`` failure is always raised for every request that was using that connection. +.. setting:: DOWNLOAD_VERIFY_CERTIFICATES + +DOWNLOAD_VERIFY_CERTIFICATES +---------------------------- + +Default: ``False`` + +Whether the HTTPS download handlers should verify the server TLS certificate +when making a request and abort the request if the verification fails. + +.. note:: + + Handling of this setting needs to be implemented inside the :ref:`download + handler `, so it's not guaranteed to be supported + by all 3rd-party handlers. The exact behavior of a handler (e.g. whether + certificate problems are logged when this setting is set to ``False``) + depends on its implementation. + +.. warning:: + Enabling this setting disables handling of + :setting:`DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING` in + :class:`~scrapy.core.downloader.handlers.http11.HTTP11DownloadHandler` and + :class:`~scrapy.core.downloader.handlers.http2.H2DownloadHandler`. + .. setting:: DUPEFILTER_CLASS DUPEFILTER_CLASS diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 8b79b4273..9b1f778e2 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -1,7 +1,7 @@ from __future__ import annotations import warnings -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from OpenSSL import SSL from twisted.internet._sslverify import _setAcceptableProtocols @@ -52,6 +52,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): tls_verbose_logging: bool = False, tls_ciphers: str | None = None, *args: Any, + verify_certificates: bool = False, **kwargs: Any, ): super().__init__(*args, **kwargs) # type: ignore[no-untyped-call] @@ -68,6 +69,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): acceptableCiphers=self.tls_ciphers, ) self._ctx = self._get_context() + self._verify_certificates = verify_certificates if method_is_overridden(type(self), ScrapyClientContextFactory, "getContext"): warnings.warn( "Overriding ScrapyClientContextFactory.getContext() is deprecated and that method" @@ -97,11 +99,13 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" ) tls_ciphers: str | None = crawler.settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] + verify_certificates = crawler.settings.getbool("DOWNLOAD_VERIFY_CERTIFICATES") return cls( # type: ignore[misc] *args, method=method, tls_verbose_logging=tls_verbose_logging, tls_ciphers=tls_ciphers, + verify_certificates=verify_certificates, **kwargs, ) @@ -129,10 +133,22 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): return ctx def creatorForNetloc(self, hostname: bytes, port: int) -> ClientTLSOptions: - return _ScrapyClientTLSOptions( - hostname.decode("ascii"), - self._ctx, - verbose_logging=self.tls_verbose_logging, + if not self._verify_certificates: + return _ScrapyClientTLSOptions( + hostname.decode("ascii"), + self._ctx, + verbose_logging=self.tls_verbose_logging, + ) + # this matches the behavior of BrowserLikeContextFactory in that it + # only uses self._ssl_method and doesn't support TLS logging or other + # features of ScrapyClientContextFactory, however it additionally + # supports self.tls_ciphers + return optionsForClientTLS( + hostname=hostname.decode("ascii"), + extraCertificateOptions={ + "method": self._ssl_method, + "acceptableCiphers": self.tls_ciphers, + }, ) @@ -158,6 +174,16 @@ class BrowserLikeContextFactory(ScrapyClientContextFactory): ``self._ssl_method`` is used from the parent class. """ + def __init__(self, *args: Any, **kwargs: Any): + warnings.warn( + "BrowserLikeContextFactory is deprecated." + " You can set DOWNLOAD_VERIFY_CERTIFICATES=True to enable" + " certificate verification instead of using it.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + super().__init__(*args, **kwargs) + def creatorForNetloc(self, hostname: bytes, port: int) -> ClientTLSOptions: return optionsForClientTLS( hostname=hostname.decode("ascii"), @@ -203,14 +229,25 @@ def _load_context_factory_from_settings(crawler: Crawler) -> IPolicyForHTTPS: Also passes values of other relevant settings to the factory class. """ + if crawler.settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] == "SENTINEL": + context_factory_cls = ScrapyClientContextFactory + else: # pragma: no cover + warnings.warn( + "The 'DOWNLOADER_CLIENTCONTEXTFACTORY' setting is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + context_factory_cls = load_object( + crawler.settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] + ) ssl_method = openssl_methods[crawler.settings.get("DOWNLOADER_CLIENT_TLS_METHOD")] - context_factory_cls = load_object( - crawler.settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] - ) - return build_from_crawler( - context_factory_cls, - crawler, - method=ssl_method, + return cast( + "IPolicyForHTTPS", + build_from_crawler( + context_factory_cls, + crawler, + method=ssl_method, + ), ) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 3820e79bd..0ce7eff3b 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -5,6 +5,7 @@ from __future__ import annotations import warnings from typing import TYPE_CHECKING +from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.misc import build_from_crawler, load_object @@ -17,7 +18,6 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy import Request - from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory from scrapy.core.downloader.webclient import ScrapyHTTPClientFactory from scrapy.crawler import Crawler from scrapy.http import Response @@ -38,9 +38,19 @@ class HTTP10DownloadHandler: self.HTTPClientFactory: type[ScrapyHTTPClientFactory] = load_object( settings["DOWNLOADER_HTTPCLIENTFACTORY"] ) - self.ClientContextFactory: type[ScrapyClientContextFactory] = load_object( - settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] - ) + if settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] == "SENTINEL": + self.ClientContextFactory: type[ScrapyClientContextFactory] = ( + ScrapyClientContextFactory + ) + else: # pragma: no cover + warnings.warn( + "The 'DOWNLOADER_CLIENTCONTEXTFACTORY' setting is deprecated.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + self.ClientContextFactory = load_object( + settings["DOWNLOADER_CLIENTCONTEXTFACTORY"] + ) self._settings: BaseSettings = settings self._crawler: Crawler = crawler diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index bc04ebcfb..a4768a4bf 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -244,10 +244,10 @@ DNSCACHE_SIZE = 10000 DNS_RESOLVER = "scrapy.resolver.CachingThreadedResolver" DNS_TIMEOUT = 60 -DOWNLOAD_DELAY = 0 - DOWNLOAD_BIND_ADDRESS = None +DOWNLOAD_DELAY = 0 + DOWNLOAD_FAIL_ON_DATALOSS = True DOWNLOAD_HANDLERS = {} @@ -265,11 +265,11 @@ DOWNLOAD_WARNSIZE = 32 * 1024 * 1024 # 32m DOWNLOAD_TIMEOUT = 180 # 3mins +DOWNLOAD_VERIFY_CERTIFICATES = False + DOWNLOADER = "scrapy.core.downloader.Downloader" -DOWNLOADER_CLIENTCONTEXTFACTORY = ( - "scrapy.core.downloader.contextfactory.ScrapyClientContextFactory" -) +DOWNLOADER_CLIENTCONTEXTFACTORY = "SENTINEL" DOWNLOADER_CLIENT_TLS_CIPHERS = "DEFAULT" # Use highest TLS/SSL protocol version supported by the platform, also allowing negotiation: DOWNLOADER_CLIENT_TLS_METHOD = "TLS" diff --git a/scrapy/utils/ssl.py b/scrapy/utils/ssl.py index 0062da52b..07c1b2ecc 100644 --- a/scrapy/utils/ssl.py +++ b/scrapy/utils/ssl.py @@ -39,10 +39,16 @@ def _make_ssl_context(settings: BaseSettings) -> ssl.SSLContext: if method_setting not in _STDLIB_PROTOCOL_MAP: raise ValueError(f"Unsupported TLS method: {method_setting}") ciphers_setting: str | None = settings["DOWNLOADER_CLIENT_TLS_CIPHERS"] + verify_setting = settings.getbool("DOWNLOAD_VERIFY_CERTIFICATES") ctx = ssl.SSLContext(_STDLIB_PROTOCOL_MAP[method_setting]) - ctx.check_hostname = False - ctx.verify_mode = ssl.CERT_NONE + if verify_setting: + ctx.check_hostname = True + ctx.verify_mode = ssl.CERT_REQUIRED + ctx.load_default_certs() + else: + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE if ciphers_setting: ctx.set_ciphers(ciphers_setting) return ctx diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index 1c11a1034..8e8e9a037 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -93,6 +93,10 @@ class TestHttp11(HttpxDownloadHandlerMixin, TestHttp11Base): class TestHttps11(HttpxDownloadHandlerMixin, TestHttps11Base): tls_log_message = "SSL connection to 127.0.0.1 using protocol TLSv1.3, cipher" + @pytest.mark.skip(reason="The check is Twisted-specific") + def test_verify_certs_deprecated(self): + pass + class TestSimpleHttps(HttpxDownloadHandlerMixin, TestSimpleHttpsBase): pass diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 88813f0b6..9a6a77560 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -25,6 +25,7 @@ from scrapy.exceptions import ( DownloadFailedError, DownloadTimeoutError, ResponseDataLossError, + ScrapyDeprecationWarning, StopDownload, UnsupportedURLSchemeError, ) @@ -729,6 +730,38 @@ class TestHttps11Base(TestHttp11Base): assert response.body == b"Works" assert self.tls_log_message in caplog.text + @coroutine_test + async def test_verify_certs_deprecated(self, mockserver: MockServer) -> None: + request = Request(mockserver.url("/text", is_secure=self.is_secure)) + with ( # noqa: PT031 + pytest.warns( + ScrapyDeprecationWarning, + match="'DOWNLOADER_CLIENTCONTEXTFACTORY' setting is deprecated", + ), + pytest.warns( + ScrapyDeprecationWarning, + match="BrowserLikeContextFactory is deprecated", + ), + ): + async with self.get_dh( + { + "DOWNLOADER_CLIENTCONTEXTFACTORY": "scrapy.core.downloader.contextfactory.BrowserLikeContextFactory" + } + ) as download_handler: + with pytest.raises( + (DownloadConnectionRefusedError, DownloadFailedError) + ): + await download_handler.download_request(request) + + @coroutine_test + async def test_verify_certs(self, mockserver: MockServer) -> None: + request = Request(mockserver.url("/text", is_secure=self.is_secure)) + async with self.get_dh( + {"DOWNLOAD_VERIFY_CERTIFICATES": True} + ) as download_handler: + with pytest.raises((DownloadConnectionRefusedError, DownloadFailedError)): + await download_handler.download_request(request) + class TestSimpleHttpsBase(ABC): """Base class for special cases tested with just one simple request"""