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/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/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/item-pipeline.rst b/docs/topics/item-pipeline.rst index 35891ce8e..a6aac78ac 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -47,6 +47,13 @@ Additionally, they may also implement the following methods: This method is called when the spider is opened. + .. versionchanged:: VERSION + Added support for :exc:`~scrapy.exceptions.CloseSpider`. + + It may raise :exc:`~scrapy.exceptions.CloseSpider` to close the spider before + it starts crawling, e.g. if a resource that the pipeline needs is + unavailable. + .. method:: close_spider(self) This method is called when the spider is closed, before the 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/docs/topics/signals.rst b/docs/topics/signals.rst index acbf7ddef..0a85c3c05 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -290,6 +290,13 @@ spider_opened reserve per-spider resources, but can be used for any task that needs to be performed when a spider is opened. + .. versionchanged:: VERSION + Added support for :exc:`~scrapy.exceptions.CloseSpider`. + + You may raise a :exc:`~scrapy.exceptions.CloseSpider` exception to close the + spider before it starts crawling, e.g. if a resource that the spider needs + is unavailable. + This signal supports :ref:`asynchronous handlers `. :param spider: the spider which has been opened diff --git a/pyproject.toml b/pyproject.toml index 5ec1d05a4..0bdcf6b51 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -319,6 +319,9 @@ markers = [ ] filterwarnings = [ "ignore::DeprecationWarning:twisted.web.static", + # Jobs that do not report coverage disable it with --no-cov, which pytest-cov + # warns about because the coverage options below stay in place. + "ignore::pytest_cov.CovDisabledWarning", # Twisted doesn't close failed sockets after CannotListenError: https://github.com/twisted/twisted/issues/6108 "ignore:Exception ignored in. 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/scrapy/core/engine.py b/scrapy/core/engine.py index b63d136a4..104daf399 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -248,7 +248,7 @@ class ExecutionEngine: ) return deferred_from_coro(self.close_async()) - async def close_async(self) -> None: + async def close_async(self, *, reason: str = "shutdown") -> None: """ Gracefully close the execution engine. If it has already been started, stop it. In all cases, close the spider and the downloader. @@ -256,9 +256,7 @@ class ExecutionEngine: if self.running: await self.stop_async() # will also close spider and downloader elif self.spider is not None: - await self.close_spider_async( - reason="shutdown" - ) # will also close downloader + await self.close_spider_async(reason=reason) # will also close downloader elif hasattr(self, "downloader"): self.downloader.close() @@ -557,10 +555,20 @@ class ExecutionEngine: nextcall = CallLaterOnce(self._start_scheduled_requests) scheduler = build_from_crawler(self.scheduler_cls, self.crawler) self._slot = _Slot(close_if_idle, nextcall, scheduler) - self._start = await self.scraper.spidermw.process_start() - if hasattr(scheduler, "open") and (d := scheduler.open(self.crawler.spider)): - await maybe_deferred_to_future(d) - await self.scraper.open_spider_async() + # A component that fails to start can ask for the spider to be closed. + # The rest of the startup runs anyway, so that components that are + # started also get stopped, and the request is honored once the spider + # is open. + close_spider_exc: CloseSpider | None = None + try: + self._start = await self.scraper.spidermw.process_start() + if hasattr(scheduler, "open") and ( + d := scheduler.open(self.crawler.spider) + ): + await maybe_deferred_to_future(d) + await self.scraper.open_spider_async() + except CloseSpider as exc: + close_spider_exc = exc stats = self.crawler.stats if argument_is_required(stats.open_spider, "spider"): warnings.warn( @@ -572,9 +580,14 @@ class ExecutionEngine: stats.open_spider(spider=self.crawler.spider) else: stats.open_spider() - await self.signals.send_catch_log_async( - signals.spider_opened, spider=self.crawler.spider + results = await self.signals.send_catch_log_async( + signals.spider_opened, spider=self.crawler.spider, dont_log=CloseSpider ) + for _, result in results: + if isinstance(result, CloseSpider): + close_spider_exc = close_spider_exc or result + if close_spider_exc is not None: + raise close_spider_exc def _spider_idle(self) -> None: """ diff --git a/scrapy/crawler.py b/scrapy/crawler.py index fa481bd42..618d46a02 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -15,7 +15,7 @@ from twisted.internet.defer import Deferred, DeferredList, inlineCallbacks from scrapy import Spider from scrapy.addons import AddonManager from scrapy.core.engine import ExecutionEngine -from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.exceptions import CloseSpider, ScrapyDeprecationWarning from scrapy.extension import ExtensionManager from scrapy.settings import SETTINGS_PRIORITIES, Settings, overridden_settings from scrapy.signalmanager import SignalManager @@ -277,8 +277,12 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - yield deferred_from_coro(self.engine.open_spider_async()) - yield deferred_from_coro(self.engine.start_async()) + try: + yield deferred_from_coro(self.engine.open_spider_async()) + except CloseSpider as exc: + yield deferred_from_coro(self.engine.close_async(reason=exc.reason)) + else: + yield deferred_from_coro(self.engine.start_async()) except Exception: self.crawling = False if self._engine is not None: @@ -307,8 +311,12 @@ class Crawler: self._apply_settings() self._update_root_log_handler() self.engine = self._create_engine() - await self.engine.open_spider_async() - await self.engine.start_async() + try: + await self.engine.open_spider_async() + except CloseSpider as exc: + await self.engine.close_async(reason=exc.reason) + else: + await self.engine.start_async() except Exception: self.crawling = False if self._engine is not None: diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index fa5bc1c1e..cd2560df1 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -56,12 +56,11 @@ class DontCloseSpider(Exception): class CloseSpider(Exception): - """Raised from a :ref:`spider callback ` or from - :meth:`~scrapy.Spider.start` to request the spider to be closed/stopped. + """Raised from a :ref:`spider callback `, or while the + spider is starting, to request the spider to be closed/stopped. .. versionchanged:: VERSION - Raising it from :meth:`~scrapy.Spider.start` closes the spider, instead - of being reported as a start error. + Added support for raising it while the spider is starting. *reason* is a string with the reason for closing. 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/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/scrapy/utils/trackref.py b/scrapy/utils/trackref.py index 0bbe68def..2882b7f96 100644 --- a/scrapy/utils/trackref.py +++ b/scrapy/utils/trackref.py @@ -14,7 +14,6 @@ This library has a minimal performance impact. from __future__ import annotations -from collections import defaultdict from operator import itemgetter from time import monotonic_ns from types import NoneType @@ -28,8 +27,8 @@ if TYPE_CHECKING: from typing_extensions import Self -live_refs: defaultdict[type, WeakKeyDictionary[object, float]] = defaultdict( - WeakKeyDictionary +live_refs: WeakKeyDictionary[type, WeakKeyDictionary[object, float]] = ( + WeakKeyDictionary() ) @@ -41,7 +40,11 @@ class object_ref: def __new__(cls, *args: Any, **kwargs: Any) -> Self: obj = object.__new__(cls) - live_refs[cls][obj] = monotonic_ns() + try: + refs = live_refs[cls] + except KeyError: + refs = live_refs[cls] = WeakKeyDictionary() + refs[obj] = monotonic_ns() return obj diff --git a/tests/ignores.txt b/tests/ignores.txt index 3717bbc95..5be8576f3 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,3 +1,4 @@ scrapy/core/downloader/handlers/http.py scrapy/extensions/statsmailer.py +scrapy/interfaces.py scrapy/mail.py 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/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/test_engine.py b/tests/test_engine.py index 8a3bceccb..53eb4a1f6 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -29,7 +29,9 @@ from tests.utils.engine import ( ) if TYPE_CHECKING: - from collections.abc import AsyncIterator + from collections.abc import AsyncIterator, Generator + + from twisted.internet.defer import Deferred from tests.mockserver.http import MockServer @@ -230,3 +232,51 @@ async def test_request_scheduled_signal(): f"{scheduler.enqueued!r} != [{keep_request!r}]" ) crawler.signals.disconnect(signal_handler, signals.request_scheduled) + + +class ClosingPipeline: + def open_spider(self): + raise CloseSpider("pipeline_reason") + + +class TestCloseSpiderOnStartup: + @coroutine_test + async def test_pipeline(self, caplog: pytest.LogCaptureFixture) -> None: + closed: list[str] = [] + + def spider_closed(reason: str) -> None: + closed.append(reason) + + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + crawler.signals.connect(spider_closed, signals.spider_closed) + with caplog.at_level(logging.INFO): + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" + assert closed == ["pipeline_reason"] + assert "Traceback" not in caplog.text + + @coroutine_test + async def test_spider_opened(self) -> None: + def spider_opened(spider: Spider) -> None: + raise CloseSpider("signal_reason") + + crawler = get_crawler(DefaultSpider) + crawler.signals.connect(spider_opened, signals.spider_opened) + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "signal_reason" + + @coroutine_test + async def test_startup_wins_over_spider_opened(self) -> None: + def spider_opened(spider: Spider) -> None: + raise CloseSpider("signal_reason") + + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + crawler.signals.connect(spider_opened, signals.spider_opened) + await crawler.crawl_async() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" + + @inline_callbacks_test + def test_deferred_crawl(self) -> Generator[Deferred[Any], Any, None]: + crawler = get_crawler(DefaultSpider, {"ITEM_PIPELINES": {ClosingPipeline: 1}}) + yield crawler.crawl() + assert crawler.stats.get_value("finish_reason") == "pipeline_reason" 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_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="") diff --git a/tests/test_utils_console.py b/tests/test_utils_console.py index 0dea8af6d..fe6a7a97a 100644 --- a/tests/test_utils_console.py +++ b/tests/test_utils_console.py @@ -122,6 +122,11 @@ class TestIPythonShell: @pytest.mark.skipif( sys.platform == "win32", reason="requires a POSIX pseudo-terminal" ) + # The child of the pseudo-terminal fork execs right away, so the deadlocks + # that Python warns about cannot happen. + @pytest.mark.filterwarnings( + "ignore:.*is multi-threaded, use of forkpty:DeprecationWarning" + ) @pytest.mark.parametrize( "script", [CONSOLE, CONSOLE_IN_RUNNING_LOOP], 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), diff --git a/tests/test_utils_trackref.py b/tests/test_utils_trackref.py index 5458aa603..9585c6d83 100644 --- a/tests/test_utils_trackref.py +++ b/tests/test_utils_trackref.py @@ -124,3 +124,13 @@ def test_iter_all(): o2 = Bar() # noqa: F841 o3 = Foo() assert set(trackref.iter_all("Foo")) == {o1, o3} + + +def test_run_time_classes() -> None: + for _ in range(10): + base = type("Baz", (trackref.object_ref,), {}) + base() + del base + garbage_collect() + assert not list(trackref.iter_all("Baz")) + assert sum(1 for cls in trackref.live_refs if cls.__name__ == "Baz") == 0 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))