Refactor connection options management to simplify API and improve consistency across SSH and rsync operations

Author: antonmedv

contrib/rsync.php

@@ -115,6 +115,7 @@
 
 use Deployer\Host\Localhost;
 use Deployer\Task\Context;
+use function Deployer\Support\rsync_rsh;
 
 set('rsync', [
     'exclude' => [
@@ -245,6 +246,6 @@
         return;
     }
 
-    $sshArguments = $host->connectionOptionsString();
-    runLocally("rsync {$rsyncFlags} -e 'ssh $sshArguments' {{rsync_options}}{{rsync_includes}}{{rsync_excludes}}{{rsync_filter}} '$src/' '{$host->connectionString()}:$dst/'", timeout: $config['timeout']);
+    $rsh = quote(rsync_rsh($host->connectionOptions()));
+    runLocally("rsync {$rsyncFlags} -e $rsh {{rsync_options}}{{rsync_includes}}{{rsync_excludes}}{{rsync_filter}} '$src/' '{$host->connectionString()}:$dst/'", timeout: $config['timeout']);
 });

src/Command/SshCommand.php

@@ -21,6 +21,7 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Question\ChoiceQuestion;
+use function Deployer\quote;
 
 /**
  * @codeCoverageIgnore
@@ -93,13 +94,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         Context::push(new Context($host));
         $host->setSshMultiplexing(false);
-        $options = $host->connectionOptionsString();
+        $connOptions = '';
+        foreach ($host->connectionOptions() as $option) {
+            $connOptions .= quote($option);
+        }
         $deployPath = $host->get('deploy_path', '~');
 
         if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
-            passthru("ssh -t $options {$host->connectionString()} \"cd $deployPath/current 2>/dev/null || cd $deployPath; $shell_path\"");
+            passthru("ssh -t $connOptions {$host->connectionString()} \"cd $deployPath/current 2>/dev/null || cd $deployPath; $shell_path\"");
         } else {
-            passthru("ssh -t $options {$host->connectionString()} 'cd $deployPath/current 2>/dev/null || cd $deployPath; $shell_path'");
+            passthru("ssh -t $connOptions {$host->connectionString()} 'cd $deployPath/current 2>/dev/null || cd $deployPath; $shell_path'");
         }
         return 0;
     }

src/Host/Host.php

@@ -266,15 +266,10 @@ public function connectionString(): string
         return $this->get('hostname');
     }
 
-    public function connectionOptionsString(): string
-    {
-        return implode(' ', array_map('Deployer\quote', $this->connectionOptionsArray()));
-    }
-
     /**
      * @return string[]
      */
-    public function connectionOptionsArray(): array
+    public function connectionOptions(): array
     {
         $options = [];
         if ($this->has('ssh_arguments')) {

src/Ssh/SshClient.php

@@ -40,7 +40,7 @@ public function run(Host $host, string $command, RunParams $params): string
             $shellCommand = "sudo -H -u {$host->get('become')} " . $shellCommand;
         }
 
-        $ssh = array_merge(['ssh'], $host->connectionOptionsArray(), [$host->connectionString(), ": $shellId; $shellCommand"]);
+        $ssh = array_merge(['ssh'], $host->connectionOptions(), [$host->connectionString(), ": $shellId; $shellCommand"]);
 
         // -vvv for ssh command
         if ($this->output->isDebug()) {

src/Support/helpers.php

@@ -233,3 +233,34 @@ function deployer_root(): string
         }
     }
 }
+
+/**
+ * Constructs an rsync-compatible remote shell command string using SSH and the provided options.
+ *
+ * @see https://github.com/RsyncProject/rsync/blob/4f6e4ea64ac3e2ac50f48103a22601f4a40ee8be/rsync.1.md?plain=1#L2152-L2162
+ *
+ *     Command-line arguments are permitted in COMMAND provided that COMMAND is
+ *     presented to rsync as a single argument.  You must use spaces (not tabs or
+ *     other whitespace) to separate the command and args from each other, and you
+ *     can use single- and/or double-quotes to preserve spaces in an argument (but
+ *     not backslashes).  Note that doubling a single-quote inside a single-quoted
+ *     string gives you a single-quote; likewise for double-quotes (though you
+ *     need to pay attention to which quotes your shell is parsing and which
+ *     quotes rsync is parsing).  Some examples:
+ *
+ *     -e 'ssh -p 2234'
+ *     -e 'ssh -o "ProxyCommand nohup ssh firewall nc -w1 %h %p"'
+ *
+ */
+function rsync_rsh(array $args): string
+{
+    $rsh = 'ssh ';
+    foreach ($args as $option) {
+        if (preg_match('/^[a-zA-Z0-9_\-=]+$/', $option)) {
+            $rsh .= ' ' . $option;
+        } else {
+            $rsh .= '"' . addslashes($option) . '" ';
+        }
+    }
+    return $rsh;
+}

src/Utility/Rsync.php

@@ -19,6 +19,7 @@
 use Symfony\Component\Process\Process;
 
 use function Deployer\quote;
+use function Deployer\Support\rsync_rsh;
 use function Deployer\writeln;
 
 class Rsync
@@ -58,9 +59,10 @@ public function call(Host $host, $source, string $destination, array $config = [
             $options[] = '--stats';
         }
 
-        $connectionOptions = $host->connectionOptionsString();
-        if ($connectionOptions !== '') {
-            $options = array_merge($options, ['-e', "ssh $connectionOptions"]);
+        $connectionOptions = $host->connectionOptions();
+        if (!empty($connectionOptions)) {
+            $rsh = rsync_rsh($connectionOptions);
+            $options = array_merge($options, ['-e', $rsh]);
         }
         if ($host->has('become') && !empty($host->get('become'))) {
             $options = array_merge($options, ['--rsync-path', "sudo -H -u {$host->get('become')} rsync"]);
@@ -75,12 +77,12 @@ function (string $value) {
             },
         ));
 
-        $commandString = $command[0];
+        $printCommandForDebug = $command[0];
         for ($i = 1; $i < count($command); $i++) {
-            $commandString .= ' ' . quote($command[$i]);
+            $printCommandForDebug .= ' ' . quote($command[$i]);
         }
         if ($this->output->isVerbose()) {
-            $this->output->writeln("[$host] $commandString");
+            $this->output->writeln("[$host] $printCommandForDebug");
         }
 
         $progressBar = null;
@@ -138,7 +140,7 @@ function (string $value) {
         } catch (ProcessFailedException $exception) {
             throw new RunException(
                 $host,
-                $commandString,
+                $printCommandForDebug,
                 $process->getExitCode(),
                 $process->getOutput(),
                 $process->getErrorOutput(),