Skip to content

Commit

Permalink
[Process] Pass the commandline as array to proc_open()
Browse files Browse the repository at this point in the history
  • Loading branch information
ausi authored and nicolas-grekas committed Nov 21, 2023
1 parent 5689df4 commit a300de8
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,12 @@ private static function startVulcain(HttpClientInterface $client)
'KEY_FILE' => __DIR__.'/Fixtures/tls/server.key',
'CERT_FILE' => __DIR__.'/Fixtures/tls/server.crt',
]);
$process->start();

try {
$process->start();
} catch (ProcessFailedException $e) {
self::markTestSkipped('vulcain failed: '.$e->getMessage());
}

register_shutdown_function($process->stop(...));
sleep('\\' === \DIRECTORY_SEPARATOR ? 10 : 1);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Process\Exception;

use Symfony\Component\Process\Process;

/**
* Exception for processes failed during startup.
*/
class ProcessStartFailedException extends ProcessFailedException
{
private Process $process;

public function __construct(Process $process, ?string $message)
{
if ($process->isStarted()) {
throw new InvalidArgumentException('Expected a process that failed during startup, but the given process was started successfully.');
}

$error = sprintf('The command "%s" failed.'."\n\nWorking directory: %s\n\nError: %s",
$process->getCommandLine(),
$process->getWorkingDirectory(),
$message ?? 'unknown'
);

// Skip parent constructor
RuntimeException::__construct($error);

$this->process = $process;
}

public function getProcess(): Process
{
return $this->process;
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Component/Process/Messenger/RunProcessContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function __construct(
Process $process,
) {
$this->exitCode = $process->getExitCode();
$this->output = $process->isOutputDisabled() ? null : $process->getOutput();
$this->errorOutput = $process->isOutputDisabled() ? null : $process->getErrorOutput();
$this->output = !$process->isStarted() || $process->isOutputDisabled() ? null : $process->getOutput();
$this->errorOutput = !$process->isStarted() || $process->isOutputDisabled() ? null : $process->getErrorOutput();
}
}
61 changes: 41 additions & 20 deletions src/Symfony/Component/Process/Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Process\Exception\LogicException;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Exception\ProcessSignaledException;
use Symfony\Component\Process\Exception\ProcessStartFailedException;
use Symfony\Component\Process\Exception\ProcessTimedOutException;
use Symfony\Component\Process\Exception\RuntimeException;
use Symfony\Component\Process\Pipes\UnixPipes;
Expand Down Expand Up @@ -233,11 +234,11 @@ public function __clone()
*
* @return int The exit status code
*
* @throws RuntimeException When process can't be launched
* @throws RuntimeException When process is already running
* @throws ProcessTimedOutException When process timed out
* @throws ProcessSignaledException When process stopped after receiving signal
* @throws LogicException In case a callback is provided and output has been disabled
* @throws ProcessStartFailedException When process can't be launched
* @throws RuntimeException When process is already running
* @throws ProcessTimedOutException When process timed out
* @throws ProcessSignaledException When process stopped after receiving signal
* @throws LogicException In case a callback is provided and output has been disabled
*
* @final
*/
Expand Down Expand Up @@ -284,9 +285,9 @@ public function mustRun(callable $callback = null, array $env = []): static
* @param callable|null $callback A PHP callback to run whenever there is some
* output available on STDOUT or STDERR
*
* @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
* @throws ProcessStartFailedException 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, array $env = []): void
{
Expand All @@ -306,12 +307,7 @@ public function start(callable $callback = null, array $env = []): void
$env += '\\' === \DIRECTORY_SEPARATOR ? array_diff_ukey($this->getDefaultEnv(), $env, 'strcasecmp') : $this->getDefaultEnv();

if (\is_array($commandline = $this->commandline)) {
$commandline = implode(' ', array_map($this->escapeArgument(...), $commandline));

if ('\\' !== \DIRECTORY_SEPARATOR) {
// exec is mandatory to deal with sending a signal to the process
$commandline = 'exec '.$commandline;
}
$commandline = array_values(array_map(strval(...), $commandline));
} else {
$commandline = $this->replacePlaceholders($commandline, $env);
}
Expand All @@ -322,6 +318,11 @@ public function start(callable $callback = null, array $env = []): void
// last exit code is output on the fourth pipe and caught to work around --enable-sigchild
$descriptors[3] = ['pipe', 'w'];

if (\is_array($commandline)) {
// exec is mandatory to deal with sending a signal to the process
$commandline = 'exec '.$this->buildShellCommandline($commandline);
}

// See https://unix.stackexchange.com/questions/71205/background-process-pipe-input
$commandline = '{ ('.$commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
$commandline .= 'pid=$!; echo $pid >&3; wait $pid 2>/dev/null; code=$?; echo $code >&3; exit $code';
Expand All @@ -338,10 +339,20 @@ public function start(callable $callback = null, array $env = []): void
throw new RuntimeException(sprintf('The provided cwd "%s" does not exist.', $this->cwd));
}

$process = @proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options);
$lastError = null;
set_error_handler(function ($type, $msg) use (&$lastError) {
$lastError = $msg;

return true;
});
try {
$process = @proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options);
} finally {
restore_error_handler();
}

if (!\is_resource($process)) {
throw new RuntimeException('Unable to launch a new process.');
throw new ProcessStartFailedException($this, $lastError);
}
$this->process = $process;
$this->status = self::STATUS_STARTED;
Expand All @@ -366,8 +377,8 @@ public function start(callable $callback = null, array $env = []): void
* @param callable|null $callback A PHP callback to run whenever there is some
* output available on STDOUT or STDERR
*
* @throws RuntimeException When process can't be launched
* @throws RuntimeException When process is already running
* @throws ProcessStartFailedException When process can't be launched
* @throws RuntimeException When process is already running
*
* @see start()
*
Expand Down Expand Up @@ -943,7 +954,7 @@ public function getLastOutputTime(): ?float
*/
public function getCommandLine(): string
{
return \is_array($this->commandline) ? implode(' ', array_map($this->escapeArgument(...), $this->commandline)) : $this->commandline;
return $this->buildShellCommandline($this->commandline);
}

/**
Expand Down Expand Up @@ -1472,8 +1483,18 @@ private function doSignal(int $signal, bool $throwException): bool
return true;
}

private function prepareWindowsCommandLine(string $cmd, array &$env): string
private function buildShellCommandline(string|array $commandline): string
{
if (\is_string($commandline)) {
return $commandline;
}

return implode(' ', array_map($this->escapeArgument(...), $commandline));
}

private function prepareWindowsCommandLine(string|array $cmd, array &$env): string
{
$cmd = $this->buildShellCommandline($cmd);
$uid = uniqid('', true);
$cmd = preg_replace_callback(
'/"(?:(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public function testRunFailedProcess()
(new RunProcessMessageHandler())(new RunProcessMessage(['invalid']));
} catch (RunProcessFailedException $e) {
$this->assertSame(['invalid'], $e->context->message->command);
$this->assertSame('\\' === \DIRECTORY_SEPARATOR ? 1 : 127, $e->context->exitCode);
$this->assertContains(
$e->context->exitCode,
[null, '\\' === \DIRECTORY_SEPARATOR ? 1 : 127],
'Exit code should be 1 on Windows, 127 on other systems, or null',
);

return;
}
Expand Down
23 changes: 23 additions & 0 deletions src/Symfony/Component/Process/Tests/ProcessTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\Process\Exception\LogicException;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Exception\ProcessSignaledException;
use Symfony\Component\Process\Exception\ProcessStartFailedException;
use Symfony\Component\Process\Exception\ProcessTimedOutException;
use Symfony\Component\Process\Exception\RuntimeException;
use Symfony\Component\Process\InputStream;
Expand Down Expand Up @@ -66,6 +67,28 @@ public function testInvalidCwd()
$cmd->run();
}

/**
* @dataProvider invalidProcessProvider
*/
public function testInvalidCommand(Process $process)
{
try {
$this->assertSame('\\' === \DIRECTORY_SEPARATOR ? 1 : 127, $process->run());
} catch (ProcessStartFailedException $e) {
// An invalid command might already fail during start since PHP 8.3 for platforms
// supporting posix_spawn(), see https://github.com/php/php-src/issues/12589
$this->assertStringContainsString('No such file or directory', $e->getMessage());
}
}

public function invalidProcessProvider()
{
return [
[new Process(['invalid'])],
[Process::fromShellCommandline('invalid')],
];
}

/**
* @group transient-on-windows
*/
Expand Down

0 comments on commit a300de8

Please sign in to comment.