diff --git a/src/Ftp/FtpAdapter.php b/src/Ftp/FtpAdapter.php index 2f0e6dc8e..09c14f71e 100644 --- a/src/Ftp/FtpAdapter.php +++ b/src/Ftp/FtpAdapter.php @@ -30,6 +30,7 @@ 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 @@ -37,7 +38,7 @@ 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 index 2656b139e..20aaa09e0 100644 --- 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 index 49c9b3f76..2a0c15a81 100644 --- 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 index 09e12a631..67bbaa68d 100644 --- 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 index e15753865..e8299f686 100644 --- 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 new file mode 100644 index 000000000..f7c2ae075 --- /dev/null +++ b/src/Ftp/StubConnectionProvider.php @@ -0,0 +1,18 @@ +connection = $this->provider->createConnection($options); + } +} diff --git a/src/Ftp/UnableToConnectToFtpHost.php b/src/Ftp/UnableToConnectToFtpHost.php index 0543498bd..8d66908d7 100644 --- 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 index d7182cc99..6f15bffe0 100644 --- 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 index b47ed0695..5fa5f9dee 100644 --- 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 index bbcfc4b22..92d9a505f 100644 --- 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 index 996169f51..5869f3226 100644 --- 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 index 2e198f2ec..fddcdb566 100644 --- 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 index e75dc15c2..3818b876b 100644 --- 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);