[Process] Accept command line arrays and per-run env vars, fixing sig…

…naling and escaping
symfony · fabpot · Feb 8, 2017 · Jan 12, 2017 · Feb 1, 2017 · 330b61fecbef76a2e0c275cbfaee1dbeab176468
commit 330b61fecbef76a2e0c275cbfaee1dbeab176468
diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md
@@ -66,6 +66,8 @@ HttpKernel
 Process
 -------
 
+ * The `ProcessUtils::escapeArgument()` method has been deprecated, use a command line array or give env vars to the `Process::start/run()` method instead.
+
  * Not inheriting environment variables is deprecated.
 
  * Configuring `proc_open()` options is deprecated.

diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -228,6 +228,8 @@ HttpKernel
 Process
 -------
 
+ * The `ProcessUtils::escapeArgument()` method has been removed, use a command line array or give env vars to the `Process::start/run()` method instead.
+
  * Environment variables are always inherited in sub-processes.
 
  * Configuring `proc_open()` options has been removed.

diff --git a/src/Symfony/Component/Process/CHANGELOG.md b/src/Symfony/Component/Process/CHANGELOG.md
@@ -4,6 +4,9 @@ CHANGELOG
 3.3.0
 -----
 
+ * added command line arrays in the `Process` class
+ * added `$env` argument to `Process::start()`, `run()`, `mustRun()` and `restart()` methods
+ * deprecated the `ProcessUtils::escapeArgument()` method
  * deprecated not inheriting environment variables
  * deprecated configuring `proc_open()` options
  * deprecated configuring enhanced Windows compatibility

diff --git a/src/Symfony/Component/Process/PhpProcess.php b/src/Symfony/Component/Process/PhpProcess.php
@@ -38,20 +38,16 @@ public function __construct($script, $cwd = null, array $env = null, $timeout =
         $executableFinder = new PhpExecutableFinder();
         if (false === $php = $executableFinder->find()) {
             $php = null;
+        } else {
+            $php = explode(' ', $php);
         }
         if ('phpdbg' === PHP_SAPI) {
             $file = tempnam(sys_get_temp_dir(), 'dbg');
             file_put_contents($file, $script);
             register_shutdown_function('unlink', $file);
-            $php .= ' '.ProcessUtils::escapeArgument($file);
+            $php[] = $file;
             $script = null;
         }
-        if ('\\' !== DIRECTORY_SEPARATOR && null !== $php) {
-            // exec is mandatory to deal with sending a signal to the process
-            // see https://github.com/symfony/symfony/issues/5030 about prepending
-            // command with exec
-            $php = 'exec '.$php;
-        }
         if (null !== $options) {
             @trigger_error(sprintf('The $options parameter of the %s constructor is deprecated since version 3.3 and will be removed in 4.0.', __CLASS__), E_USER_DEPRECATED);
         }
@@ -70,12 +66,13 @@ public function setPhpBinary($php)
     /**
      * {@inheritdoc}
      */
-    public function start(callable $callback = null)
+    public function start(callable $callback = null/*, array $env = array()*/)
     {
         if (null === $this->getCommandLine()) {
             throw new RuntimeException('Unable to find the PHP executable.');
         }
+        $env = 1 < func_num_args() ? func_get_arg(1) : null;
 
-        parent::start($callback);
+        parent::start($callback, $env);
     }
 }
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -136,7 +136,7 @@ class Process implements \IteratorAggregate
     /**
      * Constructor.
      *
-     * @param string         $commandline The command line to run
+     * @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
@@ -151,7 +151,7 @@ public function __construct($commandline, $cwd = null, array $env = null, $input
             throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
         }
 
-        $this->commandline = $commandline;
+        $this->setCommandline($commandline);
         $this->cwd = $cwd;
 
         // on Windows, if the cwd changed via chdir(), proc_open defaults to the dir where PHP was started
@@ -199,16 +199,20 @@ public function __clone()
      *
      * @param callable|null $callback A PHP callback to run whenever there is some
      *                                output available on STDOUT or STDERR
+     * @param array         $env      An array of additional env vars to set when running the process
      *
      * @return int The exit status code
      *
      * @throws RuntimeException When process can't be launched
      * @throws RuntimeException When process stopped after receiving signal
      * @throws LogicException   In case a callback is provided and output has been disabled
+     *
+     * @final since version 3.3
      */
-    public function run($callback = null)
+    public function run($callback = null/*, array $env = array()*/)
     {
-        $this->start($callback);
+        $env = 1 < func_num_args() ? func_get_arg(1) : null;
+        $this->start($callback, $env);
 
         return $this->wait();
     }
@@ -220,19 +224,23 @@ public function run($callback = null)
      * exits with a non-zero exit code.
      *
      * @param callable|null $callback
+     * @param array         $env      An array of additional env vars to set when running the process
      *
      * @return self
      *
      * @throws RuntimeException       if PHP was compiled with --enable-sigchild and the enhanced sigchild compatibility mode is not enabled
      * @throws ProcessFailedException if the process didn't terminate successfully
+     *
+     * @final since version 3.3
      */
-    public function mustRun(callable $callback = null)
+    public function mustRun(callable $callback = null/*, array $env = array()*/)
     {
         if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
             throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.');
         }
+        $env = 1 < func_num_args() ? func_get_arg(1) : null;
 
-        if (0 !== $this->run($callback)) {
+        if (0 !== $this->run($callback, $env)) {
             throw new ProcessFailedException($this);
         }
 
@@ -253,28 +261,48 @@ public function mustRun(callable $callback = null)
      *
      * @param callable|null $callback A PHP callback to run whenever there is some
      *                                output available on STDOUT or STDERR
+     * @param array         $env      An array of additional env vars to set when running the process
      *
      * @throws RuntimeException When process can't be launched
      * @throws RuntimeException When process is already running
      * @throws LogicException   In case a callback is provided and output has been disabled
      */
-    public function start(callable $callback = null)
+    public function start(callable $callback = null/*, array $env = array()*/)
     {
         if ($this->isRunning()) {
             throw new RuntimeException('Process is already running');
         }
+        if (2 <= func_num_args()) {
+            $env = func_get_arg(1);
+        } else {
+            if (__CLASS__ !== static::class) {
+                $r = new \ReflectionMethod($this, __FUNCTION__);
+                if (__CLASS__ !== $r->getDeclaringClass()->getName() && (2 > $r->getNumberOfParameters() || 'env' !== $r->getParameters()[0]->name)) {
+                    @trigger_error(sprintf('The %s::start() method expects a second "$env" argument since version 3.3. It will be made mandatory in 4.0.', static::class), E_USER_DEPRECATED);
+                }
+            }
+            $env = null;
+        }
 
         $this->resetProcessData();
         $this->starttime = $this->lastOutputTime = microtime(true);
         $this->callback = $this->buildCallback($callback);
         $this->hasCallback = null !== $callback;
         $descriptors = $this->getDescriptors();
-
+        $inheritEnv = $this->inheritEnv;
         $commandline = $this->commandline;
 
-        $env = $this->env;
+        if (null === $env) {
+            $env = $this->env;
+        } else {
+            if ($this->env) {
+                $env += $this->env;
+            }
+            $inheritEnv = true;
+        }
+
         $envBackup = array();
-        if (null !== $env && $this->inheritEnv) {
+        if (null !== $env && $inheritEnv) {
             foreach ($env as $k => $v) {
                 $envBackup[$k] = getenv($v);
                 putenv(false === $v || null === $v ? $k : "$k=$v");
@@ -284,14 +312,8 @@ public function start(callable $callback = null)
             @trigger_error(sprintf('Not inheriting environment variables is deprecated since Symfony 3.3 and will always happen in 4.0. Set "Process::inheritEnvironmentVariables()" to true instead.', __METHOD__), E_USER_DEPRECATED);
         }
         if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) {
-            $commandline = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $commandline).')';
-            foreach ($this->processPipes->getFiles() as $offset => $filename) {
-                $commandline .= ' '.$offset.'>"'.$filename.'"';
-            }
-
-            if (!isset($this->options['bypass_shell'])) {
-                $this->options['bypass_shell'] = true;
-            }
+            $this->options['bypass_shell'] = true;
+            $commandline = $this->prepareWindowsCommandLine($commandline, $envBackup);
         } elseif (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
             // last exit code is output on the fourth pipe and caught to work around --enable-sigchild
             $descriptors[3] = array('pipe', 'w');
@@ -335,22 +357,26 @@ public function start(callable $callback = null)
      *
      * @param callable|null $callback A PHP callback to run whenever there is some
      *                                output available on STDOUT or STDERR
+     * @param array         $env      An array of additional env vars to set when running the process
      *
      * @return $this
      *
      * @throws RuntimeException When process can't be launched
      * @throws RuntimeException When process is already running
      *
      * @see start()
+     *
+     * @final since version 3.3
      */
-    public function restart(callable $callback = null)
+    public function restart(callable $callback = null/*, array $env = array()*/)
     {
         if ($this->isRunning()) {
             throw new RuntimeException('Process is already running');
         }
+        $env = 1 < func_num_args() ? func_get_arg(1) : null;
 
         $process = clone $this;
-        $process->start($callback);
+        $process->start($callback, $env);
 
         return $process;
     }
@@ -909,12 +935,20 @@ public function getCommandLine()
     /**
      * Sets the command line to be executed.
      *
-     * @param string $commandline The command to execute
+     * @param string|array $commandline The command to execute
      *
      * @return self The current Process instance
      */
     public function setCommandLine($commandline)
     {
+        if (is_array($commandline)) {
+            $commandline = implode(' ', array_map(array($this, 'escapeArgument'), $commandline));
+
+            if ('\\' !== DIRECTORY_SEPARATOR) {
+                // exec is mandatory to deal with sending a signal to the process
+                $commandline = 'exec '.$commandline;
+            }
+        }
         $this->commandline = $commandline;
 
         return $this;
@@ -1589,6 +1623,50 @@ private function doSignal($signal, $throwException)
         return true;
     }
 
+    private function prepareWindowsCommandLine($cmd, array &$envBackup)
+    {
+        $uid = uniqid('', true);
+        $varCount = 0;
+        $varCache = array();
+        $cmd = preg_replace_callback(
+            '/"(
+                [^"%!^]*+
+                (?:
+                    (?: !LF! | "(?:\^[%!^])?+" )
+                    [^"%!^]*+
+                )++
+            )"/x',
+            function ($m) use (&$envBackup, &$varCache, &$varCount, $uid) {
+                if (isset($varCache[$m[0]])) {
+                    return $varCache[$m[0]];
+                }
+                if (false !== strpos($value = $m[1], "\0")) {
+                    $value = str_replace("\0", '?', $value);
+                }
+                if (false === strpbrk($value, "\"%!\n")) {
+                    return '"'.$value.'"';
+                }
+
+                $value = str_replace(array('!LF!', '"^!"', '"^%"', '"^^"', '""'), array("\n", '!', '%', '^', '"'), $value);
+                $value = preg_replace('/(\\\\*)"/', '$1$1\\"', $value);
+
+                $var = $uid.++$varCount;
+                putenv("$var=\"$value\"");
+                $envBackup[$var] = false;
+
+                return $varCache[$m[0]] = '!'.$var.'!';
+            },
+            $cmd
+        );
+
+        $cmd = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
+        foreach ($this->processPipes->getFiles() as $offset => $filename) {
+            $cmd .= ' '.$offset.'>"'.$filename.'"';
+        }
+
+        return $cmd;
+    }
+
     /**
      * Ensures the process is running or terminated, throws a LogicException if the process has a not started.
      *
@@ -1616,4 +1694,30 @@ private function requireProcessIsTerminated($functionName)
             throw new LogicException(sprintf('Process must be terminated before calling %s.', $functionName));
         }
     }
+
+    /**
+     * Escapes a string to be used as a shell argument.
+     *
+     * @param string $argument The argument that will be escaped
+     *
+     * @return string The escaped argument
+     */
+    private function escapeArgument($argument)
+    {
+        if ('\\' !== DIRECTORY_SEPARATOR) {
+            return "'".str_replace("'", "'\\''", $argument)."'";
+        }
+        if ('' === $argument = (string) $argument) {
+            return '""';
+        }
+        if (false !== strpos($argument, "\0")) {
+            $argument = str_replace("\0", '?', $argument);
+        }
+        if (!preg_match('/[()%!^"<>&|\s]/', $argument)) {
+            return $argument;
+        }
+        $argument = preg_replace('/(\\\\+)$/', '$1$1', $argument);
+
+        return '"'.str_replace(array('"', '^', '%', '!', "\n"), array('""', '"^^"', '"^%"', '"^!"', '!LF!'), $argument).'"';
+    }
 }
diff --git a/src/Symfony/Component/Process/ProcessBuilder.php b/src/Symfony/Component/Process/ProcessBuilder.php
@@ -271,9 +271,7 @@ public function getProcess()
         }
 
         $arguments = array_merge($this->prefix, $this->arguments);
-        $script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments));
-
-        $process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);
+        $process = new Process($arguments, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);
 
         if ($this->inheritEnv) {
             $process->inheritEnvironmentVariables();

diff --git a/src/Symfony/Component/Process/ProcessUtils.php b/src/Symfony/Component/Process/ProcessUtils.php
@@ -35,9 +35,13 @@ private function __construct()
      * @param string $argument The argument that will be escaped
      *
      * @return string The escaped argument
+     *
+     * @deprecated since version 3.3, to be removed in 4.0. Use a command line array or give env vars to the `Process::start/run()` method instead.
      */
     public static function escapeArgument($argument)
        {
+        @trigger_error('The '.__METHOD__.'() method is deprecated since version 3.3 and will be removed in 4.0. Use a command line array or give env vars to the Process::start/run() method instead.', E_USER_DEPRECATED);
+
         //Fix for PHP bug #43784 escapeshellarg removes % from given string
         //Fix for PHP bug #49446 escapeshellarg doesn't work on Windows
         //@see https://bugs.php.net/bug.php?id=43784

diff --git a/src/Symfony/Component/Process/Tests/PhpProcessTest.php b/src/Symfony/Component/Process/Tests/PhpProcessTest.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Component\Process\Tests;
 
-use Symfony\Component\Process\PhpExecutableFinder;
 use Symfony\Component\Process\PhpProcess;
 
 class PhpProcessTest extends \PHPUnit_Framework_TestCase
@@ -31,19 +30,18 @@ public function testNonBlockingWorks()
     public function testCommandLine()
     {
         $process = new PhpProcess(<<<'PHP'
-<?php echo 'foobar';
+<?php echo phpversion().PHP_SAPI;
 PHP
         );
 
         $commandLine = $process->getCommandLine();
 
-        $f = new PhpExecutableFinder();
-        $this->assertContains($f->find(), $commandLine, '::getCommandLine() returns the command line of PHP before start');
-
         $process->start();
         $this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after start');
 
         $process->wait();
         $this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after wait');
+
+        $this->assertSame(phpversion().PHP_SAPI, $process->getOutput());
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -66,6 +66,8 @@ HttpKernel @@
     Process
     -------
+     * The `ProcessUtils::escapeArgument()` method has been deprecated, use a command line array or give env vars to the `Process::start/run()` method instead.
      * Not inheriting environment variables is deprecated.
      * Configuring `proc_open()` options is deprecated.
@@ Expand Down @@