[Process][Console] deprecated defining commands as strings

CHANGELOG.md

@@ -1,6 +1,13 @@
 CHANGELOG
 =========
 
+4.2.0
+-----
+
+ * added the `Process::fromShellCommandline()` to run commands in a shell wrapper
+ * deprecated passing a command as string when creating a `Process` instance
+ * deprecated the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods
+
 4.1.0
 -----
 

PhpProcess.php

@@ -29,11 +29,12 @@ class PhpProcess extends Process
      * @param string|null $cwd     The working directory or null to use the working dir of the current PHP process
      * @param array|null  $env     The environment variables or null to use the same environment as the current PHP process
      * @param int         $timeout The timeout in seconds
+     * @param array|null  $php     Path to the PHP binary to use with any additional arguments
      */
-    public function __construct(string $script, string $cwd = null, array $env = null, int $timeout = 60)
+    public function __construct(string $script, string $cwd = null, array $env = null, int $timeout = 60, array $php = null)
     {
         $executableFinder = new PhpExecutableFinder();
-        if (false === $php = $executableFinder->find(false)) {
+        if (false === $php = $php ?? $executableFinder->find(false)) {
             $php = null;
         } else {
             $php = array_merge(array($php), $executableFinder->findArguments());
@@ -51,9 +52,13 @@ public function __construct(string $script, string $cwd = null, array $env = nul
 
     /**
      * Sets the path to the PHP binary to use.
+     *
+     * @deprecated since Symfony 4.2, use the $php argument of the constructor instead.
      */
     public function setPhpBinary($php)
     {
+        @trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2, use the $php argument of the constructor instead.', __METHOD__), E_USER_DEPRECATED);
+
         $this->setCommandLine($php);
     }
 

Process.php

@@ -129,21 +129,25 @@ class Process implements \IteratorAggregate
     );
 
     /**
-     * @param string|array   $commandline The command line to run
-     * @param string|null    $cwd         The working directory or null to use the working dir of the current PHP process
-     * @param array|null     $env         The environment variables or null to use the same environment as the current PHP process
-     * @param mixed|null     $input       The input as stream resource, scalar or \Traversable, or null for no input
-     * @param int|float|null $timeout     The timeout in seconds or null to disable
+     * @param array          $command The command to run and its arguments listed as separate entries
+     * @param string|null    $cwd     The working directory or null to use the working dir of the current PHP process
+     * @param array|null     $env     The environment variables or null to use the same environment as the current PHP process
+     * @param mixed|null     $input   The input as stream resource, scalar or \Traversable, or null for no input
+     * @param int|float|null $timeout The timeout in seconds or null to disable
      *
      * @throws RuntimeException When proc_open is not installed
      */
-    public function __construct($commandline, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
+    public function __construct($command, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
     {
         if (!function_exists('proc_open')) {
             throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
         }
 
-        $this->commandline = $commandline;
+        if (!\is_array($command)) {
+            @trigger_error(sprintf('Passing a command as string when creating a "%s" instance is deprecated since Symfony 4.2, pass it as an array of its arguments instead, or use the "Process::fromShellCommandline()" constructor if you need features provided by the shell.', __CLASS__), E_USER_DEPRECATED);
+        }
+
+        $this->commandline = $command;
         $this->cwd = $cwd;
 
         // on Windows, if the cwd changed via chdir(), proc_open defaults to the dir where PHP was started
@@ -163,6 +167,35 @@ public function __construct($commandline, string $cwd = null, array $env = null,
         $this->pty = false;
     }
 
+    /**
+     * Creates a Process instance as a command-line to be run in a shell wrapper.
+     *
+     * Command-lines are parsed by the shell of your OS (/bin/sh on Unix-like, cmd.exe on Windows.)
+     * This allows using e.g. pipes or conditional execution. In this mode, signals are sent to the
+     * shell wrapper and not to your commands.
+     *
+     * In order to inject dynamic values into command-lines, we strongly recommend using placeholders.
+     * This will save escaping values, which is not portable nor secure anyway:
+     *
+     *   $process = Process::fromShellCommandline('my_command "$MY_VAR"');
+     *   $process->run(null, ['MY_VAR' => $theValue]);
+     *
+     * @param string         $command The command line to pass to the shell of the OS
+     * @param string|null    $cwd     The working directory or null to use the working dir of the current PHP process
+     * @param array|null     $env     The environment variables or null to use the same environment as the current PHP process
+     * @param mixed|null     $input   The input as stream resource, scalar or \Traversable, or null for no input
+     * @param int|float|null $timeout The timeout in seconds or null to disable
+     *
+     * @throws RuntimeException When proc_open is not installed
+     */
+    public static function fromShellCommandline(string $command, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
+    {
+        $process = new static(array(), $cwd, $env, $input, $timeout);
+        $process->commandline = $command;
+
+        return $process;
+    }
+
     public function __destruct()
     {
         $this->stop(0);
@@ -892,9 +925,13 @@ public function getCommandLine()
      * @param string|array $commandline The command to execute
      *
      * @return self The current Process instance
+     *
+     * @deprecated since Symfony 4.2.
      */
     public function setCommandLine($commandline)
     {
+        @trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2.', __METHOD__), E_USER_DEPRECATED);
+
         $this->commandline = $commandline;
 
         return $this;

Tests/ProcessFailedExceptionTest.php

@@ -24,7 +24,7 @@ class ProcessFailedExceptionTest extends TestCase
      */
     public function testProcessFailedExceptionThrowsException()
     {
-        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful'))->setConstructorArgs(array('php'))->getMock();
+        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful'))->setConstructorArgs(array(array('php')))->getMock();
         $process->expects($this->once())
             ->method('isSuccessful')
             ->will($this->returnValue(true));
@@ -52,7 +52,7 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(
         $errorOutput = 'FATAL: Unexpected error';
         $workingDirectory = getcwd();
 
-        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'getOutput', 'getErrorOutput', 'getExitCode', 'getExitCodeText', 'isOutputDisabled', 'getWorkingDirectory'))->setConstructorArgs(array($cmd))->getMock();
+        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'getOutput', 'getErrorOutput', 'getExitCode', 'getExitCodeText', 'isOutputDisabled', 'getWorkingDirectory'))->setConstructorArgs(array(array($cmd)))->getMock();
         $process->expects($this->once())
             ->method('isSuccessful')
             ->will($this->returnValue(false));
@@ -85,7 +85,7 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(
 
         $this->assertEquals(
             "The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
-            $exception->getMessage()
+            str_replace("'php'", 'php', $exception->getMessage())
         );
     }
 
@@ -100,7 +100,7 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()
         $exitText = 'General error';
         $workingDirectory = getcwd();
 
-        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'isOutputDisabled', 'getExitCode', 'getExitCodeText', 'getOutput', 'getErrorOutput', 'getWorkingDirectory'))->setConstructorArgs(array($cmd))->getMock();
+        $process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'isOutputDisabled', 'getExitCode', 'getExitCodeText', 'getOutput', 'getErrorOutput', 'getWorkingDirectory'))->setConstructorArgs(array(array($cmd)))->getMock();
         $process->expects($this->once())
             ->method('isSuccessful')
             ->will($this->returnValue(false));
@@ -131,7 +131,7 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()
 
         $this->assertEquals(
             "The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
-            $exception->getMessage()
+            str_replace("'php'", 'php', $exception->getMessage())
         );
     }
 }

Tests/ProcessTest.php

@@ -55,13 +55,13 @@ public function testInvalidCwd()
     {
         try {
             // Check that it works fine if the CWD exists
-            $cmd = new Process('echo test', __DIR__);
+            $cmd = new Process(array('echo', 'test'), __DIR__);
             $cmd->run();
         } catch (\Exception $e) {
             $this->fail($e);
         }
 
-        $cmd = new Process('echo test', __DIR__.'/notfound/');
+        $cmd = new Process(array('echo', 'test'), __DIR__.'/notfound/');
         $cmd->run();
     }
 
@@ -1447,7 +1447,7 @@ public function testEscapeArgument($arg)
 
     public function testRawCommandLine()
     {
-        $p = new Process(sprintf('"%s" -r %s "a" "" "b"', self::$phpBin, escapeshellarg('print_r($argv);')));
+        $p = Process::fromShellCommandline(sprintf('"%s" -r %s "a" "" "b"', self::$phpBin, escapeshellarg('print_r($argv);')));
         $p->run();
 
         $expected = <<<EOTXT
@@ -1478,26 +1478,20 @@ public function testEnvArgument()
     {
         $env = array('FOO' => 'Foo', 'BAR' => 'Bar');
         $cmd = '\\' === DIRECTORY_SEPARATOR ? 'echo !FOO! !BAR! !BAZ!' : 'echo $FOO $BAR $BAZ';
-        $p = new Process($cmd, null, $env);
+        $p = Process::fromShellCommandline($cmd, null, $env);
         $p->run(null, array('BAR' => 'baR', 'BAZ' => 'baZ'));
 
         $this->assertSame('Foo baR baZ', rtrim($p->getOutput()));
         $this->assertSame($env, $p->getEnv());
     }
 
-    /**
-     * @param string      $commandline
-     * @param null|string $cwd
-     * @param null|array  $env
-     * @param null|string $input
-     * @param int         $timeout
-     * @param array       $options
-     *
-     * @return Process
-     */
-    private function getProcess($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60)
+    private function getProcess($commandline, string $cwd = null, array $env = null, $input = null, ?int $timeout = 60): Process
     {
-        $process = new Process($commandline, $cwd, $env, $input, $timeout);
+        if (\is_string($commandline)) {
+            $process = Process::fromShellCommandline($commandline, $cwd, $env, $input, $timeout);
+        } else {
+            $process = new Process($commandline, $cwd, $env, $input, $timeout);
+        }
         $process->inheritEnvironmentVariables();
 
         if (self::$process) {
@@ -1507,10 +1501,7 @@ private function getProcess($commandline, $cwd = null, array $env = null, $input
         return self::$process = $process;
     }
 
-    /**
-     * @return Process
-     */
-    private function getProcessForCode($code, $cwd = null, array $env = null, $input = null, $timeout = 60)
+    private function getProcessForCode(string $code, string $cwd = null, array $env = null, $input = null, ?int $timeout = 60): Process
     {
         return $this->getProcess(array(self::$phpBin, '-r', $code), $cwd, $env, $input, $timeout);
     }