From d27c6b46b11c2ceaa61b35372ad8a3c98185aa18 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 27 Jan 2025 21:25:47 +0500 Subject: [PATCH 1/4] Deprecate HTTP/1.0 support. --- scrapy/core/downloader/handlers/http10.py | 7 +++++++ scrapy/core/downloader/webclient.py | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 58f7ad577..0fbe5fc23 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -2,8 +2,10 @@ from __future__ import annotations +import warnings from typing import TYPE_CHECKING +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.misc import build_from_crawler, load_object from scrapy.utils.python import to_unicode @@ -26,6 +28,11 @@ class HTTP10DownloadHandler: lazy = False def __init__(self, settings: BaseSettings, crawler: Crawler): + warnings.warn( + "HTTP10DownloadHandler is deprecated and will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) self.HTTPClientFactory: type[ScrapyHTTPClientFactory] = load_object( settings["DOWNLOADER_HTTPCLIENTFACTORY"] ) diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index ee10ae73b..aaaf68152 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -1,6 +1,7 @@ from __future__ import annotations import re +import warnings from time import time from typing import TYPE_CHECKING from urllib.parse import ParseResult, urldefrag, urlparse, urlunparse @@ -9,6 +10,7 @@ from twisted.internet import defer from twisted.internet.protocol import ClientFactory from twisted.web.http import HTTPClient +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Headers, Response from scrapy.responsetypes import responsetypes from scrapy.utils.httpobj import urlparse_cached @@ -49,6 +51,14 @@ def _parse(url: str) -> tuple[bytes, bytes, bytes, int, bytes]: class ScrapyHTTPPageGetter(HTTPClient): delimiter = b"\n" + def __init__(self): + warnings.warn( + "ScrapyHTTPPageGetter is deprecated and will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + super().__init__() + def connectionMade(self): self.headers = Headers() # bucket for response headers @@ -140,6 +150,12 @@ class ScrapyHTTPClientFactory(ClientFactory): self.path = self.url def __init__(self, request: Request, timeout: float = 180): + warnings.warn( + "ScrapyHTTPClientFactory is deprecated and will be removed in a future Scrapy version.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) + self._url: str = urldefrag(request.url)[0] # converting to bytes to comply to Twisted interface self.url: bytes = to_bytes(self._url, encoding="ascii") From 16b998f9ca8b928b03f9f8be659ac01d4f2f623f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 Jan 2025 01:33:00 +0500 Subject: [PATCH 2/4] Sort out webclient tests. --- scrapy/core/downloader/contextfactory.py | 1 + scrapy/core/downloader/webclient.py | 2 + tests/test_core_downloader.py | 134 +++++++++++++++++++++++ tests/test_downloader_handlers.py | 2 + tests/test_webclient.py | 89 ++------------- 5 files changed, 146 insertions(+), 82 deletions(-) diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index d44c663bb..b01ee97f3 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -121,6 +121,7 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): # kept for old-style HTTP/1.0 downloader context twisted calls, # e.g. connectSSL() def getContext(self, hostname: Any = None, port: Any = None) -> SSL.Context: + # FIXME ctx: SSL.Context = self.getCertificateOptions().getContext() ctx.set_options(0x4) # OP_LEGACY_SERVER_CONNECT return ctx diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index aaaf68152..09751ea1a 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -1,3 +1,5 @@ +"""Deprecated HTTP/1.0 helper classes used by HTTP10DownloadHandler.""" + from __future__ import annotations import re diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index d929a9369..0a0c0a4f0 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -1,6 +1,31 @@ +from __future__ import annotations + +import shutil +from pathlib import Path +from tempfile import mkdtemp + +import OpenSSL.SSL +import pytest +from twisted.internet import reactor +from twisted.internet.defer import inlineCallbacks +from twisted.protocols.policies import WrappingFactory from twisted.trial import unittest +from twisted.web import server, static +from twisted.web.client import Agent, BrowserLikePolicyForHTTPS, readBody +from twisted.web.client import Response as TxResponse from scrapy.core.downloader import Slot +from scrapy.core.downloader.contextfactory import ( + ScrapyClientContextFactory, + load_context_factory_from_settings, +) +from scrapy.core.downloader.handlers.http11 import _RequestBodyProducer +from scrapy.settings import Settings +from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future +from scrapy.utils.misc import build_from_crawler +from scrapy.utils.python import to_bytes +from scrapy.utils.test import get_crawler +from tests.mockserver import PayloadResource, ssl_context_factory class SlotTest(unittest.TestCase): @@ -10,3 +35,112 @@ class SlotTest(unittest.TestCase): repr(slot), "Slot(concurrency=8, delay=0.10, randomize_delay=True)", ) + + +class ContextFactoryBaseTestCase(unittest.TestCase): + context_factory = None + + def _listen(self, site): + return reactor.listenSSL( + 0, + site, + contextFactory=self.context_factory or ssl_context_factory(), + interface="127.0.0.1", + ) + + def getURL(self, path): + return f"https://127.0.0.1:{self.portno}/{path}" + + def setUp(self): + self.tmpname = Path(mkdtemp()) + (self.tmpname / "file").write_bytes(b"0123456789") + r = static.File(str(self.tmpname)) + r.putChild(b"payload", PayloadResource()) + self.site = server.Site(r, timeout=None) + self.wrapper = WrappingFactory(self.site) + self.port = self._listen(self.wrapper) + self.portno = self.port.getHost().port + + @inlineCallbacks + def tearDown(self): + yield self.port.stopListening() + shutil.rmtree(self.tmpname) + + @staticmethod + async def get_page( + url: str, + client_context_factory: BrowserLikePolicyForHTTPS, + body: str | None = None, + ) -> bytes: + agent = Agent(reactor, contextFactory=client_context_factory) + body_producer = _RequestBodyProducer(body.encode()) if body else None + response: TxResponse = await maybe_deferred_to_future( + agent.request(b"GET", url.encode(), bodyProducer=body_producer) + ) + return await maybe_deferred_to_future(readBody(response)) # type: ignore[arg-type] + + +class ContextFactoryTestCase(ContextFactoryBaseTestCase): + @deferred_f_from_coro_f + async def testPayload(self): + s = "0123456789" * 10 + crawler = get_crawler() + settings = Settings() + client_context_factory = load_context_factory_from_settings(settings, crawler) + body = await self.get_page( + self.getURL("payload"), client_context_factory, body=s + ) + self.assertEqual(body, to_bytes(s)) + + +class ContextFactoryTLSMethodTestCase(ContextFactoryBaseTestCase): + async def _assert_factory_works( + self, client_context_factory: ScrapyClientContextFactory + ) -> None: + s = "0123456789" * 10 + body = await self.get_page( + self.getURL("payload"), client_context_factory, body=s + ) + self.assertEqual(body, to_bytes(s)) + + @deferred_f_from_coro_f + async 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 + await self._assert_factory_works(client_context_factory) + + def test_setting_none(self): + crawler = get_crawler() + settings = Settings({"DOWNLOADER_CLIENT_TLS_METHOD": None}) + with pytest.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 pytest.raises(KeyError): + load_context_factory_from_settings(settings, crawler) + + @deferred_f_from_coro_f + async 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 + await self._assert_factory_works(client_context_factory) + + @deferred_f_from_coro_f + async 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 + await self._assert_factory_works(client_context_factory) + + @deferred_f_from_coro_f + async 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 + await self._assert_factory_works(client_context_factory) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 0dcbeaec1..64f615bfe 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -422,6 +422,7 @@ class HttpTestCase(unittest.TestCase): return self.download_request(request, Spider("foo")).addCallback(_test) +@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class Http10TestCase(HttpTestCase): """HTTP 1.0 test case""" @@ -780,6 +781,7 @@ class HttpProxyTestCase(unittest.TestCase): return self.download_request(request, Spider("foo")).addCallback(_test) +@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class Http10ProxyTestCase(HttpProxyTestCase): download_handler_cls: type = HTTP10DownloadHandler diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 0a594aa7c..fa19b350b 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -8,12 +8,11 @@ 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 +import pytest from twisted.internet import defer, reactor -from twisted.internet.defer import Deferred, inlineCallbacks +from twisted.internet.defer import inlineCallbacks from twisted.internet.testing import StringTransport from twisted.protocols.policies import WrappingFactory from twisted.trial import unittest @@ -22,10 +21,8 @@ from twisted.web import resource, server, static, util from scrapy.core.downloader import webclient as client 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 @@ -38,6 +35,7 @@ from tests.mockserver import ( PayloadResource, ssl_context_factory, ) +from tests.test_core_downloader import ContextFactoryBaseTestCase def getPage(url, contextFactory=None, response_transform=None, *args, **kwargs): @@ -129,6 +127,7 @@ class ParseUrlTestCase(unittest.TestCase): self.assertEqual(client._parse(url), test, url) +@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class ScrapyHTTPPageGetterTests(unittest.TestCase): def test_earlyHeaders(self): # basic test stolen from twisted HTTPageGetter @@ -272,6 +271,7 @@ class EncodingResource(resource.Resource): return body.encode(self.out_encoding) +@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class WebClientTestCase(unittest.TestCase): def _listen(self, site): return reactor.listenTCP(0, site, interface="127.0.0.1") @@ -427,35 +427,8 @@ class WebClientTestCase(unittest.TestCase): ) -class WebClientSSLTestCase(unittest.TestCase): - context_factory = None - - def _listen(self, site): - return reactor.listenSSL( - 0, - site, - contextFactory=self.context_factory or ssl_context_factory(), - interface="127.0.0.1", - ) - - def getURL(self, path): - return f"https://127.0.0.1:{self.portno}/{path}" - - def setUp(self): - self.tmpname = Path(mkdtemp()) - (self.tmpname / "file").write_bytes(b"0123456789") - r = static.File(str(self.tmpname)) - r.putChild(b"payload", PayloadResource()) - self.site = server.Site(r, timeout=None) - self.wrapper = WrappingFactory(self.site) - self.port = self._listen(self.wrapper) - self.portno = self.port.getHost().port - - @inlineCallbacks - def tearDown(self): - yield self.port.stopListening() - shutil.rmtree(self.tmpname) - +@pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") +class WebClientSSLTestCase(ContextFactoryBaseTestCase): def testPayload(self): s = "0123456789" * 10 return getPage(self.getURL("payload"), body=s).addCallback( @@ -490,51 +463,3 @@ 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) From bc1aeeefc970fbd123699e0cb6d8486141bf8418 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 Jan 2025 01:54:43 +0500 Subject: [PATCH 3/4] Deprecate overriding ScrapyClientContextFactory.getContext(). --- scrapy/core/downloader/contextfactory.py | 9 ++++++++- tests/test_core_downloader.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index b01ee97f3..d1ba6208a 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -22,6 +22,7 @@ from scrapy.core.downloader.tls import ( openssl_methods, ) from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.utils.deprecate import method_is_overridden from scrapy.utils.misc import build_from_crawler, load_object if TYPE_CHECKING: @@ -62,6 +63,13 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): self.tls_ciphers = AcceptableCiphers.fromOpenSSLCipherString(tls_ciphers) else: self.tls_ciphers = DEFAULT_CIPHERS + if method_is_overridden(type(self), ScrapyClientContextFactory, "getContext"): + warnings.warn( + "Overriding ScrapyClientContextFactory.getContext() is deprecated and that method" + " will be removed in a future Scrapy version. Override creatorForNetloc() instead.", + category=ScrapyDeprecationWarning, + stacklevel=2, + ) @classmethod def from_settings( @@ -121,7 +129,6 @@ class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): # kept for old-style HTTP/1.0 downloader context twisted calls, # e.g. connectSSL() def getContext(self, hostname: Any = None, port: Any = None) -> SSL.Context: - # FIXME ctx: SSL.Context = self.getCertificateOptions().getContext() ctx.set_options(0x4) # OP_LEGACY_SERVER_CONNECT return ctx diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index 0a0c0a4f0..e67337fc7 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -1,8 +1,10 @@ from __future__ import annotations import shutil +import warnings from pathlib import Path from tempfile import mkdtemp +from typing import Any import OpenSSL.SSL import pytest @@ -92,6 +94,22 @@ class ContextFactoryTestCase(ContextFactoryBaseTestCase): ) self.assertEqual(body, to_bytes(s)) + def test_override_getContext(self): + class MyFactory(ScrapyClientContextFactory): + def getContext( + self, hostname: Any = None, port: Any = None + ) -> OpenSSL.SSL.Context: + ctx: OpenSSL.SSL.Context = super().getContext(hostname, port) + return ctx + + with warnings.catch_warnings(record=True) as w: + MyFactory() + self.assertEqual(len(w), 1) + self.assertIn( + "Overriding ScrapyClientContextFactory.getContext() is deprecated", + str(w[0].message), + ) + class ContextFactoryTLSMethodTestCase(ContextFactoryBaseTestCase): async def _assert_factory_works( From 0d2d2892badb2b6f47c9b597564510871c7f1518 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 Jan 2025 02:08:49 +0500 Subject: [PATCH 4/4] Silence the readBody warning. --- tests/test_core_downloader.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_core_downloader.py b/tests/test_core_downloader.py index e67337fc7..dffba303f 100644 --- a/tests/test_core_downloader.py +++ b/tests/test_core_downloader.py @@ -9,7 +9,7 @@ from typing import Any import OpenSSL.SSL import pytest from twisted.internet import reactor -from twisted.internet.defer import inlineCallbacks +from twisted.internet.defer import Deferred, inlineCallbacks from twisted.protocols.policies import WrappingFactory from twisted.trial import unittest from twisted.web import server, static @@ -79,7 +79,15 @@ class ContextFactoryBaseTestCase(unittest.TestCase): response: TxResponse = await maybe_deferred_to_future( agent.request(b"GET", url.encode(), bodyProducer=body_producer) ) - return await maybe_deferred_to_future(readBody(response)) # type: ignore[arg-type] + with warnings.catch_warnings(): + # https://github.com/twisted/twisted/issues/8227 + warnings.filterwarnings( + "ignore", + category=DeprecationWarning, + message=r".*does not have an abortConnection method", + ) + d: Deferred[bytes] = readBody(response) # type: ignore[arg-type] + return await maybe_deferred_to_future(d) class ContextFactoryTestCase(ContextFactoryBaseTestCase):