Merge pull request #6560 from wRAR/tls-method-tests

Add tests for DOWNLOADER_CLIENT_TLS_METHOD, remove dead code.

scrapy/core/downloader/contextfactory.py

@@ -111,18 +111,9 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS):
     def getCertificateOptions(self) -> CertificateOptions:
         # setting verify=True will require you to provide CAs
         # to verify against; in other words: it's not that simple
-
-        # backward-compatible SSL/TLS method:
-        #
-        # * this will respect `method` attribute in often recommended
-        #   `ScrapyClientContextFactory` subclass
-        #   (https://github.com/scrapy/scrapy/issues/1429#issuecomment-131782133)
-        #
-        # * getattr() for `_ssl_method` attribute for context factories
-        #   not calling super().__init__
         return CertificateOptions(
             verify=False,
-            method=getattr(self, "method", getattr(self, "_ssl_method", None)),
+            method=self._ssl_method,
             fixBrokenPeers=True,
             acceptableCiphers=self.tls_ciphers,
         )

tests/test_webclient.py

@@ -3,21 +3,29 @@ from twisted.internet import defer
 Tests borrowed from the twisted.web.client tests.
 """
 
+from __future__ import annotations
+
 import shutil
 from pathlib import Path
 from tempfile import mkdtemp
+from typing import Any
 
 import OpenSSL.SSL
+from pytest import raises
 from twisted.internet import defer, reactor
-from twisted.internet.defer import inlineCallbacks
+from twisted.internet.defer import Deferred, inlineCallbacks
 from twisted.internet.testing import StringTransport
 from twisted.protocols.policies import WrappingFactory
 from twisted.trial import unittest
 from twisted.web import resource, server, static, util
 
 from scrapy.core.downloader import webclient as client
-from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory
+from scrapy.core.downloader.contextfactory import (
+    ScrapyClientContextFactory,
+    load_context_factory_from_settings,
+)
 from scrapy.http import Headers, Request
+from scrapy.settings import Settings
 from scrapy.utils.misc import build_from_crawler
 from scrapy.utils.python import to_bytes, to_unicode
 from scrapy.utils.test import get_crawler
@@ -482,3 +490,51 @@ class WebClientCustomCiphersSSLTestCase(WebClientSSLTestCase):
             self.getURL("payload"), body=s, contextFactory=client_context_factory
         )
         return self.assertFailure(d, OpenSSL.SSL.Error)
+
+
+class WebClientTLSMethodTestCase(WebClientSSLTestCase):
+    def _assert_factory_works(
+        self, client_context_factory: ScrapyClientContextFactory
+    ) -> Deferred[Any]:
+        s = "0123456789" * 10
+        return getPage(
+            self.getURL("payload"), body=s, contextFactory=client_context_factory
+        ).addCallback(self.assertEqual, to_bytes(s))
+
+    def test_setting_default(self):
+        crawler = get_crawler()
+        settings = Settings()
+        client_context_factory = load_context_factory_from_settings(settings, crawler)
+        assert client_context_factory._ssl_method == OpenSSL.SSL.SSLv23_METHOD
+        return self._assert_factory_works(client_context_factory)
+
+    def test_setting_none(self):
+        crawler = get_crawler()
+        settings = Settings({"DOWNLOADER_CLIENT_TLS_METHOD": None})
+        with raises(KeyError):
+            load_context_factory_from_settings(settings, crawler)
+
+    def test_setting_bad(self):
+        crawler = get_crawler()
+        settings = Settings({"DOWNLOADER_CLIENT_TLS_METHOD": "bad"})
+        with raises(KeyError):
+            load_context_factory_from_settings(settings, crawler)
+
+    def test_setting_explicit(self):
+        crawler = get_crawler()
+        settings = Settings({"DOWNLOADER_CLIENT_TLS_METHOD": "TLSv1.2"})
+        client_context_factory = load_context_factory_from_settings(settings, crawler)
+        assert client_context_factory._ssl_method == OpenSSL.SSL.TLSv1_2_METHOD
+        return self._assert_factory_works(client_context_factory)
+
+    def test_direct_from_crawler(self):
+        # the setting is ignored
+        crawler = get_crawler(settings_dict={"DOWNLOADER_CLIENT_TLS_METHOD": "bad"})
+        client_context_factory = build_from_crawler(ScrapyClientContextFactory, crawler)
+        assert client_context_factory._ssl_method == OpenSSL.SSL.SSLv23_METHOD
+        return self._assert_factory_works(client_context_factory)
+
+    def test_direct_init(self):
+        client_context_factory = ScrapyClientContextFactory(OpenSSL.SSL.TLSv1_2_METHOD)
+        assert client_context_factory._ssl_method == OpenSSL.SSL.TLSv1_2_METHOD
+        return self._assert_factory_works(client_context_factory)