From f3a9e41d6b27ff8b560536d8afb305fd56f14815 Mon Sep 17 00:00:00 2001 From: Frank de Jonge Date: Tue, 3 May 2022 23:11:18 +0200 Subject: [PATCH] Use exceptions for connection root error resolving --- mocked-functions.php | 9 ++++ src/Ftp/FtpAdapter.php | 18 ++++++-- src/Ftp/FtpAdapterTest.php | 50 +++++++++++++++++++++++ src/Ftp/FtpConnectionProvider.php | 6 +-- src/Ftp/FtpConnectionProviderTest.php | 4 +- src/Ftp/UnableToResolveConnectionRoot.php | 30 ++++++++++++++ 6 files changed, 109 insertions(+), 8 deletions(-) create mode 100644 src/Ftp/UnableToResolveConnectionRoot.php diff --git a/mocked-functions.php b/mocked-functions.php index e6cb6d187..60038d93e 100644 --- a/mocked-functions.php +++ b/mocked-functions.php @@ -77,6 +77,15 @@ function ftp_pasv(...$arguments) return return_mocked_value('ftp_pasv'); } + function ftp_pwd(...$arguments) + { + if ( ! is_mocked('ftp_pwd')) { + return \ftp_pwd(...$arguments); + } + + return return_mocked_value('ftp_pwd'); + } + function ftp_fput(...$arguments) { if ( ! is_mocked('ftp_fput')) { diff --git a/src/Ftp/FtpAdapter.php b/src/Ftp/FtpAdapter.php index fd276b79e..61189fb5f 100644 --- a/src/Ftp/FtpAdapter.php +++ b/src/Ftp/FtpAdapter.php @@ -27,8 +27,10 @@ use League\MimeTypeDetection\MimeTypeDetector; use Throwable; +use function error_clear_last; +use function error_get_last; use function ftp_chdir; -use function ftp_pwd; +use function is_string; class FtpAdapter implements FilesystemAdapter { @@ -644,12 +646,20 @@ public function directoryExists(string $path): bool private function resolveConnectionRoot($connection): string { $root = $this->connectionOptions->root(); + error_clear_last(); - if ($root !== '') { - ftp_chdir($connection, $root); + if ($root !== '' && @ftp_chdir($connection, $root) !== true) { + throw UnableToResolveConnectionRoot::itDoesNotExist($root, error_get_last()['message'] ?? ''); } - return ftp_pwd($connection); + error_clear_last(); + $pwd = @ftp_pwd($connection); + + if ( ! is_string($pwd)) { + throw UnableToResolveConnectionRoot::couldNotGetCurrentDirectory(error_get_last()['message'] ?? ''); + } + + return $pwd; } /** diff --git a/src/Ftp/FtpAdapterTest.php b/src/Ftp/FtpAdapterTest.php index da74aa062..2656b139e 100644 --- a/src/Ftp/FtpAdapterTest.php +++ b/src/Ftp/FtpAdapterTest.php @@ -6,6 +6,9 @@ use League\Flysystem\FilesystemAdapter; +use function mock_function; +use function reset_function_mocks; + /** * @group ftp */ @@ -46,4 +49,51 @@ public function disconnect_after_destruct(): void static::clearFilesystemAdapterCache(); $this->assertFalse((new NoopCommandConnectivityChecker())->isConnected($connection)); } + + /** + * @test + */ + public function not_being_able_to_resolve_connection_root(): void + { + $options = FtpConnectionOptions::fromArray([ + 'host' => 'localhost', + 'port' => 2121, + 'timestampsOnUnixListingsEnabled' => true, + 'root' => '/invalid/root', + 'username' => 'foo', + 'password' => 'pass', + ]); + + $adapter = new FtpAdapter($options); + + $this->expectExceptionObject(UnableToResolveConnectionRoot::itDoesNotExist('/invalid/root')); + + $adapter->delete('something'); + } + + /** + * @test + */ + public function not_being_able_to_resolve_connection_root_pwd(): void + { + $options = FtpConnectionOptions::fromArray([ + 'host' => 'localhost', + 'port' => 2121, + 'timestampsOnUnixListingsEnabled' => true, + 'root' => '/home/foo/upload/', + 'username' => 'foo', + 'password' => 'pass', + ]); + + $this->expectExceptionObject(UnableToResolveConnectionRoot::couldNotGetCurrentDirectory()); + mock_function('ftp_pwd', false); + + $adapter = new FtpAdapter($options); + $adapter->delete('something'); + } + + protected function tearDown(): void + { + reset_function_mocks(); + } } diff --git a/src/Ftp/FtpConnectionProvider.php b/src/Ftp/FtpConnectionProvider.php index 04591eaa6..09e12a631 100644 --- a/src/Ftp/FtpConnectionProvider.php +++ b/src/Ftp/FtpConnectionProvider.php @@ -68,7 +68,7 @@ private function enableUtf8Mode(FtpConnectionOptions $options, $connection): voi return; } - $response = ftp_raw($connection, "OPTS UTF8 ON"); + $response = @ftp_raw($connection, "OPTS UTF8 ON"); if ( ! in_array(substr($response[0], 0, 3), ['200', '202'])) { throw new UnableToEnableUtf8Mode( @@ -88,7 +88,7 @@ private function ignorePassiveAddress(FtpConnectionOptions $options, $connection return; } - if ( ! ftp_set_option($connection, FTP_USEPASVADDRESS, ! $ignorePassiveAddress)) { + if ( ! @ftp_set_option($connection, FTP_USEPASVADDRESS, ! $ignorePassiveAddress)) { throw UnableToSetFtpOption::whileSettingOption('FTP_USEPASVADDRESS'); } } @@ -98,7 +98,7 @@ private function ignorePassiveAddress(FtpConnectionOptions $options, $connection */ private function makeConnectionPassive(FtpConnectionOptions $options, $connection): void { - if ( ! ftp_pasv($connection, $options->passive())) { + if ( ! @ftp_pasv($connection, $options->passive())) { throw new UnableToMakeConnectionPassive( 'Could not set passive mode for connection: ' . $options->host() . '::' . $options->port() ); diff --git a/src/Ftp/FtpConnectionProviderTest.php b/src/Ftp/FtpConnectionProviderTest.php index 6fe6ad70d..c5b1c9853 100644 --- a/src/Ftp/FtpConnectionProviderTest.php +++ b/src/Ftp/FtpConnectionProviderTest.php @@ -7,6 +7,8 @@ use League\Flysystem\AdapterTestUtilities\RetryOnTestException; use PHPUnit\Framework\TestCase; +use function ftp_close; + /** * @group ftp */ @@ -54,7 +56,7 @@ public function connecting_successfully(): void 'root' => '/home/foo/upload', 'username' => 'foo', 'password' => 'pass', - ]); + ]); $this->runScenario(function () use ($options) { $connection = $this->connectionProvider->createConnection($options); diff --git a/src/Ftp/UnableToResolveConnectionRoot.php b/src/Ftp/UnableToResolveConnectionRoot.php new file mode 100644 index 000000000..85836bcd2 --- /dev/null +++ b/src/Ftp/UnableToResolveConnectionRoot.php @@ -0,0 +1,30 @@ +