[Process][Console] deprecated defining commands as strings

This commit is contained in:
Nicolas Grekas 2018-07-03 13:59:14 +02:00
parent 26989d4e73
commit 8895bc1b5b
11 changed files with 197 additions and 46 deletions

View File

@ -11,6 +11,25 @@ Config
* Deprecated constructing a `TreeBuilder` without passing root node information.
Console
-------
* Deprecated passing a command as a string to `ProcessHelper::run()`,
pass the command as an array of arguments instead.
Before:
```php
$processHelper->run($output, 'ls -l');
```
After:
```php
$processHelper->run($output, array('ls', '-l'));
// alternatively, when a shell wrapper is required
$processHelper->run($output, Process::fromShellCommandline('ls -l'));
```
DoctrineBridge
--------------
@ -37,6 +56,25 @@ Form
{% endfor %}
```
Process
-------
* Deprecated the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods.
* Deprecated passing commands as strings when creating a `Process` instance.
Before:
```php
$process = new Process('ls -l');
```
After:
```php
$process = new Process(array('ls', '-l'));
// alternatively, when a shell wrapper is required
$process = Process::fromShellCommandline('ls -l');
```
Security
--------

View File

@ -21,6 +21,21 @@ Console
* Removed the `getHorizontalBorderChar()` method in favor of the `getBorderChars()` method in `TableStyle`.
* Removed the `setVerticalBorderChar()` method in favor of the `setVerticalBorderChars()` method in `TableStyle`.
* Removed the `getVerticalBorderChar()` method in favor of the `getBorderChars()` method in `TableStyle`.
* The `ProcessHelper::run()` method takes the command as an array of arguments.
Before:
```php
$processHelper->run($output, 'ls -l');
```
After:
```php
$processHelper->run($output, array('ls', '-l'));
// alternatively, when a shell wrapper is required
$processHelper->run($output, Process::fromShellCommandline('ls -l'));
```
DependencyInjection
-------------------
@ -78,6 +93,25 @@ HttpFoundation
* The `getClientSize()` method of the `UploadedFile` class has been removed.
* The `getSession()` method of the `Request` class throws an exception when session is null.
Process
-------
* Removed the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods.
* Commands must be defined as arrays when creating a `Process` instance.
Before:
```php
$process = new Process('ls -l');
```
After:
```php
$process = new Process(array('ls', '-l'));
// alternatively, when a shell wrapper is required
$process = Process::fromShellCommandline('ls -l');
```
Security
--------

View File

@ -1,6 +1,15 @@
CHANGELOG
=========
4.2.0
-----
* allowed passing commands as `array($process, 'ENV_VAR' => 'value')` to
`ProcessHelper::run()` to pass environment variables
* deprecated passing a command as a string to `ProcessHelper::run()`,
pass it the command as an array of its arguments instead
* made the `ProcessHelper` class final
4.1.0
-----

View File

@ -20,18 +20,20 @@ use Symfony\Component\Process\Process;
* The ProcessHelper class provides helpers to run external processes.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since Symfony 4.2
*/
class ProcessHelper extends Helper
{
/**
* Runs an external process.
*
* @param OutputInterface $output An OutputInterface instance
* @param string|array|Process $cmd An instance of Process or an array of arguments to escape and run or a command to run
* @param string|null $error An error message that must be displayed if something went wrong
* @param callable|null $callback A PHP callback to run whenever there is some
* output available on STDOUT or STDERR
* @param int $verbosity The threshold for verbosity
* @param OutputInterface $output An OutputInterface instance
* @param array|Process $cmd An instance of Process or an array of the command and arguments
* @param string|null $error An error message that must be displayed if something went wrong
* @param callable|null $callback A PHP callback to run whenever there is some
* output available on STDOUT or STDERR
* @param int $verbosity The threshold for verbosity
*
* @return Process The process that ran
*/
@ -44,9 +46,22 @@ class ProcessHelper extends Helper
$formatter = $this->getHelperSet()->get('debug_formatter');
if ($cmd instanceof Process) {
$process = $cmd;
} else {
$cmd = array($cmd);
}
if (!\is_array($cmd)) {
@trigger_error(sprintf('Passing a command as a string to "%s()" is deprecated since Symfony 4.2, pass it the command as an array of arguments instead.', __METHOD__), E_USER_DEPRECATED);
$cmd = array(\method_exists(Process::class, 'fromShellCommandline') ? Process::fromShellCommandline($cmd) : new Process($cmd));
}
if (\is_string($cmd[0] ?? null)) {
$process = new Process($cmd);
$cmd = array();
} elseif (($cmd[0] ?? null) instanceof Process) {
$process = $cmd[0];
unset($cmd[0]);
} else {
throw new \InvalidArgumentException(sprintf('Invalid command provided to "%s()": the command should an array whose first is element is either the path to the binary to run of a "Process" object.', __METHOD__));
}
if ($verbosity <= $output->getVerbosity()) {
@ -57,7 +72,7 @@ class ProcessHelper extends Helper
$callback = $this->wrapCallback($output, $process, $callback);
}
$process->run($callback);
$process->run($callback, $cmd);
if ($verbosity <= $output->getVerbosity()) {
$message = $process->isSuccessful() ? 'Command ran successfully' : sprintf('%s Command did not run successfully', $process->getExitCode());

View File

@ -25,6 +25,10 @@ class ProcessHelperTest extends TestCase
*/
public function testVariousProcessRuns($expected, $cmd, $verbosity, $error)
{
if (\is_string($cmd)) {
$cmd = \method_exists(Process::class, 'fromShellCommandline') ? Process::fromShellCommandline($cmd) : new Process($cmd);
}
$helper = new ProcessHelper();
$helper->setHelperSet(new HelperSet(array(new DebugFormatterHelper())));
$output = $this->getOutputStream($verbosity);
@ -41,7 +45,7 @@ class ProcessHelperTest extends TestCase
$executed = false;
$callback = function () use (&$executed) { $executed = true; };
$helper->run($output, 'php -r "echo 42;"', null, $callback);
$helper->run($output, array('php', '-r', 'echo 42;'), null, $callback);
$this->assertTrue($executed);
}
@ -81,12 +85,21 @@ EOT;
OUT out message
RES 252 Command did not run successfully
EOT;
$PHP = '\\' === \DIRECTORY_SEPARATOR ? '"!PHP!"' : '"$PHP"';
$successOutputPhp = <<<EOT
RUN php -r $PHP
OUT 42
RES Command ran successfully
EOT;
$errorMessage = 'An error occurred';
$args = new Process(array('php', '-r', 'echo 42;'));
$args = $args->getCommandLine();
$successOutputProcessDebug = str_replace("'php' '-r' 'echo 42;'", $args, $successOutputProcessDebug);
$fromShellCommandline = \method_exists(Process::class, 'fromShellCommandline') ? array(Process::class, 'fromShellCommandline') : function ($cmd) { return new Process($cmd); };
return array(
array('', 'php -r "echo 42;"', StreamOutput::VERBOSITY_VERBOSE, null),
@ -100,7 +113,9 @@ EOT;
array($syntaxErrorOutputVerbose.$errorMessage.PHP_EOL, 'php -r "fwrite(STDERR, \'error message\');usleep(50000);fwrite(STDOUT, \'out message\');exit(252);"', StreamOutput::VERBOSITY_VERY_VERBOSE, $errorMessage),
array($syntaxErrorOutputDebug.$errorMessage.PHP_EOL, 'php -r "fwrite(STDERR, \'error message\');usleep(500000);fwrite(STDOUT, \'out message\');exit(252);"', StreamOutput::VERBOSITY_DEBUG, $errorMessage),
array($successOutputProcessDebug, array('php', '-r', 'echo 42;'), StreamOutput::VERBOSITY_DEBUG, null),
array($successOutputDebug, new Process('php -r "echo 42;"'), StreamOutput::VERBOSITY_DEBUG, null),
array($successOutputDebug, $fromShellCommandline('php -r "echo 42;"'), StreamOutput::VERBOSITY_DEBUG, null),
array($successOutputProcessDebug, array(new Process(array('php', '-r', 'echo 42;'))), StreamOutput::VERBOSITY_DEBUG, null),
array($successOutputPhp, array($fromShellCommandline('php -r '.$PHP), 'PHP' => 'echo 42;'), StreamOutput::VERBOSITY_DEBUG, null),
);
}

View File

@ -330,7 +330,7 @@ final class Dotenv
throw new \LogicException('Resolving commands requires the Symfony Process component.');
}
$process = new Process('echo '.$matches[0]);
$process = \method_exists(Process::class, 'fromShellCommandline') ? Process::fromShellCommandline('echo '.$matches[0]) : new Process('echo '.$matches[0]);
$process->inheritEnvironmentVariables(true);
$process->setEnv($this->values);
try {

View File

@ -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
-----

View File

@ -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 @@ class PhpProcess extends Process
/**
* 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);
}

View File

@ -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 @@ class Process implements \IteratorAggregate
$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 @@ class Process implements \IteratorAggregate
* @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;

View File

@ -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 @@ class ProcessFailedExceptionTest extends TestCase
$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 @@ class ProcessFailedExceptionTest extends TestCase
$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 @@ class ProcessFailedExceptionTest extends TestCase
$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 @@ class ProcessFailedExceptionTest extends TestCase
$this->assertEquals(
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
$exception->getMessage()
str_replace("'php'", 'php', $exception->getMessage())
);
}
}

View File

@ -55,13 +55,13 @@ class ProcessTest extends TestCase
{
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 @@ class ProcessTest extends TestCase
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 @@ EOTXT;
{
$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 @@ EOTXT;
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);
}