8000 feature #21474 [Process] Accept command line arrays and per-run env v… · symfony/symfony@3193331 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3193331

Browse files
fabpotfabpot
committed
feature #21474 [Process] Accept command line arrays and per-run env vars, 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
2 parents 2697cee + 330b61f commit 3193331

11 files changed

+260
-114
lines changed

‎UPGRADE-3.3.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ HttpKernel
6666
Process
6767
-------
6868

69+
* The `ProcessUtils::escapeArgument()` method has been deprecated, use a command line array or give env vars to the `Process::start/run()` method instead.
70+
6971
* Not inheriting environment variables is deprecated.
7072

7173
* Configuring `proc_open()` options is deprecated.

‎UPGRADE-4.0.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ HttpKernel
228228
Process
229229
-------
230230

231+
* The `ProcessUtils::escapeArgument()` method has been removed, use a command line array or give env vars to the `Process::start/run()` method instead.
232+
231233
* Environment variables are always inherited in sub-processes.
232234

233235
* Configuring `proc_open()` options has been removed.

‎src/Symfony/Component/Process/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ CHANGELOG
44
3.3.0
55
-----
66

7+
* added command line arrays in the `Process` class
8+
* added `$env` argument to `Process::start()`, `run()`, `mustRun()` and `restart()` methods
9+
* deprecated the `ProcessUtils::escapeArgument()` method
710
* deprecated not inheriting environment variables
811
* deprecated configuring `proc_open()` options
912
* deprecated configuring enhanced Windows compatibility

‎src/Symfony/Component/Process/PhpProcess.php

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,16 @@ public function __construct($script, $cwd = null, array $env = null, $timeout =
3838
$executableFinder = new PhpExecutableFinder();
3939
if (false === $php = $executableFinder->find()) {
4040
$php = null;
41+
} else {
42+
$php = explode(' ', $php);
4143
}
4244
if ('phpdbg' === PHP_SAPI) {
4345
$file = tempnam(sys_get_temp_dir(), 'dbg');
4446
file_put_contents($file, $script);
4547
register_shutdown_function('unlink', $file);
46-
$php .= ' '.ProcessUtils::escapeArgument($file);
48+
$php[] = $file;
4749
$script = null;
4850
}
49-
if ('\\' !== DIRECTORY_SEPARATOR && null !== $php) {
50-
// exec is mandatory to deal with sending a signal to the process
51-
// see https://github.com/symfony/symfony/issues/5030 about prepending
52-
// command with exec
53-
$php = 'exec '.$php;
54-
}
5551
if (null !== $options) {
5652
@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);
5753
}
@@ -70,12 +66,13 @@ public function setPhpBinary($php)
7066
/**
7167
* {@inheritdoc}
7268
*/
73-
public function start(callable $callback = null)
69+
public function start(callable $callback = null/*, array $env = array()*/)
7470
{
7571
if (null === $this->getCommandLine()) {
7672
throw new RuntimeException('Unable to find the PHP executable.');
7773
}
74+
$env = 1 < func_num_args() ? func_get_arg(1) : null;
7875

79-
parent::start($callback);
76+
parent::start($callback, $env);
8077
}
8178
}

‎src/Symfony/Component/Process/Process.php

Lines changed: 125 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ class Process implements \IteratorAggregate
136136
/**
137137
* Constructor.
138138
*
139-
* @param string $commandline The command line to run
139+
* @param string|array $commandline The command line to run
140140
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process
141141
* @param array|null $env The environment variables or null to use the same environment as the current PHP process
142142
* @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
151151
throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
152152
}
153153

154-
$this->commandline = $commandline;
154+
$this->setCommandline($commandline);
155155
$this->cwd = $cwd;
156156

157157
// 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()
199199
*
200200
* @param callable|null $callback A PHP callback to run whenever there is some
201201
* output available on STDOUT or STDERR
202+
* @param array $env An array of additional env vars to set when running the process
202203
*
203204
* @return int The exit status code
204205
*
205206
* @throws RuntimeException When process can't be launched
206207
* @throws RuntimeException When process stopped after receiving signal
207208
* @throws LogicException In case a callback is provided and output has been disabled
209+
*
210+
* @final since version 3.3
208211
*/
209-
public function run($callback = null)
212+
public function run($callback = null/*, array $env = array()*/)
210213
{
211-
$this->start($callback);
214+
$env = 1 < func_num_args() ? func_get_arg(1) : null;
215+
$this->start($callback, $env);
212216

213217
return $this->wait();
214218
}
@@ -220,19 +224,23 @@ public function run($callback = null)
220224
* exits with a non-zero exit code.
221225
*
222226
* @param callable|null $callback
227+
* @param array $env An array of additional env vars to set when running the process
223228
*
224229
* @return self
225230
*
226231
* @throws RuntimeException if PHP was compiled with --enable-sigchild and the enhanced sigchild compatibility mode is not enabled
227232
* @throws ProcessFailedException if the process didn't terminate successfully
233+
*
234+
* @final since version 3.3
228235
*/
229-
public function mustRun(callable $callback = null)
236+
public function mustRun(callable $callback = null/*, array $env = array()*/)
230237
{
231238
if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
232239
throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.');
233240
}
241+
$env = 1 < func_num_args() ? func_get_arg(1) : null;
234242

235-
if (0 !== $this->run($callback)) {
243+
if (0 !== $this->run($callback, $env)) {
236244
throw new ProcessFailedException($this);
237245
}
238246

@@ -253,28 +261,48 @@ public function mustRun(callable $callback = null)
253261
*
254262
* @param callable|null $callback A PHP callback to run whenever there is some
255263
* output available on STDOUT or STDERR
264+
* @param array $env An array of additional env vars to set when running the process
256265
*
257266
* @throws RuntimeException When process can't be launched
258267
* @throws RuntimeException When process is already running
259268
* @throws LogicException In case a callback is provided and output has been disabled
260269
*/
261-
public function start(callable $callback = null)
270+
public function start(callable $callback = null/*, array $env = array()*/)
262271
{
263272
if ($this->isRunning()) {
264273
throw new RuntimeException('Process is already running');
265274
}
275+
if (2 <= func_num_args()) {
276+
$env = func_get_arg(1);
277+
} else {
278+
if (__CLASS__ !== static::class) {
279+
$r = new \ReflectionMethod($this, __FUNCTION__);
280+
if (__CLASS__ !== $r->getDeclaringClass()->getName() && (2 > $r->getNumberOfParameters() || 'env' !== $r->getParameters()[0]->name)) {
281+
@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);
282+
}
283+
}
284+
$env = null;
285+
}
266286

267287
$this->resetProcessData();
268288
$this->starttime = $this->lastOutputTime = microtime(true);
269289
$this->callback = $this->buildCallback($callback);
270290
$this->hasCallback = null !== $callback;
271291
$descriptors = $this->getDescriptors();
272-
292+
$inheritEnv = $this->inheritEnv;
273293
$commandline = $this->commandline;
274294

275-
$env = $this->env;
295+
if (null === $env) {
296+
$env = $this->env;
297+
} else {
298+
if ($this->env) {
299+
$env += $this->env;
300+
}
301+
$inheritEnv = true;
302+
}
303+
276304
$envBackup = array();
277-
if (null !== $env && $this->inheritEnv) {
305+
if (null !== $env && $inheritEnv) {
278306
foreach ($env as $k => $v) {
279307
$envBackup[$k] = getenv($v);
280308
putenv(false === $v || null === $v ? $k : "$k=$v");
@@ -284,14 +312,8 @@ public function start(callable $callback = null)
284312
@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);
285313
}
286314
if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) {
287-
$commandline = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $commandline).')';
288-
foreach ($this->processPipes->getFiles() as $offset => $filename) {
289-
$commandline .= ' '.$offset.'>"'.$filename.'"';
290-
}
291-
292-
if (!isset($this->options['bypass_shell'])) {
293-
$this->options['bypass_shell'] = true;
294-
}
315+
$this->options['bypass_shell'] = true;
316+
$commandline = $this->prepareWindowsCommandLine($commandline, $envBackup);
295317
} elseif (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
296318
// last exit code is output on the fourth pipe and caught to work around --enable-sigchild
297319
$descriptors[3] = array('pipe', 'w');
@@ -335,22 +357,26 @@ public function start(callable $callback = null)
335357
*
336358
* @param callable|null $callback A PHP callback to run whenever there is some
337359
* output available on STDOUT or STDERR
360+
* @param array $env An array of additional env vars to set when running the process
338361
*
339362
* @return $this
340363
*
341364
* @throws RuntimeException When process can't be launched
342365
* @throws RuntimeException When process is already running
343366
*
344367
* @see start()
368+
*
369+
* @final since version 3.3
345370
*/
346-
public function restart(callable $callback = null)
371+
public function restart(callable $callback = null/*, array $env = array()*/)
347372
{
348373
if ($this->isRunning()) {
349374
throw new RuntimeException('Process is already running');
350375
}
376+
$env = 1 < func_num_args() ? func_get_arg(1) : null;
351377

352378
$process = clone $this;
353-
$process->start($callback);
379+
$process->start($callback, $env);
354380

355381
return $process;
356382
}
@@ -909,12 +935,20 @@ public function getCommandLine()
909935
/**
910936
* Sets the command line to be executed.
911937
*
912-
* @param string $commandline The command to execute
938+
* @param string|array $commandline The command to execute
913939
*
914940
* @return self The current Process instance
915941
*/
916942
public function setCommandLine($commandline)
917943
{
944+
if (is_array($commandline)) {
945+
$commandline = implode(' ', array_map(array($this, 'escapeArgument'), $commandline));
946+
947+
if ('\\' !== DIRECTORY_SEPARATOR) {
948+
// exec is mandatory to deal with sending a signal to the process
949+
$commandline = 'exec '.$commandline;
950+
}
951+
}
918952
$this->commandline = $commandline;
919953

920954
return $this;
@@ -1589,6 +1623,50 @@ private function doSignal($signal, $throwException)
15891623
return true;
15901624
}
15911625

1626+
private function prepareWindowsCommandLine($cmd, array &$envBackup)
1627+
{
1628+
$uid = uniqid('', true);
1629+
$varCount = 0;
1630+
$varCache = array();
1631+
$cmd = preg_replace_callback(
1632+
'/"(
1633+
[^"%!^]*+
1634+
(?:
1635+
(?: !LF! | "(?:\^[%!^])?+" )
1636+
[^"%!^]*+
1637+
)++
1638+
)"/x',
1639+
function ($m) use (&$envBackup, &$varCache, &$varCount, $uid) {
1640+
if (isset($varCache[$m[0]])) {
1641+
return $varCache[$m[0]];
1642+
}
1643+
if (false !== strpos($value = $m[1], "\0")) {
1644+
$value = str_replace("\0", '?', $value);
1645+
}
1646+
if (false === strpbrk($value, "\"%!\n")) {
1647+
return '"'.$value.'"';
1648+
}
1649+
1650+
$value = str_replace(array('!LF!', '"^!"', '"^%"', '"^^"', '""'), array("\n", '!', '%', '^', '"'), $value);
1651+
$value = preg_replace('/(\\\\*)"/', '$1$1\\"', $value);
1652+
1653+
$var = $uid.++$varCount;
1654+
putenv("$var=\"$value\"");
1655+
$envBackup[$var] = false;
1656+
1657+
return $varCache[$m[0]] = '!'.$var.'!';
1658+
},
1659+
$cmd
1660+
);
1661+
1662+
$cmd = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
1663+
foreach ($this->processPipes->getFiles() as $offset => $filename) {
1664+
$cmd .= ' '.$offset.'>"'.$filename.'"';
1665+
}
1666+
1667+
return $cmd;
1668+
}
1669+
15921670
/**
15931671
* Ensures the process is running or terminated, throws a LogicException if the process has a not started.
15941672
*
@@ -1616,4 +1694,30 @@ private function requireProcessIsTerminated($functionName)
16161694
throw new LogicException(sprintf('Process must be terminated before calling %s.', $functionName));
16171695
}
16181696
}
1697+
1698+
/**
1699+
* Escapes a string to be used as a shell argument.
1700+
*
1701+
* @param string $argument The argument that will be escaped
1702+
*
1703+
* @return string The escaped argument
1704+
*/
1705+
private function escapeArgument($argument)
1706+
{
1707+
if ('\\' !== DIRECTORY_SEPARATOR) {
1708+
return "'".str_replace("'", "'\\''", $argument)."'";
1709+
}
1710+
if ('' === $argument = (string) $argument) {
1711+
return '""';
1712+
}
1713+
if (false !== strpos($argument, "\0")) {
1714+
$argument = str_replace("\0", '?', $argument);
1715+
}
1716+
if (!preg_match('/[()%!^"<>&|\s]/', $argument)) {
1717+
return $argument;
1718+
}
1719+
$argument = preg_replace('/(\\\\+)$/', '$1$1', $argument);
1720+
1721+
return '"'.str_replace(array('"', '^', '%', '!', "\n"), array('""', '"^^"', '"^%"', '"^!"', '!LF!'), $argument).'"';
1722+
}
16191723
}

‎src/Symfony/Component/Process/ProcessBuilder.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,7 @@ public function getProcess()
271271
}
272272

273273
$arguments = array_merge($this->prefix, $this->arguments);
274-
$script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments));
275-
276-
$process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);
274+
$process = new Process($arguments, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);
277275

278276
if ($this->inheritEnv) {
279277
$process->inheritEnvironmentVariables();

‎src/Symfony/Component/Process/ProcessUtils.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,13 @@ private function __construct()
3535
* @param string $argument The argument that will be escaped
3636
*
3737
* @return string The escaped argument
38+
*
39+
* @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.
3840
*/
3941
public static function escapeArgument($argument)
4042
{
43+
@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);
44+
4145
//Fix for PHP bug #43784 escapeshellarg removes % from given string
4246
//Fix for PHP bug #49446 escapeshellarg doesn't work on Windows
4347
//@see https://bugs.php.net/bug.php?id=43784

‎src/Symfony/Component/Process/Tests/PhpProcessTest.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Symfony\Component\Process\Tests;
1313

14-
use Symfony\Component\Process\PhpExecutableFinder;
1514
use Symfony\Component\Process\PhpProcess;
1615

1716
class PhpProcessTest extends \PHPUnit_Framework_TestCase
@@ -31,19 +30,18 @@ public function testNonBlockingWorks()
3130
public function testCommandLine()
3231
{
3332
$process = new PhpProcess(<<<'PHP'
34-
<?php echo 'foobar';
33+
<?php echo phpversion().PHP_SAPI;
3534
PHP
3635
);
3736

3837
$commandLine = $process->getCommandLine();
3938

40-
$f = new PhpExecutableFinder();
41-
$this->assertContains($f->find(), $commandLine, '::getCommandLine() returns the command line of PHP before start');
42-
4339
$process->start();
4440
$this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after start');
4541

4642
$process->wait();
4743
$this->assertContains($commandLine, $process->getCommandLine(), '::getCommandLine() returns the command line of PHP after wait');
44+
45+
$this->assertSame(phpversion().PHP_SAPI, $process->getOutput());
4846
}
4947
}

0 commit comments

Comments
 (0)
0