-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process][Console] deprecated defining commands as strings #27821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
|
||
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 | ||
-------- | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,18 +20,20 @@ | |
* 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 @@ public function run(OutputInterface $output, $cmd, $error = null, callable $call | |
$formatter = $this->getHelperSet()->get('debug_formatter'); | ||
|
||
if ($cmd instanceof Process) { | ||
$process = $cmd; | ||
} else { | ||
$cmd = array($cmd); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why wrapping the Process object in an array, to unwrap it after that ? this looks really weird. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to normalize the data structure and simplify the following lines |
||
} | ||
|
||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is no adding support for passing an array with a Process as first argument (and other stuff after that). does it actually make sense ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See PR description:
|
||
$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 @@ public function run(OutputInterface $output, $cmd, $error = null, callable $call | |
$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()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
* @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; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here