mirror of https://github.com/scrapy/scrapy.git
Deprecate DOWNLOADER_CLIENTCONTEXTFACTORY, add DOWNLOAD_VERIFY_CERTIFICATES (#7379)
* Deprecate DOWNLOADER_CLIENTCONTEXTFACTORY. * Deprecate BrowserLikeContextFactory. * Add DOWNLOAD_VERIFY_CERTIFICATES. * Cleanup, add acceptableCiphers to the verifying path.
This commit is contained in:
parent
72bcf8cb46
commit
eabb149f4b
|
|
@ -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`)
|
||||
|
||||
|
|
|
|||
|
|
@ -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`
|
||||
|
|
|
|||
|
|
@ -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 <topics-download-handlers>`, 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 <topics-download-handlers>`, 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 <topics-download-handlers>`, 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 <topics-download-handlers>`, 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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"""
|
||||
|
|
|
|||
Loading…
Reference in New Issue