Skip to content

Commit 11ab5d7

Browse files
ausinicolas-grekas
authored andcommitted
[Process] Pass the commandline as array to proc_open()
1 parent 5610dee commit 11ab5d7

File tree

5 files changed

+116
-23
lines changed

5 files changed

+116
-23
lines changed
+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Process\Exception;
13+
14+
use Symfony\Component\Process\Process;
15+
16+
/**
17+
* Exception for processes failed during startup.
18+
*/
19+
class ProcessStartFailedException extends ProcessFailedException
20+
{
21+
private Process $process;
22+
23+
public function __construct(Process $process, ?string $message)
24+
{
25+
if ($process->isStarted()) {
26+
throw new InvalidArgumentException('Expected a process that failed during startup, but the given process was started successfully.');
27+
}
28+
29+
$error = sprintf('The command "%s" failed.'."\n\nWorking directory: %s\n\nError: %s",
30+
$process->getCommandLine(),
31+
$process->getWorkingDirectory(),
32+
$message ?? 'unknown'
33+
);
34+
35+
// Skip parent constructor
36+
RuntimeException::__construct($error);
37+
38+
$this->process = $process;
39+
}
40+
41+
public function getProcess(): Process
42+
{
43+
return $this->process;
44+
}
45+
}

Messenger/RunProcessContext.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function __construct(
2727
Process $process,
2828
) {
2929
$this->exitCode = $process->getExitCode();
30-
$this->output = $process->isOutputDisabled() ? null : $process->getOutput();
31-
$this->errorOutput = $process->isOutputDisabled() ? null : $process->getErrorOutput();
30+
$this->output = !$process->isStarted() || $process->isOutputDisabled() ? null : $process->getOutput();
31+
$this->errorOutput = !$process->isStarted() || $process->isOutputDisabled() ? null : $process->getErrorOutput();
3232
}
3333
}

Process.php

+41-20
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Process\Exception\LogicException;
1616
use Symfony\Component\Process\Exception\ProcessFailedException;
1717
use Symfony\Component\Process\Exception\ProcessSignaledException;
18+
use Symfony\Component\Process\Exception\ProcessStartFailedException;
1819
use Symfony\Component\Process\Exception\ProcessTimedOutException;
1920
use Symfony\Component\Process\Exception\RuntimeException;
2021
use Symfony\Component\Process\Pipes\UnixPipes;
@@ -233,11 +234,11 @@ public function __clone()
233234
*
234235
* @return int The exit status code
235236
*
236-
* @throws RuntimeException When process can't be launched
237-
* @throws RuntimeException When process is already running
238-
* @throws ProcessTimedOutException When process timed out
239-
* @throws ProcessSignaledException When process stopped after receiving signal
240-
* @throws LogicException In case a callback is provided and output has been disabled
237+
* @throws ProcessStartFailedException When process can't be launched
238+
* @throws RuntimeException When process is already running
239+
* @throws ProcessTimedOutException When process timed out
240+
* @throws ProcessSignaledException When process stopped after receiving signal
241+
* @throws LogicException In case a callback is provided and output has been disabled
241242
*
242243
* @final
243244
*/
@@ -284,9 +285,9 @@ public function mustRun(callable $callback = null, array $env = []): static
284285
* @param callable|null $callback A PHP callback to run whenever there is some
285286
* output available on STDOUT or STDERR
286287
*
287-
* @throws RuntimeException When process can't be launched
288-
* @throws RuntimeException When process is already running
289-
* @throws LogicException In case a callback is provided and output has been disabled
288+
* @throws ProcessStartFailedException When process can't be launched
289+
* @throws RuntimeException When process is already running
290+
* @throws LogicException In case a callback is provided and output has been disabled
290291
*/
291292
public function start(callable $callback = null, array $env = []): void
292293
{
@@ -306,12 +307,7 @@ public function start(callable $callback = null, array $env = []): void
306307
$env += '\\' === \DIRECTORY_SEPARATOR ? array_diff_ukey($this->getDefaultEnv(), $env, 'strcasecmp') : $this->getDefaultEnv();
307308

308309
if (\is_array($commandline = $this->commandline)) {
309-
$commandline = implode(' ', array_map($this->escapeArgument(...), $commandline));
310-
311-
if ('\\' !== \DIRECTORY_SEPARATOR) {
312-
// exec is mandatory to deal with sending a signal to the process
313-
$commandline = 'exec '.$commandline;
314-
}
310+
$commandline = array_values(array_map(strval(...), $commandline));
315311
} else {
316312
$commandline = $this->replacePlaceholders($commandline, $env);
317313
}
@@ -322,6 +318,11 @@ public function start(callable $callback = null, array $env = []): void
322318
// last exit code is output on the fourth pipe and caught to work around --enable-sigchild
323319
$descriptors[3] = ['pipe', 'w'];
324320

321+
if (\is_array($commandline)) {
322+
// exec is mandatory to deal with sending a signal to the process
323+
$commandline = 'exec '.$this->buildShellCommandline($commandline);
324+
}
325+
325326
// See https://unix.stackexchange.com/questions/71205/background-process-pipe-input
326327
$commandline = '{ ('.$commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
327328
$commandline .= 'pid=$!; echo $pid >&3; wait $pid 2>/dev/null; code=$?; echo $code >&3; exit $code';
@@ -338,10 +339,20 @@ public function start(callable $callback = null, array $env = []): void
338339
throw new RuntimeException(sprintf('The provided cwd "%s" does not exist.', $this->cwd));
339340
}
340341

341-
$process = @proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options);
342+
$lastError = null;
343+
set_error_handler(function ($type, $msg) use (&$lastError) {
344+
$lastError = $msg;
345+
346+
return true;
347+
});
348+
try {
349+
$process = @proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options);
350+
} finally {
351+
restore_error_handler();
352+
}
342353

343354
if (!\is_resource($process)) {
344-
throw new RuntimeException('Unable to launch a new process.');
355+
throw new ProcessStartFailedException($this, $lastError);
345356
}
346357
$this->process = $process;
347358
$this->status = self::STATUS_STARTED;
@@ -366,8 +377,8 @@ public function start(callable $callback = null, array $env = []): void
366377
* @param callable|null $callback A PHP callback to run whenever there is some
367378
* output available on STDOUT or STDERR
368379
*
369-
* @throws RuntimeException When process can't be launched
370-
* @throws RuntimeException When process is already running
380+
* @throws ProcessStartFailedException When process can't be launched
381+
* @throws RuntimeException When process is already running
371382
*
372383
* @see start()
373384
*
@@ -943,7 +954,7 @@ public function getLastOutputTime(): ?float
943954
*/
944955
public function getCommandLine(): string
945956
{
946-
return \is_array($this->commandline) ? implode(' ', array_map($this->escapeArgument(...), $this->commandline)) : $this->commandline;
957+
return $this->buildShellCommandline($this->commandline);
947958
}
948959

949960
/**
@@ -1472,8 +1483,18 @@ private function doSignal(int $signal, bool $throwException): bool
14721483
return true;
14731484
}
14741485

1475-
private function prepareWindowsCommandLine(string $cmd, array &$env): string
1486+
private function buildShellCommandline(string|array $commandline): string
1487+
{
1488+
if (\is_string($commandline)) {
1489+
return $commandline;
1490+
}
1491+
1492+
return implode(' ', array_map($this->escapeArgument(...), $commandline));
1493+
}
1494+
1495+
private function prepareWindowsCommandLine(string|array $cmd, array &$env): string
14761496
{
1497+
$cmd = $this->buildShellCommandline($cmd);
14771498
$uid = uniqid('', true);
14781499
$cmd = preg_replace_callback(
14791500
'/"(?:(

Tests/Messenger/RunProcessMessageHandlerTest.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ public function testRunFailedProcess()
3333
(new RunProcessMessageHandler())(new RunProcessMessage(['invalid']));
3434
} catch (RunProcessFailedException $e) {
3535
$this->assertSame(['invalid'], $e->context->message->command);
36-
$this->assertSame('\\' === \DIRECTORY_SEPARATOR ? 1 : 127, $e->context->exitCode);
36+
$this->assertContains(
37+
$e->context->exitCode,
38+
[null, '\\' === \DIRECTORY_SEPARATOR ? 1 : 127],
39+
'Exit code should be 1 on Windows, 127 on other systems, or null',
40+
);
3741

3842
return;
3943
}

Tests/ProcessTest.php

+23
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\Process\Exception\LogicException;
1717
use Symfony\Component\Process\Exception\ProcessFailedException;
1818
use Symfony\Component\Process\Exception\ProcessSignaledException;
19+
use Symfony\Component\Process\Exception\ProcessStartFailedException;
1920
use Symfony\Component\Process\Exception\ProcessTimedOutException;
2021
use Symfony\Component\Process\Exception\RuntimeException;
2122
use Symfony\Component\Process\InputStream;
@@ -66,6 +67,28 @@ public function testInvalidCwd()
6667
$cmd->run();
6768
}
6869

70+
/**
71+
* @dataProvider invalidProcessProvider
72+
*/
73+
public function testInvalidCommand(Process $process)
74+
{
75+
try {
76+
$this->assertSame('\\' === \DIRECTORY_SEPARATOR ? 1 : 127, $process->run());
77+
} catch (ProcessStartFailedException $e) {
78+
// An invalid command might already fail during start since PHP 8.3 for platforms
79+
// supporting posix_spawn(), see https://github.com/php/php-src/issues/12589
80+
$this->assertStringContainsString('No such file or directory', $e->getMessage());
81+
}
82+
}
83+
84+
public function invalidProcessProvider()
85+
{
86+
return [
87+
[new Process(['invalid'])],
88+
[Process::fromShellCommandline('invalid')],
89+
];
90+
}
91+
6992
/**
7093
* @group transient-on-windows
7194
*/

0 commit comments

Comments
 (0)