Capture more output from sftp connection errors and retry on auth fai…

…lure too.
thephpleague · Dec 3, 2023 · 416754a · 416754a
1 parent ced52ce
commit 416754a
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
     }
 }