Merge pull request #1763 from thephpleague/feature/close-sftp-and-ftp…

…-connections Add ability to proactively close FTP and SFTP connections.
thephpleague · Mar 9, 2024 · 85f70d1 · 85f70d1
2 parents 383e17d + 13969af
commit 85f70d1
Show file tree

Hide file tree

Showing 13 changed files with 119 additions and 24 deletions.
diff --git a/src/Ftp/FtpAdapter.php b/src/Ftp/FtpAdapter.php
@@ -30,14 +30,15 @@
 use function error_clear_last;
 use function error_get_last;
 use function ftp_chdir;
+use function ftp_close;
 use function is_string;
 
 class FtpAdapter implements FilesystemAdapter
 {
     private const SYSTEM_TYPE_WINDOWS = 'windows';
     private const SYSTEM_TYPE_UNIX = 'unix';
 
-    private FtpConnectionProvider $connectionProvider;
+    private ConnectionProvider $connectionProvider;
     private ConnectivityChecker $connectivityChecker;
 
     /**
@@ -55,7 +56,7 @@ class FtpAdapter implements FilesystemAdapter
 
     public function __construct(
         private FtpConnectionOptions $connectionOptions,
-        FtpConnectionProvider $connectionProvider = null,
+        ConnectionProvider $connectionProvider = null,
         ConnectivityChecker $connectivityChecker = null,
         VisibilityConverter $visibilityConverter = null,
         MimeTypeDetector $mimeTypeDetector = null,
@@ -74,10 +75,7 @@ public function __construct(
      */
     public function __destruct()
     {
-        if ($this->hasFtpConnection()) {
-            @ftp_close($this->connection);
-        }
-        $this->connection = false;
+        $this->disconnect();
     }
 
     /**
@@ -104,6 +102,14 @@ private function connection()
         return $this->connection;
     }
 
+    public function disconnect(): void
+    {
+        if ($this->hasFtpConnection()) {
+            @ftp_close($this->connection);
+        }
+        $this->connection = false;
+    }
+
     private function isPureFtpdServer(): bool
     {
         if ($this->isPureFtpdServer !== null) {

diff --git a/src/Ftp/FtpAdapterTest.php b/src/Ftp/FtpAdapterTest.php
@@ -26,8 +26,13 @@ protected static function createFilesystemAdapter(): FilesystemAdapter
        ]);
 
         static::$connectivityChecker = new ConnectivityCheckerThatCanFail(new NoopCommandConnectivityChecker());
+        static::$connectionProvider = new StubConnectionProvider(new FtpConnectionProvider());
 
-        return new FtpAdapter($options, null, static::$connectivityChecker);
+        return new FtpAdapter(
+            $options,
+            static::$connectionProvider,
+            static::$connectivityChecker,
+        );
     }
 
     /**
@@ -45,11 +50,26 @@ public function disconnect_after_destruct(): void
         unset($reflection);
 
         $this->assertTrue(false !== ftp_pwd($connection));
-        unset($adapter);
+        $adapter->__destruct();
         static::clearFilesystemAdapterCache();
         $this->assertFalse((new NoopCommandConnectivityChecker())->isConnected($connection));
     }
 
+    /**
+     * @test
+     */
+    public function it_can_disconnect(): void
+    {
+        /** @var FtpAdapter $adapter */
+        $adapter = $this->adapter();
+
+        $this->assertFalse($adapter->fileExists('not-existing.file'));
+
+        self::assertTrue(static::$connectivityChecker->isConnected(static::$connectionProvider->connection));
+        $adapter->disconnect();
+        self::assertFalse(static::$connectivityChecker->isConnected(static::$connectionProvider->connection));
+    }
+
     /**
      * @test
      */

diff --git a/src/Ftp/FtpAdapterTestCase.php b/src/Ftp/FtpAdapterTestCase.php
@@ -32,10 +32,9 @@ protected function setUp(): void
         $this->retryOnException(UnableToConnectToFtpHost::class);
     }
 
-    /**
-     * @var ConnectivityCheckerThatCanFail
-     */
-    protected static $connectivityChecker;
+    protected static ConnectivityCheckerThatCanFail $connectivityChecker;
+
+    protected static ?StubConnectionProvider $connectionProvider;
 
     /**
      * @after
@@ -45,6 +44,12 @@ public function resetFunctionMocks(): void
         reset_function_mocks();
     }
 
+    public static function clearFilesystemAdapterCache(): void
+    {
+        parent::clearFilesystemAdapterCache();
+        static::$connectionProvider = null;
+    }
+
     /**
      * @test
      */

diff --git a/src/Ftp/FtpConnectionProvider.php b/src/Ftp/FtpConnectionProvider.php
@@ -4,6 +4,8 @@
 
 namespace League\Flysystem\Ftp;
 
+use function error_clear_last;
+use function error_get_last;
 use const FTP_USEPASVADDRESS;
 
 class FtpConnectionProvider implements ConnectionProvider
@@ -40,10 +42,11 @@ public function createConnection(FtpConnectionOptions $options)
      */
     private function createConnectionResource(string $host, int $port, int $timeout, bool $ssl)
     {
+        error_clear_last();
         $connection = $ssl ? @ftp_ssl_connect($host, $port, $timeout) : @ftp_connect($host, $port, $timeout);
 
         if ($connection === false) {
-            throw UnableToConnectToFtpHost::forHost($host, $port, $ssl);
+            throw UnableToConnectToFtpHost::forHost($host, $port, $ssl, error_get_last()['message'] ?? '');
         }
 
         return $connection;

diff --git a/src/Ftp/NoopCommandConnectivityCheckerTest.php b/src/Ftp/NoopCommandConnectivityCheckerTest.php
@@ -26,7 +26,7 @@ public function detecting_a_good_connection(): void
     {
         $options = FtpConnectionOptions::fromArray([
            'host' => 'localhost',
-           'port' => 2122,
+           'port' => 2121,
            'root' => '/home/foo/upload',
            'username' => 'foo',
            'password' => 'pass',

diff --git a/src/Ftp/StubConnectionProvider.php b/src/Ftp/StubConnectionProvider.php
@@ -0,0 +1,18 @@
+<?php
+declare(strict_types=1);
+
+namespace League\Flysystem\Ftp;
+
+class StubConnectionProvider implements ConnectionProvider
+{
+    public mixed $connection;
+
+    public function __construct(private ConnectionProvider $provider)
+    {
+    }
+
+    public function createConnection(FtpConnectionOptions $options)
+    {
+        return $this->connection = $this->provider->createConnection($options);
+    }
+}
diff --git a/src/Ftp/UnableToConnectToFtpHost.php b/src/Ftp/UnableToConnectToFtpHost.php
@@ -8,10 +8,10 @@
 
 final class UnableToConnectToFtpHost extends RuntimeException implements FtpConnectionException
 {
-    public static function forHost(string $host, int $port, bool $ssl): UnableToConnectToFtpHost
+    public static function forHost(string $host, int $port, bool $ssl, string $reason = ''): UnableToConnectToFtpHost
     {
         $usingSsl = $ssl ? ', using ssl' : '';
 
-        return new UnableToConnectToFtpHost("Unable to connect to host $host at port $port$usingSsl.");
+        return new UnableToConnectToFtpHost("Unable to connect to host $host at port $port$usingSsl. $reason");
     }
 }
diff --git a/src/PhpseclibV3/ConnectionProvider.php b/src/PhpseclibV3/ConnectionProvider.php
@@ -6,6 +6,9 @@
 
 use phpseclib3\Net\SFTP;
 
+/**
+ * @method void disconnect()
+ */
 interface ConnectionProvider
 {
     public function provideConnection(): SFTP;

diff --git a/src/PhpseclibV3/SftpAdapter.php b/src/PhpseclibV3/SftpAdapter.php
@@ -58,6 +58,11 @@ public function fileExists(string $path): bool
         }
     }
 
+    public function disconnect(): void
+    {
+        $this->connectionProvider->disconnect();
+    }
+
     public function directoryExists(string $path): bool
     {
         $location = $this->prefixer->prefixDirectoryPath($path);

diff --git a/src/PhpseclibV3/SftpAdapterTest.php b/src/PhpseclibV3/SftpAdapterTest.php
@@ -30,7 +30,7 @@ public static function setUpBeforeClass(): void
     }
 
     /**
-     * @var ConnectionProvider
+     * @var StubSftpConnectionProvider
      */
     private static $connectionProvider;
 
@@ -214,7 +214,24 @@ public function list_contents_directory_does_not_exist(): void
         $this->assertCount(0, iterator_to_array($contents));
     }
 
-    private static function connectionProvider(): ConnectionProvider
+    /**
+     * @test
+     */
+    public function it_can_proactively_close_a_connection(): void
+    {
+        /** @var SftpAdapter $adapter */
+        $adapter = $this->adapter();
+
+        self::assertFalse($adapter->fileExists('does not exists at all'));
+
+        self::assertTrue(static::$connectionProvider->connection->isConnected());
+
+        $adapter->disconnect();
+
+        self::assertFalse(static::$connectionProvider->connection->isConnected());
+    }
+
+    private static function connectionProvider(): StubSftpConnectionProvider
     {
         if ( ! static::$connectionProvider instanceof ConnectionProvider) {
             static::$connectionProvider = new StubSftpConnectionProvider('localhost', 'foo', 'pass', 2222);
@@ -229,8 +246,7 @@ private static function connectionProvider(): ConnectionProvider
     private function adapterWithInvalidRoot(): SftpAdapter
     {
         $provider = static::connectionProvider();
-        $adapter = new SftpAdapter($provider, '/invalid');
 
-        return $adapter;
+        return new SftpAdapter($provider, '/invalid');
     }
 }
diff --git a/src/PhpseclibV3/SftpConnectionProvider.php b/src/PhpseclibV3/SftpConnectionProvider.php
@@ -83,6 +83,13 @@ public function provideConnection(): SFTP
         return $this->connection = $connection;
     }
 
+    public function disconnect(): void
+    {
+        if ($this->connection) {
+            $this->connection->disconnect();
+        }
+    }
+
     private function setupConnection(): SFTP
     {
         $connection = new SFTP($this->host, $this->port, $this->timeout);

diff --git a/src/PhpseclibV3/SimpleConnectivityChecker.php b/src/PhpseclibV3/SimpleConnectivityChecker.php
@@ -5,11 +5,16 @@
 namespace League\Flysystem\PhpseclibV3;
 
 use phpseclib3\Net\SFTP;
+use Throwable;
 
 class SimpleConnectivityChecker implements ConnectivityChecker
 {
     public function isConnected(SFTP $connection): bool
     {
-        return $connection->isConnected();
+        try {
+            return $connection->ping();
+        } catch (Throwable) {
+            return false;
+        }
     }
 }
diff --git a/src/PhpseclibV3/StubSftpConnectionProvider.php b/src/PhpseclibV3/StubSftpConnectionProvider.php
@@ -9,9 +9,9 @@
 class StubSftpConnectionProvider implements ConnectionProvider
 {
     /**
-     * @var SftpStub
+     * @var SftpStub|null
      */
-    private $connection;
+    public $connection;
 
     public function __construct(
         private string $host,
@@ -21,9 +21,16 @@ public function __construct(
     ) {
     }
 
+    public function disconnect(): void
+    {
+        if ($this->connection) {
+            $this->connection->disconnect();
+        }
+    }
+
     public function provideConnection(): SFTP
     {
-        if ( ! $this->connection instanceof SFTP) {
+        if ( ! $this->connection instanceof SFTP || ! $this->connection->isConnected()) {
             $connection = new SftpStub($this->host, $this->port);
             $connection->login($this->username, $this->password);