Add FTPS support to the FTP feed export storage (#7953)

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 <extras>` extra)
 -   :ref:`topics-feed-storage-gcs` (requires the :ref:`gcs <extras>` 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
 <https://stackoverflow.com/a/1699163>`_. 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 <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
+          <https://github.com/scrapy-plugins/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

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 <topics-feed-storage-ftp>` and
+temporary files before uploading with :ref:`FTP feed storage <feed-storage-ftp>` and
 :ref:`Amazon S3 <topics-feed-storage-s3>`.
 
 .. setting:: FEED_STORAGE_GCS_ACL

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,
         )
 
 

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",

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)

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())

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)

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="")