From 4cc23566377f83665b615837b1a3178b157d25d0 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 10:59:44 +0200 Subject: [PATCH 1/7] Implement browser-like bad header handling for the default download handler (#7806) * Implement browser-like bad header handling for the default download handler * Remove dead code --- docs/topics/download-handlers.rst | 35 ++++++--- scrapy/core/downloader/handlers/http11.py | 86 ++++++++++++++++++++- tests/mockserver/http.py | 2 + tests/mockserver/http_resources.py | 33 ++++++++ tests/test_downloader_handler_httpx.py | 2 + tests/utils/bases/download_handlers_http.py | 39 +++++++++- 6 files changed, 183 insertions(+), 14 deletions(-) diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 94e75ab6f..433a6d139 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -130,17 +130,23 @@ using different handlers. Here is a comparison of some features of the built-in HTTP handlers, see the individual handler docs for more differences: -================== ================= ===================== ==================== -Feature H2DownloadHandler HTTP11DownloadHandler HttpxDownloadHandler -================== ================= ===================== ==================== -Requires asyncio No No Yes -Requires a reactor Yes Yes No -HTTP/1.1 No Yes Yes -HTTP/2 Yes No Yes -TLS implementation ``cryptography`` ``cryptography`` Stdlib ``ssl`` -HTTP proxies No Yes Yes -SOCKS proxies No No Yes -================== ================= ===================== ==================== +=================== ================= ===================== ==================== +Feature H2DownloadHandler HTTP11DownloadHandler HttpxDownloadHandler +=================== ================= ===================== ==================== +Requires asyncio No No Yes +Requires a reactor Yes Yes No +HTTP/1.1 No Yes Yes +HTTP/2 Yes No Yes +TLS implementation ``cryptography`` ``cryptography`` Stdlib ``ssl`` +HTTP proxies No Yes Yes +SOCKS proxies No No Yes +Bad header handling Not applicable Skip bad Fail +=================== ================= ===================== ==================== + +Bad header handling is what a handler does when a response has a bad header +line, e.g. one with no colon in it, which some servers send. Handlers that skip +bad header lines, like web browsers do, still parse the header lines that follow +them; other handlers also lose those, or cannot download such responses at all. You can find additional HTTP download handlers in the scrapy-download-handlers-incubator_ package. This package is made by the Scrapy @@ -191,6 +197,7 @@ Features and limitations HTTP proxies No (not implemented) SOCKS proxies No (not supported by the library) HTTP/2 Yes +Bad header handling Not applicable (HTTP/2 only) ``response.certificate`` :class:`twisted.internet.ssl.Certificate` object Per-request ``bindaddress`` Yes TLS implementation ``pyOpenSSL``/``cryptography`` @@ -239,11 +246,16 @@ Features and limitations HTTP proxies Yes SOCKS proxies No (not supported by the library) HTTP/2 No (implemented as a separate handler) +Bad header handling Skip bad, like web browsers do ``response.certificate`` :class:`twisted.internet.ssl.Certificate` object Per-request ``bindaddress`` Yes TLS implementation ``pyOpenSSL``/``cryptography`` =========================== ================================================ +.. versionchanged:: VERSION + Bad header lines with no colon in them are now skipped, instead of making + the whole response impossible to download. + Other limitations: - IPv6 support requires setting :setting:`TWISTED_DNS_RESOLVER` @@ -297,6 +309,7 @@ Features and limitations HTTP proxies Yes SOCKS proxies Yes (SOCKS5) HTTP/2 Yes +Bad header handling Fail (not supported by the library) ``response.certificate`` DER bytes Per-request ``bindaddress`` No (not supported by the library) TLS implementation Standard library ``ssl`` diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 3b5c08666..c288ae792 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -17,12 +17,19 @@ from twisted.internet.defer import Deferred, succeed from twisted.internet.endpoints import TCP4ClientEndpoint from twisted.internet.protocol import Factory, Protocol, connectionDone from twisted.python.failure import Failure +from twisted.web._newclient import ( + HEADER, + STATUS, + HTTP11ClientProtocol, + HTTPClientParser, +) from twisted.web.client import ( URI, Agent, HTTPConnectionPool, ResponseDone, ResponseFailed, + _HTTP11ClientFactory, ) from twisted.web.client import Response as TxResponse from twisted.web.http import PotentialDataLoss, _DataLoss @@ -60,7 +67,8 @@ from ._base_http import BaseHttpDownloadHandler if TYPE_CHECKING: from twisted.internet.base import ReactorBase - from twisted.internet.interfaces import IConsumer + from twisted.internet.interfaces import IAddress, IConsumer + from twisted.web._newclient import Request as TxRequest # typing.NotRequired requires Python 3.11 from typing_extensions import NotRequired @@ -95,7 +103,7 @@ class HTTP11DownloadHandler(BaseHttpDownloadHandler): self._pool.maxPersistentPerHost = crawler.settings.getint( "CONCURRENT_REQUESTS_PER_DOMAIN" ) - self._pool._factory.noisy = False + self._pool._factory = _LenientHTTP11ClientFactory self._contextFactory: IPolicyForHTTPS = _load_context_factory_from_settings( crawler @@ -740,3 +748,77 @@ class _ResponseReader(Protocol): reason = Failure(exc) self._finished.errback(reason) + + +class _LenientHTTPClientParser(HTTPClientParser): + """Response parser that skips bad response header lines, those with no + colon in them, instead of failing to parse the whole response. + + Some servers send such lines, and web browsers skip them and keep parsing + the header lines that follow. See + https://github.com/scrapy/scrapy/issues/210. + """ + + def lineReceived(self, line: bytes) -> None: + # A copy of twisted.web._newclient.HTTPParser.lineReceived() where the + # header name and value are only extracted from header lines that have + # a colon. + + # Handle the normal CR LF case. + if line[-1:] == b"\r": + line = line[:-1] + + if self.state == STATUS: + self.statusReceived(line) # type: ignore[no-untyped-call] + self.state = HEADER + return + + # HEADER is the only other state in which lines are received, as the + # parser switches to raw mode for the response body. + if not line or line[0] not in b" \t": + if self._partialHeader is not None: + header = b"".join(self._partialHeader) + if b":" in header: + name, value = header.split(b":", 1) + self.headerReceived(name, value.strip()) # type: ignore[no-untyped-call] + else: + logger.debug( + f"Skipping the bad response header line {header!r}, as " + f"it has no colon." + ) + if not line: + # Empty line means the header section is over. + self.allHeadersReceived() # type: ignore[no-untyped-call] + else: + # Line not beginning with LWS is another header. + self._partialHeader = [line] + else: + # A line beginning with LWS is a continuation of a header begun on + # a previous line. + self._partialHeader.append(line) # type: ignore[union-attr] + + +class _LenientHTTP11ClientProtocol(HTTP11ClientProtocol): + """Protocol that parses responses with :class:`_LenientHTTPClientParser`.""" + + def request(self, request: TxRequest) -> Deferred[IResponse]: + d: Deferred[IResponse] = super().request(request) + # HTTP11ClientProtocol.request() hardcodes the parser class, so the + # only way to use a different one is to replace the class of the parser + # object that it creates. This is safe because + # _LenientHTTPClientParser defines no additional state. The parser is + # always there because HTTPConnectionPool only reuses connections whose + # protocol is in the QUIESCENT state, for which request() always + # creates a parser. + assert self._parser is not None + self._parser.__class__ = _LenientHTTPClientParser + return d + + +class _LenientHTTP11ClientFactory(_HTTP11ClientFactory): + """Factory that builds :class:`_LenientHTTP11ClientProtocol` protocols.""" + + noisy = False + + def buildProtocol(self, addr: IAddress | None) -> HTTP11ClientProtocol: + return _LenientHTTP11ClientProtocol(self._quiescentCallback) # type: ignore[no-untyped-call] diff --git a/tests/mockserver/http.py b/tests/mockserver/http.py index c4fd4464e..6074f1475 100644 --- a/tests/mockserver/http.py +++ b/tests/mockserver/http.py @@ -11,6 +11,7 @@ from tests import tests_datadir from .http_base import BaseMockServer, main_factory from .http_resources import ( ArbitraryLengthPayloadResource, + BadHeader, BaseResource, BrokenChunkedResource, BrokenDownloadResource, @@ -52,6 +53,7 @@ class Root(BaseResource): put_child(self, b"partial", Partial()) put_child(self, b"drop", Drop()) put_child(self, b"raw", Raw()) + put_child(self, b"bad-header", BadHeader()) put_child(self, b"echo", Echo()) put_child(self, b"payload", PayloadResource()) put_child(self, b"alpayload", ArbitraryLengthPayloadResource()) diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index cb028bc10..969e29a8c 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -210,6 +210,39 @@ class Raw(LeafResource): request.finish() +class BadHeader(LeafResource): + """Sends a response with a bad header line, one with no colon in it, like + some servers do, between two good ones. + + One of the good header lines is split into two lines, so that handling of + such headers is also covered. + """ + + response = ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: 5\r\n" + b"Content-Type: text/html\r\n" + b"X-Folded-Header: one\r\n" + b"\ttwo\r\n" + b'\r\n' + b"X-After-Bad-Header: works\r\n" + b"\r\n" + b"Works" + ) + + def render_GET(self, request: Request) -> int: + request.startedWriting = 1 + self.deferRequest(request, 0, self._delayedRender, request) + return NOT_DONE_YET + + def _delayedRender(self, request: Request) -> None: + request.write(self.response) + # Clients that stop parsing headers at the bad one don't get + # Content-Length, so they need the connection to be closed to know that + # the response body is over. + close_connection(request) + + class Echo(LeafResource): def render_GET(self, request: Request) -> bytes: assert request.content diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index 976daacaf..27a44227d 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -61,6 +61,7 @@ class HttpxDownloadHandlerMixin: class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): handler_supports_bindaddress_meta = False + handler_bad_header_handling = "fail" @pytest.mark.skipif( sys.platform == "darwin", @@ -82,6 +83,7 @@ class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): class TestHttps(HttpxDownloadHandlerMixin, TestHttpsBase): handler_supports_bindaddress_meta = False + handler_bad_header_handling = "fail" tls_log_message = "SSL connection to 127.0.0.1 using protocol TLSv1.3, cipher" @pytest.mark.skip(reason="The check is Twisted-specific") diff --git a/tests/utils/bases/download_handlers_http.py b/tests/utils/bases/download_handlers_http.py index 65244b938..9b4a38724 100644 --- a/tests/utils/bases/download_handlers_http.py +++ b/tests/utils/bases/download_handlers_http.py @@ -11,7 +11,7 @@ from contextlib import asynccontextmanager from http import HTTPStatus from ipaddress import IPv4Address from socket import gethostbyname -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Literal from urllib.parse import urlparse import pytest @@ -60,6 +60,9 @@ if TYPE_CHECKING: from tests.mockserver.http import MockServer +BadHeaderHandling = Literal["skip-bad", "skip-rest", "fail"] + + class TestHttpBase(ABC): is_secure: bool = False http2: bool = False @@ -72,6 +75,14 @@ class TestHttpBase(ABC): # h2.connection.H2Connection.receive_data()), thus closing all streams that # were using it, and we handle this as a normal exception. handler_supports_http2_dataloss: bool = True + # What the handler does with a bad response header line, e.g. one with no + # colon in it: + # "skip-bad": the bad line is skipped and the header lines that follow it + # are still parsed, which is what web browsers do; + # "skip-rest": the bad line is skipped along with the header lines that + # follow it; + # "fail": the response cannot be downloaded at all. + handler_bad_header_handling: BadHeaderHandling = "skip-bad" # default headers added by the underlying library that cannot be suppressed always_present_req_headers: ClassVar[frozenset[str]] = frozenset() default_handler_settings: ClassVar[dict[str, Any]] = {} @@ -645,6 +656,32 @@ class TestHttpBase(ABC): in caplog.text ) + @coroutine_test + async def test_download_bad_header(self, mockserver: MockServer) -> None: + if self.http2: + pytest.skip("Header lines are specific to HTTP/1.x") + request = Request(mockserver.url("/bad-header", is_secure=self.is_secure)) + async with self.get_dh() as download_handler: + if self.handler_bad_header_handling == "fail": + with pytest.raises(DownloadFailedError): + await download_handler.download_request(request) + return + response = await download_handler.download_request(request) + assert response.status == 200 + assert response.body == b"Works" + # the header line that precedes the bad one + assert response.headers.get(b"Content-Type") == b"text/html" + # the header split into two lines, also before the bad one + folded_header = response.headers.get(b"X-Folded-Header") + assert folded_header is not None + # the separator between both parts depends on the handler + assert folded_header.split() == [b"one", b"two"] + # the header line that follows the bad one + expected_value = ( + b"works" if self.handler_bad_header_handling == "skip-bad" else None + ) + assert response.headers.get(b"X-After-Bad-Header") == expected_value + @coroutine_test async def test_download_chunked_content(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/chunked", is_secure=self.is_secure)) From fe96c1f54b8f635e9274a854214b07887b9d8d61 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 11:07:42 +0200 Subject: [PATCH 2/7] Serialize dates and times as ISO 8601 in ScrapyJSONEncoder (#7918) --- docs/topics/extensions.rst | 4 ++-- scrapy/utils/serialize.py | 11 ++--------- tests/test_exporters.py | 4 ++-- tests/test_utils_serialize.py | 10 +++++++++- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/docs/topics/extensions.rst b/docs/topics/extensions.rst index 78b38cc3f..6d61cc342 100644 --- a/docs/topics/extensions.rst +++ b/docs/topics/extensions.rst @@ -374,8 +374,8 @@ This extension periodically logs rich stat data as a JSON object:: "elapsed": 360.008903, "log_interval": 60.0, "log_interval_real": 60.006694, - "start_time": "2023-08-03 23:24:57", - "utcnow": "2023-08-03 23:30:57" + "start_time": "2023-08-03T23:24:57.148903+00:00", + "utcnow": "2023-08-03T23:30:57.157806+00:00" } } diff --git a/scrapy/utils/serialize.py b/scrapy/utils/serialize.py index 5d06bbe30..803b63bef 100644 --- a/scrapy/utils/serialize.py +++ b/scrapy/utils/serialize.py @@ -10,18 +10,11 @@ from scrapy.http import Request, Response class ScrapyJSONEncoder(json.JSONEncoder): - DATE_FORMAT = "%Y-%m-%d" - TIME_FORMAT = "%H:%M:%S" - def default(self, o: Any) -> Any: if isinstance(o, set): return list(o) - if isinstance(o, datetime.datetime): - return o.strftime(f"{self.DATE_FORMAT} {self.TIME_FORMAT}") - if isinstance(o, datetime.date): - return o.strftime(self.DATE_FORMAT) - if isinstance(o, datetime.time): - return o.strftime(self.TIME_FORMAT) + if isinstance(o, (datetime.datetime, datetime.date, datetime.time)): + return o.isoformat() if isinstance(o, decimal.Decimal): return str(o) if isinstance(o, defer.Deferred): diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 956697a04..4359f4ff4 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -578,7 +578,7 @@ class TestJsonLinesItemExporter(TestBaseItemExporter): self.ie.finish_exporting() del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) - item["time"] = str(item["time"]) + item["time"] = item["time"].isoformat() assert exported == item @@ -661,7 +661,7 @@ class TestJsonItemExporter(TestJsonLinesItemExporter): self.ie.finish_exporting() del self.ie # See the first “del self.ie” in this file for context. exported = json.loads(to_unicode(self.output.getvalue())) - item["time"] = str(item["time"]) + item["time"] = item["time"].isoformat() assert exported == [item] diff --git a/tests/test_utils_serialize.py b/tests/test_utils_serialize.py index 2702c2cce..6becad13e 100644 --- a/tests/test_utils_serialize.py +++ b/tests/test_utils_serialize.py @@ -20,11 +20,17 @@ class TestJsonEncoder: def test_encode_decode(self, encoder: ScrapyJSONEncoder) -> None: dt = datetime.datetime(2010, 1, 2, 10, 11, 12) - dts = "2010-01-02 10:11:12" + dts = "2010-01-02T10:11:12" + dt_aware = datetime.datetime( + 2010, 1, 2, 10, 11, 12, 133700, tzinfo=datetime.timezone.utc + ) + dt_awares = "2010-01-02T10:11:12.133700+00:00" d = datetime.date(2010, 1, 2) ds = "2010-01-02" t = datetime.time(10, 11, 12) ts = "10:11:12" + t_us = datetime.time(10, 11, 12, 133700) + t_uss = "10:11:12.133700" dec = Decimal("1000.12") decs = "1000.12" s = {"foo"} @@ -36,7 +42,9 @@ class TestJsonEncoder: ("foo", "foo"), (d, ds), (t, ts), + (t_us, t_uss), (dt, dts), + (dt_aware, dt_awares), (dec, decs), (["foo", d], ["foo", ds]), (s, ss), From f1694269d8c9437461936ed449e81aa2ccb3b7cb Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 11:33:58 +0200 Subject: [PATCH 3/7] Add FTPS support to the FTP feed export storage (#7953) --- docs/topics/feed-exports.rst | 32 +++++++++++++++++++++++++++-- docs/topics/settings.rst | 2 +- scrapy/extensions/feedexport.py | 2 ++ scrapy/settings/default_settings.py | 1 + scrapy/utils/ftp.py | 16 +++++++++++---- tests/keys/__init__.py | 6 +++++- tests/mockserver/ftp.py | 32 +++++++++++++++++++++-------- tests/test_feedexport_storages.py | 19 +++++++++++++++++ 8 files changed, 93 insertions(+), 17 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 2f686fd0f..467abc989 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -104,7 +104,8 @@ storage backend types which are defined by the URI scheme. The storages backends supported out of the box are: - :ref:`topics-feed-storage-fs` -- :ref:`topics-feed-storage-ftp` +- :ref:`feed-storage-ftp` +- :ref:`feed-storage-ftps` - :ref:`topics-feed-storage-s3` (requires the :ref:`s3 ` extra) - :ref:`topics-feed-storage-gcs` (requires the :ref:`gcs ` extra) - :ref:`topics-feed-storage-stdout` @@ -168,6 +169,7 @@ you specify a path (e.g. ``/tmp/export.csv``). Alternatively you can also use a :class:`pathlib.Path` object. .. _topics-feed-storage-ftp: +.. _feed-storage-ftp: FTP --- @@ -178,6 +180,9 @@ The feeds are stored in a FTP server. - Example URI: ``ftp://user:pass@ftp.example.com/path/to/export.csv`` - Required external libraries: none +FTP sends credentials and data in cleartext. Use :ref:`feed-storage-ftps` +instead where possible. + FTP supports two different connection modes: `active or passive `_. Scrapy uses the passive connection mode by default. To use the active connection mode instead, set the @@ -192,6 +197,28 @@ storage backend is: ``True``. This storage backend uses :ref:`delayed file delivery `. +.. _feed-storage-ftps: + +FTPS +---- + +The feeds are stored in a FTP server, over a TLS connection, with the +certificate of the server verified. + +.. versionadded:: VERSION + +- URI scheme: ``ftps`` +- Example URI: ``ftps://user:pass@ftp.example.com/path/to/export.csv`` +- Required external libraries: none + +See :ref:`feed-storage-ftp` for connection modes, the ``overwrite`` default and +file delivery. + +.. note:: For SFTP, an unrelated protocol built on SSH, use + `scrapy-feedexporter-sftp + `_. + + .. _topics-feed-storage-s3: S3 @@ -502,7 +529,7 @@ as a fallback value if that key is not provided for a specific feed definition: - :ref:`topics-feed-storage-fs`: ``False`` - - :ref:`topics-feed-storage-ftp`: ``True`` + - :ref:`feed-storage-ftp` and :ref:`feed-storage-ftps`: ``True`` .. note:: Some FTP servers may not support appending to files (the ``APPE`` FTP command). @@ -624,6 +651,7 @@ Default: "s3": "scrapy.extensions.feedexport.S3FeedStorage", "gs": "scrapy.extensions.feedexport.GCSFeedStorage", "ftp": "scrapy.extensions.feedexport.FTPFeedStorage", + "ftps": "scrapy.extensions.feedexport.FTPFeedStorage", } A dict containing the built-in feed storage backends supported by Scrapy. You diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index de74ad5cf..27ef3f7ef 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1393,7 +1393,7 @@ FEED_TEMPDIR Default: ``None`` The Feed Temp dir allows you to set a custom folder to save crawler -temporary files before uploading with :ref:`FTP feed storage ` and +temporary files before uploading with :ref:`FTP feed storage ` and :ref:`Amazon S3 `. .. setting:: FEED_STORAGE_GCS_ACL diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 32abc82d3..874023e25 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -363,6 +363,7 @@ class FTPFeedStorage(BlockingFeedStorage): self.username: str = u.username or "" self.password: str = unquote(u.password or "") self.path: str = u.path + self.tls: bool = u.scheme == "ftps" self.use_active_mode: bool = use_active_mode self.overwrite: bool = not feed_options or feed_options.get("overwrite", True) @@ -390,6 +391,7 @@ class FTPFeedStorage(BlockingFeedStorage): password=self.password, use_active_mode=self.use_active_mode, overwrite=self.overwrite, + tls=self.tls, ) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a44b36c8a..d7b30b849 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -378,6 +378,7 @@ FEED_STORAGES_BASE = { "": "scrapy.extensions.feedexport.FileFeedStorage", "file": "scrapy.extensions.feedexport.FileFeedStorage", "ftp": "scrapy.extensions.feedexport.FTPFeedStorage", + "ftps": "scrapy.extensions.feedexport.FTPFeedStorage", "gs": "scrapy.extensions.feedexport.GCSFeedStorage", "s3": "scrapy.extensions.feedexport.S3FeedStorage", "stdout": "scrapy.extensions.feedexport.StdoutFeedStorage", diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index a3e7a4306..f08e5303d 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -1,7 +1,8 @@ import posixpath from contextlib import closing -from ftplib import FTP, error_perm +from ftplib import FTP, FTP_TLS, error_perm from posixpath import dirname +from ssl import create_default_context from typing import IO @@ -29,13 +30,20 @@ def ftp_store_file( password: str, use_active_mode: bool = False, overwrite: bool = True, + tls: bool = False, ) -> None: - """Opens a FTP connection with passed credentials,sets current directory - to the directory extracted from given path, then uploads the file to server + """Opens a FTP connection with passed credentials, sets current directory + to the directory extracted from given path, then uploads the file to server. + + If *tls* is ``True``, the connection is secured with TLS (FTPS), and the + certificate of the server is verified. """ - with FTP() as ftp, closing(file): + ftp = FTP_TLS(context=create_default_context()) if tls else FTP() + with ftp, closing(file): ftp.connect(host, port) ftp.login(username, password) + if isinstance(ftp, FTP_TLS): + ftp.prot_p() if use_active_mode: ftp.set_pasv(False) file.seek(0) diff --git a/tests/keys/__init__.py b/tests/keys/__init__.py index 9b73ca4f0..804c9b6a8 100644 --- a/tests/keys/__init__.py +++ b/tests/keys/__init__.py @@ -1,4 +1,5 @@ from datetime import datetime, timedelta, timezone +from ipaddress import IPv4Address from pathlib import Path from cryptography.hazmat.backends import default_backend @@ -12,6 +13,7 @@ from cryptography.hazmat.primitives.serialization import ( from cryptography.x509 import ( CertificateBuilder, DNSName, + IPAddress, Name, NameAttribute, SubjectAlternativeName, @@ -53,7 +55,9 @@ def generate_keys(): .not_valid_before(datetime.now(tz=timezone.utc)) .not_valid_after(datetime.now(tz=timezone.utc) + timedelta(days=10)) .add_extension( - SubjectAlternativeName([DNSName("localhost")]), + SubjectAlternativeName( + [DNSName("localhost"), IPAddress(IPv4Address("127.0.0.1"))] + ), critical=False, ) .sign(key, SHA256(), default_backend()) diff --git a/tests/mockserver/ftp.py b/tests/mockserver/ftp.py index 72760b5ac..d88e953a7 100644 --- a/tests/mockserver/ftp.py +++ b/tests/mockserver/ftp.py @@ -10,7 +10,7 @@ from tempfile import mkdtemp from typing import TYPE_CHECKING from pyftpdlib.authorizers import DummyAuthorizer -from pyftpdlib.handlers import FTPHandler +from pyftpdlib.handlers import FTPHandler, TLS_FTPHandler from pyftpdlib.servers import FTPServer from tests.utils import get_script_run_env @@ -25,28 +25,32 @@ if TYPE_CHECKING: class MockFTPServer: """Creates an FTP server on a random port with a default passwordless user (anonymous) and a temporary root path that you can read from the - :attr:`path` attribute.""" + :attr:`path` attribute. + + If *tls* is ``True``, the server requires FTPS, using the test certificate + from :file:`tests/keys`. + """ proc: Popen[str] port: int path: Path - def __init__(self) -> None: + def __init__(self, tls: bool = False) -> None: self.host: str = "127.0.0.1" + self.tls: bool = tls def __enter__(self) -> Self: self.path = Path(mkdtemp()) self.proc = Popen( - [sys.executable, "-u", "-m", "tests.mockserver.ftp", "-d", str(self.path)], + [sys.executable, "-u", "-m", "tests.mockserver.ftp", "-d", str(self.path)] + + (["--tls"] if self.tls else []), stderr=PIPE, env=get_script_run_env(), text=True, ) assert self.proc.stderr is not None for line in self.proc.stderr: - if "starting FTP server" in line and ( - m := re.search(r"starting FTP server on ([^ :]+):(\d+),", line) - ): + if m := re.search(r"starting FTPS? .*on ([^ :]+):(\d+),", line): self.port = int(m.group(2)) break else: @@ -68,18 +72,28 @@ class MockFTPServer: self.proc.communicate() def url(self, path: str) -> str: - return f"ftp://{self.host}:{self.port}/{path}" + scheme = "ftps" if self.tls else "ftp" + return f"{scheme}://{self.host}:{self.port}/{path}" def main() -> None: parser = ArgumentParser() parser.add_argument("-d", "--directory", required=True) + parser.add_argument("--tls", action="store_true") args = parser.parse_args() authorizer = DummyAuthorizer() full_permissions = "elradfmwMT" authorizer.add_anonymous(args.directory, perm=full_permissions) - handler = FTPHandler + if args.tls: + keys = Path(__file__).parent.parent / "keys" + handler = TLS_FTPHandler + handler.certfile = str(keys / "localhost.crt") + handler.keyfile = str(keys / "localhost.key") + handler.tls_control_required = True + handler.tls_data_required = True + else: + handler = FTPHandler handler.authorizer = authorizer address = ("127.0.0.1", 0) server = FTPServer(address, handler) diff --git a/tests/test_feedexport_storages.py b/tests/test_feedexport_storages.py index d7fb5c9c2..23b39431e 100644 --- a/tests/test_feedexport_storages.py +++ b/tests/test_feedexport_storages.py @@ -7,6 +7,7 @@ import sys import tempfile from io import BytesIO from pathlib import Path +from ssl import SSLCertVerificationError from typing import IO, Any from unittest import mock from urllib.parse import quote @@ -169,6 +170,24 @@ class TestFTPFeedStorage: await self._store(url, b"bar", settings=settings) self._assert_stored(ftp_server.path / filename, b"bar") + @coroutine_test + async def test_tls(self, monkeypatch): + monkeypatch.setenv( + "SSL_CERT_FILE", str(Path(__file__).parent / "keys" / "localhost.crt") + ) + with MockFTPServer(tls=True) as ftp_server: + filename = "file" + await self._store(ftp_server.url(filename), b"foo") + self._assert_stored(ftp_server.path / filename, b"foo") + + @coroutine_test + async def test_tls_untrusted_certificate(self): + with ( + MockFTPServer(tls=True) as ftp_server, + pytest.raises(SSLCertVerificationError), + ): + await self._store(ftp_server.url("file"), b"foo") + def test_uri_auth_quote(self): # RFC3986: 3.2.1. User Information pw_quoted = quote(string.punctuation, safe="") From 15885a8db4683e974fbcbb56a484846108afc350 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 10 Aug 2026 17:55:10 +0200 Subject: [PATCH 4/7] Generalize the use of build_from_crawler() internally (#7808) * Generalize the use of build_from_crawler() internally * Do not use build_from_crawler() for spiders, since they are guaranteed having from_crawler() --- scrapy/core/downloader/__init__.py | 5 +- scrapy/core/scraper.py | 12 ++- scrapy/crawler.py | 4 +- scrapy/downloadermiddlewares/robotstxt.py | 6 +- scrapy/extensions/memusage.py | 3 +- scrapy/extensions/statsmailer.py | 3 +- tests/test_command_shell.py | 2 +- tests/test_downloader_handler_twisted_ftp.py | 2 +- .../test_downloader_handler_twisted_http11.py | 3 +- .../test_downloader_handler_twisted_http2.py | 3 +- tests/test_downloadermiddleware.py | 3 +- tests/test_downloadermiddleware_cookies.py | 21 +++--- ...est_downloadermiddleware_defaultheaders.py | 3 +- ...st_downloadermiddleware_downloadtimeout.py | 3 +- tests/test_downloadermiddleware_httpauth.py | 3 +- tests/test_downloadermiddleware_httpcache.py | 3 +- ...st_downloadermiddleware_httpcompression.py | 31 ++++---- tests/test_downloadermiddleware_httpproxy.py | 3 +- tests/test_downloadermiddleware_offsite.py | 29 ++++---- tests/test_downloadermiddleware_redirect.py | 10 +-- ...wnloadermiddleware_redirect_metarefresh.py | 8 +- tests/test_downloadermiddleware_retry.py | 9 ++- tests/test_downloadermiddleware_robotstxt.py | 43 +++++++---- tests/test_downloadermiddleware_stats.py | 5 +- tests/test_downloadermiddleware_useragent.py | 3 +- tests/test_dupefilters.py | 9 ++- tests/test_engine.py | 3 +- tests/test_extension_debug.py | 11 +-- tests/test_extension_memdebug.py | 7 +- tests/test_extension_memusage.py | 3 +- tests/test_extension_periodic_log.py | 3 +- tests/test_extension_statsmailer.py | 7 +- tests/test_extension_telnet.py | 5 +- tests/test_extension_throttle.py | 18 ++--- tests/test_feedexport.py | 16 ++-- tests/test_feedexport_batch.py | 3 +- tests/test_feedexport_storages.py | 39 ++++++---- tests/test_feedexport_uri_params.py | 15 ++-- tests/test_logformatter.py | 16 ++-- tests/test_logstats.py | 7 +- tests/test_middleware.py | 3 +- tests/test_pipeline_files.py | 63 +++++++++------- tests/test_pipeline_images.py | 56 +++++++------- tests/test_pipeline_media.py | 9 ++- tests/test_pipelines.py | 3 +- tests/test_pqueues.py | 32 ++++---- tests/test_resolver.py | 5 +- tests/test_robotstxt_interface.py | 13 +++- tests/test_scheduler.py | 10 +-- tests/test_spidermiddleware.py | 11 +-- tests/test_spidermiddleware_base.py | 9 ++- tests/test_spidermiddleware_depth.py | 2 +- tests/test_spidermiddleware_httperror.py | 9 ++- tests/test_spidermiddleware_metacopy.py | 7 +- tests/test_spidermiddleware_urllength.py | 2 +- tests/test_spiderstate.py | 3 +- tests/test_squeues_request.py | 21 +++--- tests/test_stats.py | 5 +- tests/utils/bases/redirect.py | 74 +++++++++---------- 59 files changed, 405 insertions(+), 314 deletions(-) diff --git a/scrapy/core/downloader/__init__.py b/scrapy/core/downloader/__init__.py index f9ee62838..2089b1224 100644 --- a/scrapy/core/downloader/__init__.py +++ b/scrapy/core/downloader/__init__.py @@ -28,6 +28,7 @@ from scrapy.utils.defer import ( maybe_deferred_to_future, ) from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.misc import build_from_crawler if TYPE_CHECKING: from collections.abc import Generator @@ -99,8 +100,8 @@ class Downloader: # AUTOTHROTTLE_START_DELAY. self._delay: float = self.settings.getfloat("DOWNLOAD_DELAY") self.randomize_delay: bool = self.settings.getbool("RANDOMIZE_DOWNLOAD_DELAY") - self.middleware: DownloaderMiddlewareManager = ( - DownloaderMiddlewareManager.from_crawler(crawler) + self.middleware: DownloaderMiddlewareManager = build_from_crawler( + DownloaderMiddlewareManager, crawler ) self._slot_gc_loop: AsyncioLoopingCall | LoopingCall | None = None self.per_slot_settings: dict[str, dict[str, Any]] = self.settings.getdict( diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 6426b1751..351375d7b 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -36,7 +36,11 @@ from scrapy.utils.defer import ( ) from scrapy.utils.deprecate import method_is_overridden from scrapy.utils.log import failure_to_exc_info, logformatter_adapter -from scrapy.utils.misc import load_object, warn_on_generator_with_return_value +from scrapy.utils.misc import ( + build_from_crawler, + load_object, + warn_on_generator_with_return_value, +) from scrapy.utils.python import global_object_name from scrapy.utils.spider import iterate_spider_output @@ -102,13 +106,13 @@ class Slot: class Scraper: def __init__(self, crawler: Crawler) -> None: self.slot: Slot | None = None - self.spidermw: SpiderMiddlewareManager = SpiderMiddlewareManager.from_crawler( - crawler + self.spidermw: SpiderMiddlewareManager = build_from_crawler( + SpiderMiddlewareManager, crawler ) itemproc_cls: type[ItemPipelineManager] = load_object( crawler.settings["ITEM_PROCESSOR"] ) - self.itemproc: ItemPipelineManager = itemproc_cls.from_crawler(crawler) + self.itemproc: ItemPipelineManager = build_from_crawler(itemproc_cls, crawler) self._itemproc_has_async: dict[str, bool] = {} for method in [ "open_spider", diff --git a/scrapy/crawler.py b/scrapy/crawler.py index f0cfba6b9..444a5fb67 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -156,7 +156,7 @@ class Crawler: self.stats = load_object(self.settings["STATS_CLASS"])(self) lf_cls: type[LogFormatter] = load_object(self.settings["LOG_FORMATTER"]) - self.logformatter = lf_cls.from_crawler(self) + self.logformatter = build_from_crawler(lf_cls, self) self.request_fingerprinter = build_from_crawler( load_object(self.settings["REQUEST_FINGERPRINTER_CLASS"]), @@ -200,7 +200,7 @@ class Crawler: logger.debug("Not using a Twisted reactor") self._apply_reactorless_default_settings() - self.extensions = ExtensionManager.from_crawler(self) + self.extensions = build_from_crawler(ExtensionManager, self) self.settings.freeze() d = dict(overridden_settings(self.settings)) diff --git a/scrapy/downloadermiddlewares/robotstxt.py b/scrapy/downloadermiddlewares/robotstxt.py index d7aa8738c..016cf9acb 100644 --- a/scrapy/downloadermiddlewares/robotstxt.py +++ b/scrapy/downloadermiddlewares/robotstxt.py @@ -18,7 +18,7 @@ from scrapy.http.request import NO_CALLBACK from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.misc import load_object +from scrapy.utils.misc import build_from_crawler, load_object if TYPE_CHECKING: # typing.Self requires Python 3.11 @@ -49,7 +49,7 @@ class RobotsTxtMiddleware: ) # check if parser dependencies are met, this should throw an error otherwise. - self._parserimpl.from_crawler(self.crawler, b"") + build_from_crawler(self._parserimpl, self.crawler, b"") @classmethod def from_crawler(cls, crawler: Crawler) -> Self: @@ -120,7 +120,7 @@ class RobotsTxtMiddleware: ) -> None: self._stats.inc_value("robotstxt/response_count") self._stats.inc_value(f"robotstxt/response_status_count/{response.status}") - rp = self._parserimpl.from_crawler(self.crawler, response.body) + rp = build_from_crawler(self._parserimpl, self.crawler, response.body) await self.crawler.signals.send_catch_log_async( signal=signals.robots_parsed, robotparser=rp, diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index ec761cfbf..bf1f1bec4 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -19,6 +19,7 @@ from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.utils.asyncio import AsyncioLoopingCall, create_looping_call from scrapy.utils.defer import _schedule_coro from scrapy.utils.engine import get_engine_status +from scrapy.utils.misc import build_from_crawler if TYPE_CHECKING: from twisted.internet.task import LoopingCall @@ -57,7 +58,7 @@ class MemoryUsage: category=ScrapyDeprecationWarning, stacklevel=2, ) - self.mail = MailSender.from_crawler(crawler) + self.mail = build_from_crawler(MailSender, crawler) self.limit: int = crawler.settings.getint("MEMUSAGE_LIMIT_MB") * 1024 * 1024 self.warning: int = crawler.settings.getint("MEMUSAGE_WARNING_MB") * 1024 * 1024 diff --git a/scrapy/extensions/statsmailer.py b/scrapy/extensions/statsmailer.py index 7647cf33d..3dc38dded 100644 --- a/scrapy/extensions/statsmailer.py +++ b/scrapy/extensions/statsmailer.py @@ -12,6 +12,7 @@ from typing import TYPE_CHECKING from scrapy import Spider, signals from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning from scrapy.mail import MailSender +from scrapy.utils.misc import build_from_crawler if TYPE_CHECKING: from twisted.internet.defer import Deferred @@ -41,7 +42,7 @@ class StatsMailer: recipients: list[str] = crawler.settings.getlist("STATSMAILER_RCPTS") if not recipients: raise NotConfigured - mail: MailSender = MailSender.from_crawler(crawler) + mail: MailSender = build_from_crawler(MailSender, crawler) o = cls(crawler.stats, recipients, mail) crawler.signals.connect(o.spider_closed, signal=signals.spider_closed) return o diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 6bc1ebbbb..44178727e 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -365,7 +365,7 @@ class TestShell: crawler.engine = MagicMock() crawler.engine.open_spider_async = AsyncMock() shell = Shell(crawler) - spider = Spider("test") + spider = Spider.from_crawler(crawler, "test") await shell._open_spider(spider) assert shell.spider is spider assert crawler.spider is spider diff --git a/tests/test_downloader_handler_twisted_ftp.py b/tests/test_downloader_handler_twisted_ftp.py index 14de97b21..28a067fb7 100644 --- a/tests/test_downloader_handler_twisted_ftp.py +++ b/tests/test_downloader_handler_twisted_ftp.py @@ -212,4 +212,4 @@ class TestAnonymousFTP(TestFTPBase): def test_not_configured_without_reactor() -> None: crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False}) with pytest.raises(NotConfigured): - FTPDownloadHandler.from_crawler(crawler) + build_from_crawler(FTPDownloadHandler, crawler) diff --git a/tests/test_downloader_handler_twisted_http11.py b/tests/test_downloader_handler_twisted_http11.py index 32dfd7540..db353750d 100644 --- a/tests/test_downloader_handler_twisted_http11.py +++ b/tests/test_downloader_handler_twisted_http11.py @@ -11,6 +11,7 @@ from scrapy import Spider from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.crawler import Crawler from scrapy.exceptions import NotConfigured +from scrapy.utils.misc import build_from_crawler from tests.utils.bases.download_handlers_http import ( TestHttpBase, TestHttpProxyBase, @@ -51,7 +52,7 @@ class HTTP11DownloadHandlerMixin: def test_not_configured_without_reactor() -> None: crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False}) with pytest.raises(NotConfigured): - HTTP11DownloadHandler.from_crawler(crawler) + build_from_crawler(HTTP11DownloadHandler, crawler) class TestHttp(HTTP11DownloadHandlerMixin, TestHttpBase): diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 9d4e161f3..2c3954b5e 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -13,6 +13,7 @@ from scrapy import Spider from scrapy.crawler import Crawler from scrapy.exceptions import DownloadFailedError, NotConfigured from scrapy.http import Request +from scrapy.utils.misc import build_from_crawler from tests.utils.bases.download_handlers_http import ( TestHttpProxyBase, TestHttpsBase, @@ -66,7 +67,7 @@ def test_not_configured_without_reactor() -> None: crawler = Crawler(Spider, {"TWISTED_REACTOR_ENABLED": False}) with pytest.raises(NotConfigured): - H2DownloadHandler.from_crawler(crawler) + build_from_crawler(H2DownloadHandler, crawler) class TestHttp2(H2DownloadHandlerMixin, TestHttpsBase): diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 92eb18d33..9b89b0760 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -14,6 +14,7 @@ from scrapy.exceptions import ScrapyDeprecationWarning, _InvalidOutput from scrapy.http import Request, Response from scrapy.spiders import Spider from scrapy.utils.defer import 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, get_from_asyncio_queue from tests.utils.decorators import coroutine_test @@ -30,7 +31,7 @@ class TestManagerBase: async def get_mwman(self) -> AsyncGenerator[DownloaderMiddlewareManager]: crawler = get_crawler(Spider, self.settings_dict) crawler.spider = crawler._create_spider("foo") - mwman = DownloaderMiddlewareManager.from_crawler(crawler) + mwman = build_from_crawler(DownloaderMiddlewareManager, crawler) crawler.engine = crawler._create_engine() await crawler.engine.open_spider_async() try: diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index e4b66fe10..6c4c80eae 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -10,6 +10,7 @@ from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.http.request import CookiesT, VerboseCookie +from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes from scrapy.utils.request import _to_verbose_cookies from scrapy.utils.spider import DefaultSpider @@ -72,8 +73,8 @@ class TestCookiesMiddleware: def setup_method(self): crawler = get_crawler(DefaultSpider) crawler.spider = crawler._create_spider() - self.mw = CookiesMiddleware.from_crawler(crawler) - self.redirect_middleware = RedirectMiddleware.from_crawler(crawler) + self.mw = build_from_crawler(CookiesMiddleware, crawler) + self.redirect_middleware = build_from_crawler(RedirectMiddleware, crawler) def teardown_method(self): del self.mw @@ -94,19 +95,19 @@ class TestCookiesMiddleware: def test_setting_false_cookies_enabled(self): with pytest.raises(NotConfigured): - CookiesMiddleware.from_crawler( - get_crawler(settings_dict={"COOKIES_ENABLED": False}) + build_from_crawler( + CookiesMiddleware, get_crawler(settings_dict={"COOKIES_ENABLED": False}) ) def test_setting_default_cookies_enabled(self): assert isinstance( - CookiesMiddleware.from_crawler(get_crawler()), CookiesMiddleware + build_from_crawler(CookiesMiddleware, get_crawler()), CookiesMiddleware ) def test_setting_true_cookies_enabled(self): assert isinstance( - CookiesMiddleware.from_crawler( - get_crawler(settings_dict={"COOKIES_ENABLED": True}) + build_from_crawler( + CookiesMiddleware, get_crawler(settings_dict={"COOKIES_ENABLED": True}) ), CookiesMiddleware, ) @@ -115,7 +116,7 @@ class TestCookiesMiddleware: self, caplog: pytest.LogCaptureFixture ) -> None: crawler = get_crawler(settings_dict={"COOKIES_DEBUG": True}) - mw = CookiesMiddleware.from_crawler(crawler) + mw = build_from_crawler(CookiesMiddleware, crawler) caplog.clear() with caplog.at_level( logging.DEBUG, logger="scrapy.downloadermiddlewares.cookies" @@ -145,7 +146,7 @@ class TestCookiesMiddleware: def test_debug_no_cookies(self, caplog: pytest.LogCaptureFixture) -> None: crawler = get_crawler(settings_dict={"COOKIES_DEBUG": True}) - mw = CookiesMiddleware.from_crawler(crawler) + mw = build_from_crawler(CookiesMiddleware, crawler) caplog.clear() with caplog.at_level( logging.DEBUG, logger="scrapy.downloadermiddlewares.cookies" @@ -161,7 +162,7 @@ class TestCookiesMiddleware: self, caplog: pytest.LogCaptureFixture ) -> None: crawler = get_crawler(settings_dict={"COOKIES_DEBUG": False}) - mw = CookiesMiddleware.from_crawler(crawler) + mw = build_from_crawler(CookiesMiddleware, crawler) caplog.clear() with caplog.at_level( logging.DEBUG, logger="scrapy.downloadermiddlewares.cookies" diff --git a/tests/test_downloadermiddleware_defaultheaders.py b/tests/test_downloadermiddleware_defaultheaders.py index 8c89c3ffb..507097df9 100644 --- a/tests/test_downloadermiddleware_defaultheaders.py +++ b/tests/test_downloadermiddleware_defaultheaders.py @@ -3,6 +3,7 @@ from __future__ import annotations from scrapy.downloadermiddlewares.defaultheaders import DefaultHeadersMiddleware from scrapy.http import Request from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler @@ -13,7 +14,7 @@ def get_defaults_mw() -> tuple[dict[bytes, list[bytes]], DefaultHeadersMiddlewar to_bytes(k): [to_bytes(v)] for k, v in crawler.settings.get("DEFAULT_REQUEST_HEADERS").items() } - return defaults, DefaultHeadersMiddleware.from_crawler(crawler) + return defaults, build_from_crawler(DefaultHeadersMiddleware, crawler) def test_process_request(): diff --git a/tests/test_downloadermiddleware_downloadtimeout.py b/tests/test_downloadermiddleware_downloadtimeout.py index 9b64cf349..7e2d77136 100644 --- a/tests/test_downloadermiddleware_downloadtimeout.py +++ b/tests/test_downloadermiddleware_downloadtimeout.py @@ -5,6 +5,7 @@ from typing import Any from scrapy.downloadermiddlewares.downloadtimeout import DownloadTimeoutMiddleware from scrapy.http import Request from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler @@ -12,7 +13,7 @@ def get_request_spider_mw(settings: dict[str, Any] | None = None): crawler = get_crawler(Spider, settings) spider = crawler._create_spider("foo") request = Request("http://scrapytest.org/") - return request, spider, DownloadTimeoutMiddleware.from_crawler(crawler) + return request, spider, build_from_crawler(DownloadTimeoutMiddleware, crawler) def test_default_download_timeout(): diff --git a/tests/test_downloadermiddleware_httpauth.py b/tests/test_downloadermiddleware_httpauth.py index dd5af3bc5..e4b414f45 100644 --- a/tests/test_downloadermiddleware_httpauth.py +++ b/tests/test_downloadermiddleware_httpauth.py @@ -7,6 +7,7 @@ from scrapy.downloadermiddlewares.httpauth import HttpAuthMiddleware from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler _DOMAIN_NOT_SET = object() @@ -21,7 +22,7 @@ def make_mw( } if domain is not _DOMAIN_NOT_SET: settings["HTTPAUTH_DOMAIN"] = domain - return HttpAuthMiddleware.from_crawler(get_crawler(settings_dict=settings)) + return build_from_crawler(HttpAuthMiddleware, get_crawler(settings_dict=settings)) # --- Spider attribute tests (deprecated) --- diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index dc8228470..50ff5e63f 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -17,6 +17,7 @@ from scrapy.exceptions import IgnoreRequest from scrapy.extensions.httpcache import DummyPolicy from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler if TYPE_CHECKING: @@ -87,7 +88,7 @@ class TestBase: def _middleware(self, **new_settings: Any) -> Generator[HttpCacheMiddleware]: with self._get_crawler(**new_settings) as crawler: assert crawler.spider - mw = HttpCacheMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCacheMiddleware, crawler) mw.spider_opened(crawler.spider) try: yield mw diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index a43bb51ba..4e0fd7c8b 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -18,6 +18,7 @@ from scrapy.responsetypes import responsetypes from scrapy.spiders import Spider from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.gz import gunzip +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler from tests import tests_datadir @@ -59,7 +60,7 @@ def _skip_if_no_zstd() -> None: class TestHttpCompression: def setup_method(self): self.crawler = get_crawler(Spider) - self.mw = HttpCompressionMiddleware.from_crawler(self.crawler) + self.mw = build_from_crawler(HttpCompressionMiddleware, self.crawler) assert self.crawler.stats self.crawler.stats.open_spider() @@ -93,20 +94,22 @@ class TestHttpCompression: def test_setting_false_compression_enabled(self): with pytest.raises(NotConfigured): - HttpCompressionMiddleware.from_crawler( - get_crawler(settings_dict={"COMPRESSION_ENABLED": False}) + build_from_crawler( + HttpCompressionMiddleware, + get_crawler(settings_dict={"COMPRESSION_ENABLED": False}), ) def test_setting_default_compression_enabled(self): assert isinstance( - HttpCompressionMiddleware.from_crawler(get_crawler()), + build_from_crawler(HttpCompressionMiddleware, get_crawler()), HttpCompressionMiddleware, ) def test_setting_true_compression_enabled(self): assert isinstance( - HttpCompressionMiddleware.from_crawler( - get_crawler(settings_dict={"COMPRESSION_ENABLED": True}) + build_from_crawler( + HttpCompressionMiddleware, + get_crawler(settings_dict={"COMPRESSION_ENABLED": True}), ), HttpCompressionMiddleware, ) @@ -496,7 +499,7 @@ class TestHttpCompression: settings = {"DOWNLOAD_MAXSIZE": 1_000_000} crawler = get_crawler(Spider, settings_dict=settings) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") # 11_511_612 B @@ -525,7 +528,7 @@ class TestHttpCompression: settings = {"DOWNLOAD_MAXSIZE": 1_000_000} crawler = get_crawler(Spider, settings_dict=settings) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse("bomb-gzip") # 11_511_612 B @@ -552,7 +555,7 @@ class TestHttpCompression: crawler = get_crawler(DownloadMaxSizeSpider) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") @@ -584,7 +587,7 @@ class TestHttpCompression: def _test_compression_bomb_request_meta(self, compression_id: str) -> None: crawler = get_crawler(Spider) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") @@ -616,7 +619,7 @@ class TestHttpCompression: settings = {"DOWNLOAD_WARNSIZE": 10_000_000} crawler = get_crawler(Spider, settings_dict=settings) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") @@ -668,7 +671,7 @@ class TestHttpCompression: crawler = get_crawler(DownloadWarnSizeSpider) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") @@ -721,7 +724,7 @@ class TestHttpCompression: ) -> None: crawler = get_crawler(Spider) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(f"bomb-{compression_id}") response.meta["download_warnsize"] = 10_000_000 @@ -769,7 +772,7 @@ class TestHttpCompression: def _get_truncated_response(self, compression_id: str) -> Response: crawler = get_crawler(Spider) spider = crawler._create_spider("scrapytest.org") - mw = HttpCompressionMiddleware.from_crawler(crawler) + mw = build_from_crawler(HttpCompressionMiddleware, crawler) mw.open_spider(spider) response = self._getresponse(compression_id) truncated_body = response.body[: len(response.body) // 2] diff --git a/tests/test_downloadermiddleware_httpproxy.py b/tests/test_downloadermiddleware_httpproxy.py index 54d4601a7..a7a6f2079 100644 --- a/tests/test_downloadermiddleware_httpproxy.py +++ b/tests/test_downloadermiddleware_httpproxy.py @@ -6,6 +6,7 @@ from scrapy.downloadermiddlewares.httpproxy import HttpProxyMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request from scrapy.spiders import Spider +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler @@ -20,7 +21,7 @@ class TestHttpProxyMiddleware: def test_not_enabled(self): crawler = get_crawler(Spider, {"HTTPPROXY_ENABLED": False}) with pytest.raises(NotConfigured): - HttpProxyMiddleware.from_crawler(crawler) + build_from_crawler(HttpProxyMiddleware, crawler) def test_no_environment_proxies(self): os.environ.clear() diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py index 1f91dcd4b..40e09be13 100644 --- a/tests/test_downloadermiddleware_offsite.py +++ b/tests/test_downloadermiddleware_offsite.py @@ -7,6 +7,7 @@ from scrapy import Request, Spider from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware from scrapy.exceptions import IgnoreRequest from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.misc import build_from_crawler from scrapy.utils.test import get_crawler UNSET = object() @@ -31,7 +32,7 @@ UNSET = object() def test_process_request_domain_filtering(allowed_domain, url, allowed): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request(url) if allowed: @@ -53,7 +54,7 @@ def test_process_request_domain_filtering(allowed_domain, url, allowed): def test_process_request_dont_filter(value, filtered): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) kwargs: dict[str, Any] = {} if value is not UNSET: @@ -82,7 +83,7 @@ def test_process_request_dont_filter(value, filtered): def test_process_request_allow_offsite(allow_offsite, dont_filter, filtered): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) kwargs: dict[str, Any] = {"meta": {}} if allow_offsite is not UNSET: @@ -111,7 +112,7 @@ def test_process_request_no_allowed_domains(value): if value is not UNSET: kwargs["allowed_domains"] = value crawler.spider = crawler._create_spider(name="a", **kwargs) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request("https://example.com") assert mw.process_request(request) is None @@ -121,7 +122,7 @@ def test_process_request_invalid_domains(): crawler = get_crawler(Spider) 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) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request("https://a.example") assert mw.process_request(request) is None @@ -150,7 +151,7 @@ def test_process_request_invalid_domains(): def test_request_scheduled_domain_filtering(allowed_domain, url, allowed): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request(url) if allowed: @@ -172,7 +173,7 @@ def test_request_scheduled_domain_filtering(allowed_domain, url, allowed): def test_request_scheduled_dont_filter(value, filtered): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) kwargs: dict[str, Any] = {} if value is not UNSET: @@ -199,7 +200,7 @@ def test_request_scheduled_no_allowed_domains(value): if value is not UNSET: kwargs["allowed_domains"] = value crawler.spider = crawler._create_spider(name="a", **kwargs) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request("https://example.com") mw.request_scheduled(request, crawler.spider) @@ -209,7 +210,7 @@ def test_request_scheduled_invalid_domains(): crawler = get_crawler(Spider) 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) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request("https://a.example") mw.request_scheduled(request, crawler.spider) @@ -222,7 +223,7 @@ def test_request_scheduled_invalid_domains(): 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 = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) req1 = Request("http://other.org/1") req2 = Request("http://other.org/2") @@ -246,7 +247,7 @@ def test_should_follow_override(): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) - mw = RootOnlyOffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(RootOnlyOffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) assert mw.process_request(Request("https://example.com/1")) is None with pytest.raises(IgnoreRequest): @@ -256,7 +257,7 @@ def test_should_follow_override(): def test_ignore_request_reason(): crawler = get_crawler(Spider) crawler.spider = crawler._create_spider(name="a", allowed_domains=["example.com"]) - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(crawler.spider) request = Request("http://other.org/1") with pytest.raises( @@ -274,7 +275,7 @@ def test_dynamic_allowed_domains(): crawler = get_crawler(DomainSpider) spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) crawler.spider = spider - mw = OffsiteMiddleware.from_crawler(crawler) + mw = build_from_crawler(OffsiteMiddleware, crawler) mw.spider_opened(spider) with pytest.raises(IgnoreRequest): @@ -300,7 +301,7 @@ def test_dynamic_allowed_domains_caching(): crawler = get_crawler(DomainSpider) spider = DomainSpider.from_crawler(crawler, allowed_domains=["a.example"]) crawler.spider = spider - mw = TrackingMiddleware.from_crawler(crawler) + mw = build_from_crawler(TrackingMiddleware, crawler) mw.spider_opened(spider) for _ in range(3): diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index de97aaadb..4cebcb281 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -27,7 +27,7 @@ class TestRedirectMiddleware(TestRedirectBase): def setup_method(self): crawler = get_crawler(DefaultSpider) crawler.spider = crawler._create_spider() - self.mw = self.mwcls.from_crawler(crawler) + self.mw = build_from_crawler(self.mwcls, crawler) def get_response(self, request, location, status=302): headers = {"Location": location} @@ -208,7 +208,7 @@ class TestRedirectMiddleware(TestRedirectBase): response = Response(source_url, headers=resp_headers, status=302) crawler = get_crawler() referer_mw = build_from_crawler(RefererMiddleware, crawler) - redirect_mw = self.mwcls.from_crawler(crawler) + redirect_mw = build_from_crawler(self.mwcls, crawler) redirect_mw._referer_spider_middleware = referer_mw redirect_request = redirect_mw.process_response(source_request, response) if expected_referer: @@ -223,7 +223,7 @@ class TestRedirectMiddleware(TestRedirectBase): source_url, headers={"Referer": "http://example.com/old"} ) response = Response(source_url, headers={"Location": redirect_url}, status=302) - redirect_mw = self.mwcls.from_crawler(get_crawler()) + redirect_mw = build_from_crawler(self.mwcls, get_crawler()) redirect_mw._referer_spider_middleware = None redirect_request = redirect_mw.process_response(source_request, response) assert "Referer" not in redirect_request.headers @@ -352,7 +352,7 @@ class TestRedirectMiddleware(TestRedirectBase): @pytest.mark.parametrize(SCHEME_PARAMS, REDIRECT_SCHEME_CASES) def test_redirect_schemes(url, location, target): crawler = get_crawler(Spider) - mw = RedirectMiddleware.from_crawler(crawler) + mw = build_from_crawler(RedirectMiddleware, crawler) request = Request(url) response = Response(url, headers={"Location": location}, status=301) redirect = mw.process_response(request, response) @@ -477,4 +477,4 @@ def test_warning_subclass(caplog): def test_not_configured(): crawler = get_crawler(DefaultSpider, {"REDIRECT_ENABLED": False}) with pytest.raises(NotConfigured): - RedirectMiddleware.from_crawler(crawler) + build_from_crawler(RedirectMiddleware, crawler) diff --git a/tests/test_downloadermiddleware_redirect_metarefresh.py b/tests/test_downloadermiddleware_redirect_metarefresh.py index 83dc6825f..3887ddd26 100644 --- a/tests/test_downloadermiddleware_redirect_metarefresh.py +++ b/tests/test_downloadermiddleware_redirect_metarefresh.py @@ -32,7 +32,7 @@ class TestMetaRefreshMiddleware(TestRedirectBase): def setup_method(self): crawler = get_crawler(Spider) - self.mw = self.mwcls.from_crawler(crawler) + self.mw = build_from_crawler(self.mwcls, crawler) def _body( self, interval: int = 5, url: str = "http://example.org/newpage" @@ -95,7 +95,7 @@ class TestMetaRefreshMiddleware(TestRedirectBase): """Test that Scrapy 1.x behavior remains possible""" settings = {"METAREFRESH_IGNORE_TAGS": ["script", "noscript"]} crawler = get_crawler(Spider, settings) - mw = MetaRefreshMiddleware.from_crawler(crawler) + mw = build_from_crawler(MetaRefreshMiddleware, crawler) req = Request(url="http://example.org") body = ( """