Permalink
Browse files

feature #21474 [Process] Accept command line arrays and per-run env v…

…ars, fixing signaling and escaping (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Process] Accept command line arrays and per-run env vars, fixing signaling and escaping

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #12488, #11972, #10025, #11335, #5759, #5030, #19993, #10486
| License       | MIT
| Doc PR        | -

I think I found a way to fix this network of issues once for all.
Of all the linked ones, only the last two are still open: the remaining were closed in dead ends.

Instead of trying to make `ProcessUtil::escapeArgument` work correctly on Windows - which is impossible as discussed in #21347 - this PR deprecates it in favor of a more powerful approach.

Depending on the use case:

- when a simple command should be run, `Process` now accepts an array of arguments (the "binary" being the first arg). Making this the responsibility of `Process` (instead of `ProcessBuilder`) gives two benefits:
  - escape becomes an internal detail that doesn't leak - thus can't be misused ([see here](#21347 (comment)))
  - since we know we're running a single command, we can prefix it automatically by "exec" - thus fixing a long standing issue with signaling

```php
        $p = new Process(array('php', '-r', 'echo 123;'));
        echo $p->getCommandLine();
        // displays on Linux:
        // exec 'php' '-r' 'echo 123;'
```

- when a shell expression is required, passing a string is still allowed. To make it easy and look-like sql prepared statements, env vars can be used when running the command. Since the shell is OS-specific (think Windows vs Linux) - this PR assumes no portability, so one should just use each shell's specific syntax.

From the fixtures:
```php
        $env = array('FOO' => 'Foo', 'BAR' => 'Bar');
        $cmd = '\\' === DIRECTORY_SEPARATOR ? 'echo !FOO! !BAR! !BAZ!' : 'echo $FOO $BAR $BAZ';
        $p = new Process($cmd, null, $env);
        $p->run(null, array('BAR' => 'baR', 'BAZ' => 'baZ'));

        $this->assertSame('Foo baR baZ', rtrim($p->getOutput()));
        $this->assertSame($env, $p->getEnv());
```

Commits
-------

330b61f [Process] Accept command line arrays and per-run env vars, fixing signaling and escaping
  • Loading branch information...
2 parents 2697cee + 330b61f commit 3193331855df2cca085f4352ec58d66b4b54890e @fabpot fabpot committed Feb 8, 2017
View
@@ -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.
View
@@ -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.
@@ -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
@@ -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);
}
}
@@ -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).'"';
+ }
}
@@ -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();
@@ -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
@@ -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());
}
}
Oops, something went wrong.

0 comments on commit 3193331

Please sign in to comment.