Repository: PyFilesystem/pyfilesystem2
Pull request #449 (2021-01-31 18:08:14)

FTPS support
## Type of changes

- New feature

## Checklist

- [x] I've run the latest [black](https://github.com/ambv/black) with default args on new code.
- [x] I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
- [x] I've added tests for new code.
- [x] I accept that @PyFilesystem/maintainers may be pedantic in the code review.

## Description

Added FTPS (FTP over TLS) support to FTPFS. Addresses #437.

* Try to import FTP_TLS from the standard library, but if it's not available just default to a plain FTP connection, and set the `tls` attribute to False
* Add a boolean `tls` argument to the constructor, and as an attribute
* Add details and examples to the class docstring
* Show the ftps:// scheme in the URL when TLS is enabled
* Enable TLS when the ftps:// scheme is used in open/open_fs
* Add basic test



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b75569f..31cb320 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Added
+
+- Added FTP over TLS (FTPS) support to FTPFS.
+  Closes [#437](https://github.com/PyFilesystem/pyfilesystem2/issues/437),
+  [#449](https://github.com/PyFilesystem/pyfilesystem2/pull/449).
+
 ### Changed
 
 - Make `FS.upload` explicit about the expected error when the parent directory of the destination does not exist.
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
index 3c2729c..46408d8 100644
--- a/CONTRIBUTORS.md
+++ b/CONTRIBUTORS.md
@@ -13,5 +13,6 @@ Many thanks to the following developers for contributing to this project:
 - [Martin Larralde](https://github.com/althonos)
 - [Morten Engelhardt Olsen](https://github.com/xoriath)
 - [Nick Henderson](https://github.com/nwh)
+- [Oliver Galvin](https://github.com/odgalvin)
 - [Will McGugan](https://github.com/willmcgugan)
 - [Zmej Serow](https://github.com/zmej-serow)
diff --git a/fs/ftpfs.py b/fs/ftpfs.py
index d2e37d7..3d8b094 100644
--- a/fs/ftpfs.py
+++ b/fs/ftpfs.py
@@ -14,12 +14,18 @@ import typing
 from collections import OrderedDict
 from contextlib import contextmanager
 from ftplib import FTP
+
+try:
+    from ftplib import FTP_TLS
+except ImportError as err:
+    FTP_TLS = err  # type: ignore
 from ftplib import error_perm
 from ftplib import error_temp
 from typing import cast
 
 from six import PY2
 from six import text_type
+from six import raise_from
 
 from . import errors
 from .base import FS
@@ -346,7 +352,30 @@ class FTPFile(io.RawIOBase):
 
 
 class FTPFS(FS):
-    """A FTP (File Transport Protocol) Filesystem."""
+    """A FTP (File Transport Protocol) Filesystem.
+
+    Optionally, the connection can be made securely via TLS. This is known as
+    FTPS, or FTP Secure. TLS will be enabled when using the ftps:// protocol,
+    or when setting the `tls` argument to True in the constructor.
+
+
+    Examples:
+        Create with the constructor::
+
+            >>> from fs.ftpfs import FTPFS
+            >>> ftp_fs = FTPFS()
+
+        Or via an FS URL::
+
+            >>> import fs
+            >>> ftp_fs = fs.open_fs('ftp://')
+
+        Or via an FS URL, using TLS::
+
+            >>> import fs
+            >>> ftp_fs = fs.open_fs('ftps://')
+
+    """
 
     _meta = {
         "invalid_path_chars": "\0",
@@ -366,6 +395,7 @@ class FTPFS(FS):
         timeout=10,  # type: int
         port=21,  # type: int
         proxy=None,  # type: Optional[Text]
+        tls=False,  # type: bool
     ):
         # type: (...) -> None
         """Create a new `FTPFS` instance.
@@ -380,6 +410,7 @@ class FTPFS(FS):
             port (int): FTP port number (default 21).
             proxy (str, optional): An FTP proxy, or ``None`` (default)
                 for no proxy.
+            tls (bool): Attempt to use FTP over TLS (FTPS) (default: False)
 
         """
         super(FTPFS, self).__init__()
@@ -390,6 +421,10 @@ class FTPFS(FS):
         self.timeout = timeout
         self.port = port
         self.proxy = proxy
+        self.tls = tls
+
+        if self.tls and isinstance(FTP_TLS, Exception):
+            raise_from(errors.CreateFailed("FTP over TLS not supported"), FTP_TLS)
 
         self.encoding = "latin-1"
         self._ftp = None  # type: Optional[FTP]
@@ -432,11 +467,15 @@ class FTPFS(FS):
     def _open_ftp(self):
         # type: () -> FTP
         """Open a new ftp object."""
-        _ftp = FTP()
+        _ftp = FTP_TLS() if self.tls else FTP()
         _ftp.set_debuglevel(0)
         with ftp_errors(self):
             _ftp.connect(self.host, self.port, self.timeout)
             _ftp.login(self.user, self.passwd, self.acct)
+            try:
+                _ftp.prot_p()  # type: ignore
+            except AttributeError:
+                pass
             self._features = {}
             try:
                 feat_response = _decode(_ftp.sendcmd("FEAT"), "latin-1")
@@ -471,7 +510,9 @@ class FTPFS(FS):
             _user_part = ""
         else:
             _user_part = "{}:{}@".format(self.user, self.passwd)
-        url = "ftp://{}{}".format(_user_part, _host_part)
+
+        scheme = "ftps" if self.tls else "ftp"
+        url = "{}://{}{}".format(scheme, _user_part, _host_part)
         return url
 
     @property
diff --git a/fs/opener/ftpfs.py b/fs/opener/ftpfs.py
index af64606..cd313a3 100644
--- a/fs/opener/ftpfs.py
+++ b/fs/opener/ftpfs.py
@@ -23,7 +23,7 @@ if typing.TYPE_CHECKING:
 class FTPOpener(Opener):
     """`FTPFS` opener."""
 
-    protocols = ["ftp"]
+    protocols = ["ftp", "ftps"]
 
     @CreateFailed.catch_all
     def open_fs(
@@ -48,6 +48,7 @@ class FTPOpener(Opener):
             passwd=parse_result.password,
             proxy=parse_result.params.get("proxy"),
             timeout=int(parse_result.params.get("timeout", "10")),
+            tls=bool(parse_result.protocol == "ftps"),
         )
         if dir_path:
             if create:


Test patch:
diff --git a/tests/test_ftpfs.py b/tests/test_ftpfs.py
index fd34c13..b414283 100644
--- a/tests/test_ftpfs.py
+++ b/tests/test_ftpfs.py
@@ -88,6 +88,10 @@ class TestFTPFSClass(unittest.TestCase):
         self.assertIsInstance(ftp_fs, FTPFS)
         self.assertEqual(ftp_fs.host, "ftp.example.org")
 
+        ftps_fs = open_fs("ftps://will:wfc@ftp.example.org")
+        self.assertIsInstance(ftps_fs, FTPFS)
+        self.assertTrue(ftps_fs.tls)
+
 
 class TestFTPErrors(unittest.TestCase):
     """Test the ftp_errors context manager."""
diff --git a/tests/test_opener.py b/tests/test_opener.py
index fde555a..e7fae98 100644
--- a/tests/test_opener.py
+++ b/tests/test_opener.py
@@ -300,7 +300,14 @@ class TestOpeners(unittest.TestCase):
     def test_open_ftp(self, mock_FTPFS):
         open_fs("ftp://foo:bar@ftp.example.org")
         mock_FTPFS.assert_called_once_with(
-            "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10
+            "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=False
+        )
+
+    @mock.patch("fs.ftpfs.FTPFS")
+    def test_open_ftps(self, mock_FTPFS):
+        open_fs("ftps://foo:bar@ftp.example.org")
+        mock_FTPFS.assert_called_once_with(
+            "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=True
         )
 
     @mock.patch("fs.ftpfs.FTPFS")
@@ -313,4 +320,5 @@ class TestOpeners(unittest.TestCase):
             user="foo",
             proxy="ftp.proxy.org",
             timeout=10,
+            tls=False,
         )