From 416754afe38ff3870c2d334e2a8ac26d823d0b27 Mon Sep 17 00:00:00 2001 From: Frank de Jonge Date: Sun, 3 Dec 2023 16:37:47 +0100 Subject: [PATCH] Capture more output from sftp connection errors and retry on auth failure too. --- src/PhpseclibV3/SftpConnectionProvider.php | 44 +++++++++++++------ .../SftpConnectionProviderTest.php | 10 +---- src/PhpseclibV3/UnableToAuthenticate.php | 25 ++++++++--- src/PhpseclibV3/UnableToConnectToSftpHost.php | 5 ++- src/PhpseclibV3/UnableToLoadPrivateKey.php | 5 ++- 5 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/PhpseclibV3/SftpConnectionProvider.php b/src/PhpseclibV3/SftpConnectionProvider.php index 86ee17269..996169f51 100644 --- a/src/PhpseclibV3/SftpConnectionProvider.php +++ b/src/PhpseclibV3/SftpConnectionProvider.php @@ -51,17 +51,29 @@ public function provideConnection(): SFTP { $tries = 0; start: + $tries++; - $connection = $this->connection instanceof SFTP - ? $this->connection - : $this->setupConnection(); + try { + $connection = $this->connection instanceof SFTP + ? $this->connection + : $this->setupConnection(); + } catch (Throwable $exception) { + if ($tries <= $this->maxTries) { + goto start; + } + + if ($exception instanceof FilesystemException) { + throw $exception; + } + + throw UnableToConnectToSftpHost::atHostname($this->host, $exception); + } if ( ! $this->connectivityChecker->isConnected($connection)) { $connection->disconnect(); $this->connection = null; - if ($tries < $this->maxTries) { - $tries++; + if ($tries <= $this->maxTries) { goto start; } @@ -82,10 +94,7 @@ private function setupConnection(): SFTP $this->authenticate($connection); } catch (Throwable $exception) { $connection->disconnect(); - - if ($exception instanceof FilesystemException) { - throw $exception; - } + throw $exception; } return $connection; @@ -124,8 +133,15 @@ private function authenticate(SFTP $connection): void $this->authenticateWithPrivateKey($connection); } elseif ($this->useAgent) { $this->authenticateWithAgent($connection); - } elseif ( ! $connection->login($this->username, $this->password)) { - throw UnableToAuthenticate::withPassword(); + } else { + $this->authenticateWithUsernameAndPassword($connection); + } + } + + private function authenticateWithUsernameAndPassword(SFTP $connection): void + { + if ( ! $connection->login($this->username, $this->password)) { + throw UnableToAuthenticate::withPassword($connection->getLastError()); } } @@ -159,7 +175,7 @@ private function authenticateWithPrivateKey(SFTP $connection): void return; } - throw UnableToAuthenticate::withPrivateKey(); + throw UnableToAuthenticate::withPrivateKey($connection->getLastError()); } private function loadPrivateKey(): AsymmetricKey @@ -175,7 +191,7 @@ private function loadPrivateKey(): AsymmetricKey return PublicKeyLoader::load($this->privateKey); } catch (NoKeyLoadedException $exception) { - throw new UnableToLoadPrivateKey(); + throw new UnableToLoadPrivateKey(null, $exception); } } @@ -184,7 +200,7 @@ private function authenticateWithAgent(SFTP $connection): void $agent = new Agent(); if ( ! $connection->login($this->username, $agent)) { - throw UnableToAuthenticate::withSshAgent(); + throw UnableToAuthenticate::withSshAgent($connection->getLastError()); } } } diff --git a/src/PhpseclibV3/SftpConnectionProviderTest.php b/src/PhpseclibV3/SftpConnectionProviderTest.php index 0b72fb9e0..a86310849 100644 --- a/src/PhpseclibV3/SftpConnectionProviderTest.php +++ b/src/PhpseclibV3/SftpConnectionProviderTest.php @@ -4,7 +4,6 @@ namespace League\Flysystem\PhpseclibV3; -use League\Flysystem\AdapterTestUtilities\ToxiproxyManagement; use phpseclib3\Net\SFTP; use PHPUnit\Framework\TestCase; use Throwable; @@ -277,13 +276,10 @@ public function isConnected(SFTP $connection): bool { ++$this->calls; - return $connection->isConnected(); + return false; } }; - $managesConnectionToxics = ToxiproxyManagement::forServer(); - $managesConnectionToxics->resetPeerOnRequest('sftp', 10); - $maxTries = 2; $provider = SftpConnectionProvider::fromArray( @@ -304,8 +300,6 @@ public function isConnected(SFTP $connection): bool try { $provider->provideConnection(); } finally { - $managesConnectionToxics->removeAllToxics(); - self::assertSame($maxTries + 1, $connectivityChecker->calls); } } @@ -389,7 +383,7 @@ public function runWithRetries(callable $scenario, string $expected = null): voi } catch (Throwable $exception) { if (($expected === null || is_a($exception, $expected) === false) && $tries < 10) { $tries++; - sleep($tries); +// sleep($tries); goto start; } diff --git a/src/PhpseclibV3/UnableToAuthenticate.php b/src/PhpseclibV3/UnableToAuthenticate.php index 0608e8c03..b4906fe2d 100644 --- a/src/PhpseclibV3/UnableToAuthenticate.php +++ b/src/PhpseclibV3/UnableToAuthenticate.php @@ -9,18 +9,31 @@ class UnableToAuthenticate extends RuntimeException implements FilesystemException { - public static function withPassword(): UnableToAuthenticate + private ?string $connectionError; + + public function __construct(string $message, string $lastError = null) + { + parent::__construct($message); + $this->connectionError = $lastError; + } + + public static function withPassword(string $lastError = null): UnableToAuthenticate + { + return new UnableToAuthenticate('Unable to authenticate using a password.', $lastError); + } + + public static function withPrivateKey(string $lastError = null): UnableToAuthenticate { - return new UnableToAuthenticate('Unable to authenticate using a password.'); + return new UnableToAuthenticate('Unable to authenticate using a private key.', $lastError); } - public static function withPrivateKey(): UnableToAuthenticate + public static function withSshAgent(string $lastError = null): UnableToAuthenticate { - return new UnableToAuthenticate('Unable to authenticate using a private key.'); + return new UnableToAuthenticate('Unable to authenticate using an SSH agent.', $lastError); } - public static function withSshAgent(): UnableToAuthenticate + public function connectionError(): ?string { - return new UnableToAuthenticate('Unable to authenticate using an SSH agent.'); + return $this->connectionError; } } diff --git a/src/PhpseclibV3/UnableToConnectToSftpHost.php b/src/PhpseclibV3/UnableToConnectToSftpHost.php index d075cf96e..052903fbd 100644 --- a/src/PhpseclibV3/UnableToConnectToSftpHost.php +++ b/src/PhpseclibV3/UnableToConnectToSftpHost.php @@ -6,11 +6,12 @@ use League\Flysystem\FilesystemException; use RuntimeException; +use Throwable; class UnableToConnectToSftpHost extends RuntimeException implements FilesystemException { - public static function atHostname(string $host): UnableToConnectToSftpHost + public static function atHostname(string $host, Throwable $previous = null): UnableToConnectToSftpHost { - return new UnableToConnectToSftpHost("Unable to connect to host: $host"); + return new UnableToConnectToSftpHost("Unable to connect to host: $host", 0, $previous); } } diff --git a/src/PhpseclibV3/UnableToLoadPrivateKey.php b/src/PhpseclibV3/UnableToLoadPrivateKey.php index 5737e01cd..16e2af5f4 100644 --- a/src/PhpseclibV3/UnableToLoadPrivateKey.php +++ b/src/PhpseclibV3/UnableToLoadPrivateKey.php @@ -6,11 +6,12 @@ use League\Flysystem\FilesystemException; use RuntimeException; +use Throwable; class UnableToLoadPrivateKey extends RuntimeException implements FilesystemException { - public function __construct(string $message = "Unable to load private key.") + public function __construct(?string $message = 'Unable to load private key.', ?Throwable $previous = null) { - parent::__construct($message); + parent::__construct($message ?? 'Unable to load private key.', 0, $previous); } }