8000 [Process] Pass the commandline as array to `proc_open()` by ausi · Pull Request #52409 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Process] Pass the commandline as array to proc_open() #52409

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

Merged
merged 1 commit into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8000
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