diff --git a/CHANGELOG.md b/CHANGELOG.md index dc0a0cc5..3e33cd0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +7.1 +--- + + * Add `Process::setIgnoredSignals()` to disable signal propagation to the child process + 6.4 --- diff --git a/Exception/ProcessFailedException.php b/Exception/ProcessFailedException.php index 19b40570..de8a9e98 100644 --- a/Exception/ProcessFailedException.php +++ b/Exception/ProcessFailedException.php @@ -20,15 +20,14 @@ */ class ProcessFailedException extends RuntimeException { - private Process $process; - - public function __construct(Process $process) - { + public function __construct( + private Process $process, + ) { if ($process->isSuccessful()) { throw new InvalidArgumentException('Expected a failed process, but the given process was successful.'); } - $error = sprintf('The command "%s" failed.'."\n\nExit Code: %s(%s)\n\nWorking directory: %s", + $error = \sprintf('The command "%s" failed.'."\n\nExit Code: %s(%s)\n\nWorking directory: %s", $process->getCommandLine(), $process->getExitCode(), $process->getExitCodeText(), @@ -36,7 +35,7 @@ public function __construct(Process $process) ); if (!$process->isOutputDisabled()) { - $error .= sprintf("\n\nOutput:\n================\n%s\n\nError Output:\n================\n%s", + $error .= \sprintf("\n\nOutput:\n================\n%s\n\nError Output:\n================\n%s", $process->getOutput(), $process->getErrorOutput() ); @@ -47,10 +46,7 @@ public function __construct(Process $process) $this->process = $process; } - /** - * @return Process - */ - public function getProcess() + public function getProcess(): Process { return $this->process; } diff --git a/Exception/ProcessSignaledException.php b/Exception/ProcessSignaledException.php index 0fed8ac3..3fd13e5d 100644 --- a/Exception/ProcessSignaledException.php +++ b/Exception/ProcessSignaledException.php @@ -20,13 +20,10 @@ */ final class ProcessSignaledException extends RuntimeException { - private Process $process; - - public function __construct(Process $process) - { - $this->process = $process; - - parent::__construct(sprintf('The process has been signaled with signal "%s".', $process->getTermSignal())); + public function __construct( + private Process $process, + ) { + parent::__construct(\sprintf('The process has been signaled with signal "%s".', $process->getTermSignal())); } public function getProcess(): Process diff --git a/Exception/ProcessStartFailedException.php b/Exception/ProcessStartFailedException.php new file mode 100644 index 00000000..37254725 --- /dev/null +++ b/Exception/ProcessStartFailedException.php @@ -0,0 +1,43 @@ + + * + * 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 +{ + public function __construct( + private 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); + } + + public function getProcess(): Process + { + return $this->process; + } +} diff --git a/Exception/ProcessTimedOutException.php b/Exception/ProcessTimedOutException.php index 1cecdae7..d3fe4934 100644 --- a/Exception/ProcessTimedOutException.php +++ b/Exception/ProcessTimedOutException.php @@ -23,41 +23,28 @@ class ProcessTimedOutException extends RuntimeException public const TYPE_GENERAL = 1; public const TYPE_IDLE = 2; - private Process $process; - private int $timeoutType; - - public function __construct(Process $process, int $timeoutType) - { - $this->process = $process; - $this->timeoutType = $timeoutType; - - parent::__construct(sprintf( + public function __construct( + private Process $process, + private int $timeoutType, + ) { + parent::__construct(\sprintf( 'The process "%s" exceeded the timeout of %s seconds.', $process->getCommandLine(), $this->getExceededTimeout() )); } - /** - * @return Process - */ - public function getProcess() + public function getProcess(): Process { return $this->process; } - /** - * @return bool - */ - public function isGeneralTimeout() + public function isGeneralTimeout(): bool { return self::TYPE_GENERAL === $this->timeoutType; } - /** - * @return bool - */ - public function isIdleTimeout() + public function isIdleTimeout(): bool { return self::TYPE_IDLE === $this->timeoutType; } @@ -67,7 +54,7 @@ public function getExceededTimeout(): ?float return match ($this->timeoutType) { self::TYPE_GENERAL => $this->process->getTimeout(), self::TYPE_IDLE => $this->process->getIdleTimeout(), - default => throw new \LogicException(sprintf('Unknown timeout type "%d".', $this->timeoutType)), + default => throw new \LogicException(\sprintf('Unknown timeout type "%d".', $this->timeoutType)), }; } } diff --git a/ExecutableFinder.php b/ExecutableFinder.php index 1838d54b..5cc65251 100644 --- a/ExecutableFinder.php +++ b/ExecutableFinder.php @@ -31,20 +31,19 @@ class ExecutableFinder /** * Replaces default suffixes of executable. - * - * @return void */ - public function setSuffixes(array $suffixes) + public function setSuffixes(array $suffixes): void { $this->suffixes = $suffixes; } /** - * Adds new possible suffix to check for executable. + * Adds new possible suffix to check for executable, including the dot (.). * - * @return void + * $finder = new ExecutableFinder(); + * $finder->addSuffix('.foo'); */ - public function addSuffix(string $suffix) + public function addSuffix(string $suffix): void { $this->suffixes[] = $suffix; } @@ -68,10 +67,9 @@ public function find(string $name, ?string $default = null, array $extraDirs = [ $extraDirs ); - $suffixes = []; + $suffixes = $this->suffixes; if ('\\' === \DIRECTORY_SEPARATOR) { $pathExt = getenv('PATHEXT'); - $suffixes = $this->suffixes; $suffixes = array_merge($suffixes, $pathExt ? explode(\PATH_SEPARATOR, $pathExt) : ['.exe', '.bat', '.cmd', '.com']); } $suffixes = '' !== pathinfo($name, PATHINFO_EXTENSION) ? array_merge([''], $suffixes) : array_merge($suffixes, ['']); diff --git a/InputStream.php b/InputStream.php index 931217c8..586e7429 100644 --- a/InputStream.php +++ b/InputStream.php @@ -28,10 +28,8 @@ class InputStream implements \IteratorAggregate /** * Sets a callback that is called when the write buffer becomes empty. - * - * @return void */ - public function onEmpty(?callable $onEmpty = null) + public function onEmpty(?callable $onEmpty = null): void { $this->onEmpty = null !== $onEmpty ? $onEmpty(...) : null; } @@ -41,36 +39,30 @@ public function onEmpty(?callable $onEmpty = null) * * @param resource|string|int|float|bool|\Traversable|null $input The input to append as scalar, * stream resource or \Traversable - * - * @return void */ - public function write(mixed $input) + public function write(mixed $input): void { if (null === $input) { return; } if ($this->isClosed()) { - throw new RuntimeException(sprintf('"%s" is closed.', static::class)); + throw new RuntimeException(\sprintf('"%s" is closed.', static::class)); } $this->input[] = ProcessUtils::validateInput(__METHOD__, $input); } /** * Closes the write buffer. - * - * @return void */ - public function close() + public function close(): void { $this->open = false; } /** * Tells whether the write buffer is closed or not. - * - * @return bool */ - public function isClosed() + public function isClosed(): bool { return !$this->open; } diff --git a/Messenger/RunProcessContext.php b/Messenger/RunProcessContext.php index b5ade072..5e223040 100644 --- a/Messenger/RunProcessContext.php +++ b/Messenger/RunProcessContext.php @@ -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(); } } diff --git a/PhpExecutableFinder.php b/PhpExecutableFinder.php index e24ca008..9f9218f9 100644 --- a/PhpExecutableFinder.php +++ b/PhpExecutableFinder.php @@ -74,6 +74,10 @@ public function find(bool $includeArgs = true): string|false $dirs[] = 'C:\xampp\php\\'; } + if ($herdPath = getenv('HERD_HOME')) { + $dirs[] = $herdPath.\DIRECTORY_SEPARATOR.'bin'; + } + return $this->executableFinder->find('php', false, $dirs); } diff --git a/PhpProcess.php b/PhpProcess.php index 6e2ab59f..0e7ff846 100644 --- a/PhpProcess.php +++ b/PhpProcess.php @@ -52,13 +52,10 @@ public function __construct(string $script, ?string $cwd = null, ?array $env = n public static function fromShellCommandline(string $command, ?string $cwd = null, ?array $env = null, mixed $input = null, ?float $timeout = 60): static { - throw new LogicException(sprintf('The "%s()" method cannot be called when using "%s".', __METHOD__, self::class)); + throw new LogicException(\sprintf('The "%s()" method cannot be called when using "%s".', __METHOD__, self::class)); } - /** - * @return void - */ - public function start(?callable $callback = null, array $env = []) + public function start(?callable $callback = null, array $env = []): void { if (null === $this->getCommandLine()) { throw new RuntimeException('Unable to find the PHP executable.'); diff --git a/PhpSubprocess.php b/PhpSubprocess.php index 04fd8ea8..bdd4173c 100644 --- a/PhpSubprocess.php +++ b/PhpSubprocess.php @@ -75,7 +75,7 @@ public function __construct(array $command, ?string $cwd = null, ?array $env = n public static function fromShellCommandline(string $command, ?string $cwd = null, ?array $env = null, mixed $input = null, ?float $timeout = 60): static { - throw new LogicException(sprintf('The "%s()" method cannot be called when using "%s".', __METHOD__, self::class)); + throw new LogicException(\sprintf('The "%s()" method cannot be called when using "%s".', __METHOD__, self::class)); } public function start(?callable $callback = null, array $env = []): void diff --git a/Pipes/AbstractPipes.php b/Pipes/AbstractPipes.php index cbbb7277..51a566f3 100644 --- a/Pipes/AbstractPipes.php +++ b/Pipes/AbstractPipes.php @@ -101,7 +101,7 @@ protected function write(): ?array } elseif (!isset($this->inputBuffer[0])) { if (!\is_string($input)) { if (!\is_scalar($input)) { - throw new InvalidArgumentException(sprintf('"%s" yielded a value of type "%s", but only scalars and stream resources are supported.', get_debug_type($this->input), get_debug_type($input))); + throw new InvalidArgumentException(\sprintf('"%s" yielded a value of type "%s", but only scalars and stream resources are supported.', get_debug_type($this->input), get_debug_type($input))); } $input = (string) $input; } diff --git a/Pipes/UnixPipes.php b/Pipes/UnixPipes.php index a0e48dd3..6f95a332 100644 --- a/Pipes/UnixPipes.php +++ b/Pipes/UnixPipes.php @@ -22,16 +22,12 @@ */ class UnixPipes extends AbstractPipes { - private ?bool $ttyMode; - private bool $ptyMode; - private bool $haveReadSupport; - - public function __construct(?bool $ttyMode, bool $ptyMode, mixed $input, bool $haveReadSupport) - { - $this->ttyMode = $ttyMode; - $this->ptyMode = $ptyMode; - $this->haveReadSupport = $haveReadSupport; - + public function __construct( + private ?bool $ttyMode, + private bool $ptyMode, + mixed $input, + private bool $haveReadSupport, + ) { parent::__construct($input); } diff --git a/Pipes/WindowsPipes.php b/Pipes/WindowsPipes.php index 8033442a..116b8e30 100644 --- a/Pipes/WindowsPipes.php +++ b/Pipes/WindowsPipes.php @@ -33,12 +33,11 @@ class WindowsPipes extends AbstractPipes Process::STDOUT => 0, Process::STDERR => 0, ]; - private bool $haveReadSupport; - - public function __construct(mixed $input, bool $haveReadSupport) - { - $this->haveReadSupport = $haveReadSupport; + public function __construct( + mixed $input, + private bool $haveReadSupport, + ) { if ($this->haveReadSupport) { // Fix for PHP bug #51800: reading from STDOUT pipe hangs forever on Windows if the output is too big. // Workaround for this problem is to use temporary files instead of pipes on Windows platform. @@ -53,7 +52,7 @@ public function __construct(mixed $input, bool $haveReadSupport) set_error_handler(function ($type, $msg) use (&$lastError) { $lastError = $msg; }); for ($i = 0;; ++$i) { foreach ($pipes as $pipe => $name) { - $file = sprintf('%s\\sf_proc_%02X.%s', $tmpDir, $i, $name); + $file = \sprintf('%s\\sf_proc_%02X.%s', $tmpDir, $i, $name); if (!$h = fopen($file.'.lock', 'w')) { if (file_exists($file.'.lock')) { diff --git a/Process.php b/Process.php index 6dfb25e7..6fe1086e 100644 --- a/Process.php +++ b/Process.php @@ -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; @@ -76,19 +77,21 @@ class Process implements \IteratorAggregate private bool $tty = false; private bool $pty; private array $options = ['suppress_errors' => true, 'bypass_shell' => true]; + private array $ignoredSignals = []; private WindowsPipes|UnixPipes $processPipes; private ?int $latestSignal = null; private static ?bool $sigchild = null; + private static array $executables = []; /** * Exit codes translation table. * * User-defined errors must use exit codes in the 64-113 range. */ - public static $exitCodes = [ + public static array $exitCodes = [ 0 => 'OK', 1 => 'General error', 2 => 'Misuse of shell builtins', @@ -199,10 +202,7 @@ public function __sleep(): array throw new \BadMethodCallException('Cannot serialize '.__CLASS__); } - /** - * @return void - */ - public function __wakeup() + public function __wakeup(): void { throw new \BadMethodCallException('Cannot unserialize '.__CLASS__); } @@ -236,11 +236,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 */ @@ -287,13 +287,11 @@ 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 * - * @return void - * - * @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 = []) + public function start(?callable $callback = null, array $env = []): void { if ($this->isRunning()) { throw new RuntimeException('Process is already running.'); @@ -311,12 +309,7 @@ public function start(?callable $callback = null, array $env = []) $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); } @@ -327,6 +320,11 @@ public function start(?callable $callback = null, array $env = []) // 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'; @@ -340,13 +338,42 @@ public function start(?callable $callback = null, array $env = []) } if (!is_dir($this->cwd)) { - throw new RuntimeException(sprintf('The provided cwd "%s" does not exist.', $this->cwd)); + 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; + }); + + $oldMask = []; + + if ($this->ignoredSignals && \function_exists('pcntl_sigprocmask')) { + // we block signals we want to ignore, as proc_open will use fork / posix_spawn which will copy the signal mask this allow to block + // signals in the child process + pcntl_sigprocmask(\SIG_BLOCK, $this->ignoredSignals, $oldMask); + } + + try { + $process = @proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options); + + // Ensure array vs string commands behave the same + if (!$process && \is_array($commandline)) { + $process = @proc_open('exec '.$this->buildShellCommandline($commandline), $descriptors, $this->processPipes->pipes, $this->cwd, $envPairs, $this->options); + } + } finally { + if ($this->ignoredSignals && \function_exists('pcntl_sigprocmask')) { + // we restore the signal mask here to avoid any side effects + pcntl_sigprocmask(\SIG_SETMASK, $oldMask); + } + + restore_error_handler(); + } if (!$process) { - throw new RuntimeException('Unable to launch a new process.'); + throw new ProcessStartFailedException($this, $lastError); } $this->process = $process; $this->status = self::STATUS_STARTED; @@ -371,8 +398,8 @@ public function start(?callable $callback = null, array $env = []) * @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() * @@ -948,7 +975,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); } /** @@ -1140,11 +1167,9 @@ public function setInput(mixed $input): static * In case you run a background process (with the start method), you should * trigger this method regularly to ensure the process timeout * - * @return void - * * @throws ProcessTimedOutException In case the timeout was reached */ - public function checkTimeout() + public function checkTimeout(): void { if (self::STATUS_STARTED !== $this->status) { return; @@ -1182,10 +1207,8 @@ public function getStartTime(): float * * Enabling the "create_new_console" option allows a subprocess to continue * to run after the main process exited, on both Windows and *nix - * - * @return void */ - public function setOptions(array $options) + public function setOptions(array $options): void { if ($this->isRunning()) { throw new RuntimeException('Setting options while the process is running is not possible.'); @@ -1197,12 +1220,26 @@ public function setOptions(array $options) foreach ($options as $key => $value) { if (!\in_array($key, $existingOptions)) { $this->options = $defaultOptions; - throw new LogicException(sprintf('Invalid option "%s" passed to "%s()". Supported options are "%s".', $key, __METHOD__, implode('", "', $existingOptions))); + throw new LogicException(\sprintf('Invalid option "%s" passed to "%s()". Supported options are "%s".', $key, __METHOD__, implode('", "', $existingOptions))); } $this->options[$key] = $value; } } + /** + * Defines a list of posix signals that will not be propagated to the process. + * + * @param list<\SIG*> $signals + */ + public function setIgnoredSignals(array $signals): void + { + if ($this->isRunning()) { + throw new RuntimeException('Setting ignored signals while the process is running is not possible.'); + } + + $this->ignoredSignals = $signals; + } + /** * Returns whether TTY is supported on the current operating system. */ @@ -1279,10 +1316,8 @@ protected function buildCallback(?callable $callback = null): \Closure * Updates the status of the process, reads pipes. * * @param bool $blocking Whether to use a blocking read call - * - * @return void */ - protected function updateStatus(bool $blocking) + protected function updateStatus(bool $blocking): void { if (self::STATUS_STARTED !== $this->status) { return; @@ -1444,6 +1479,11 @@ private function resetProcessData(): void */ private function doSignal(int $signal, bool $throwException): bool { + // Signal seems to be send when sigchild is enable, this allow blocking the signal correctly in this case + if ($this->isSigchildEnabled() && \in_array($signal, $this->ignoredSignals)) { + return false; + } + if (null === $pid = $this->getPid()) { if ($throwException) { throw new LogicException('Cannot send signal on a non running process.'); @@ -1453,10 +1493,10 @@ private function doSignal(int $signal, bool $throwException): bool } if ('\\' === \DIRECTORY_SEPARATOR) { - exec(sprintf('taskkill /F /T /PID %d 2>&1', $pid), $output, $exitCode); + exec(\sprintf('taskkill /F /T /PID %d 2>&1', $pid), $output, $exitCode); if ($exitCode && $this->isRunning()) { if ($throwException) { - throw new RuntimeException(sprintf('Unable to kill the process (%s).', implode(' ', $output))); + throw new RuntimeException(\sprintf('Unable to kill the process (%s).', implode(' ', $output))); } return false; @@ -1466,12 +1506,12 @@ private function doSignal(int $signal, bool $throwException): bool $ok = @proc_terminate($this->process, $signal); } elseif (\function_exists('posix_kill')) { $ok = @posix_kill($pid, $signal); - } elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $pid), [2 => ['pipe', 'w']], $pipes)) { + } elseif ($ok = proc_open(\sprintf('kill -%d %d', $signal, $pid), [2 => ['pipe', 'w']], $pipes)) { $ok = false === fgets($pipes[2]); } if (!$ok) { if ($throwException) { - throw new RuntimeException(sprintf('Error while sending signal "%s".', $signal)); + throw new RuntimeException(\sprintf('Error while sending signal "%s".', $signal)); } return false; @@ -1486,9 +1526,25 @@ 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; + } + + if ('\\' === \DIRECTORY_SEPARATOR && isset($commandline[0][0]) && \strlen($commandline[0]) === strcspn($commandline[0], ':/\\')) { + // On Windows, we don't rely on the OS to find the executable if possible to avoid lookups + // in the current directory which could be untrusted. Instead we use the ExecutableFinder. + $commandline[0] = (self::$executables[$commandline[0]] ??= (new ExecutableFinder())->find($commandline[0])) ?? $commandline[0]; + } + + return implode(' ', array_map($this->escapeArgument(...), $commandline)); + } + + private function prepareWindowsCommandLine(string|array $cmd, array &$env): string { - $uid = uniqid('', true); + $cmd = $this->buildShellCommandline($cmd); + $uid = bin2hex(random_bytes(4)); $cmd = preg_replace_callback( '/"(?:( [^"%!^]*+ @@ -1547,7 +1603,7 @@ function ($m) use (&$env, $uid) { private function requireProcessIsStarted(string $functionName): void { if (!$this->isStarted()) { - throw new LogicException(sprintf('Process must be started before calling "%s()".', $functionName)); + throw new LogicException(\sprintf('Process must be started before calling "%s()".', $functionName)); } } @@ -1559,7 +1615,7 @@ private function requireProcessIsStarted(string $functionName): void private function requireProcessIsTerminated(string $functionName): void { if (!$this->isTerminated()) { - throw new LogicException(sprintf('Process must be terminated before calling "%s()".', $functionName)); + throw new LogicException(\sprintf('Process must be terminated before calling "%s()".', $functionName)); } } @@ -1589,7 +1645,7 @@ private function replacePlaceholders(string $commandline, array $env): string { return preg_replace_callback('/"\$\{:([_a-zA-Z]++[_a-zA-Z0-9]*+)\}"/', function ($matches) use ($commandline, $env) { if (!isset($env[$matches[1]]) || false === $env[$matches[1]]) { - throw new InvalidArgumentException(sprintf('Command line is missing a value for parameter "%s": ', $matches[1]).$commandline); + throw new InvalidArgumentException(\sprintf('Command line is missing a value for parameter "%s": ', $matches[1]).$commandline); } return $this->escapeArgument($env[$matches[1]]); diff --git a/ProcessUtils.php b/ProcessUtils.php index 092c5ccf..a2dbde9f 100644 --- a/ProcessUtils.php +++ b/ProcessUtils.php @@ -56,7 +56,7 @@ public static function validateInput(string $caller, mixed $input): mixed return new \IteratorIterator($input); } - throw new InvalidArgumentException(sprintf('"%s" only accepts strings, Traversable objects or stream resources.', $caller)); + throw new InvalidArgumentException(\sprintf('"%s" only accepts strings, Traversable objects or stream resources.', $caller)); } return $input; diff --git a/Tests/ExecutableFinderTest.php b/Tests/ExecutableFinderTest.php index c102ab68..87288360 100644 --- a/Tests/ExecutableFinderTest.php +++ b/Tests/ExecutableFinderTest.php @@ -85,6 +85,31 @@ public function testFindWithExtraDirs() $this->assertSamePath(\PHP_BINARY, $result); } + public function testFindWithoutSuffix() + { + $fixturesDir = __DIR__.\DIRECTORY_SEPARATOR.'Fixtures'; + $name = 'executable_without_suffix'; + + $finder = new ExecutableFinder(); + $result = $finder->find($name, null, [$fixturesDir]); + + $this->assertSamePath($fixturesDir.\DIRECTORY_SEPARATOR.$name, $result); + } + + public function testFindWithAddedSuffixes() + { + $fixturesDir = __DIR__.\DIRECTORY_SEPARATOR.'Fixtures'; + $name = 'executable_with_added_suffix'; + $suffix = '.foo'; + + $finder = new ExecutableFinder(); + $finder->addSuffix($suffix); + + $result = $finder->find($name, null, [$fixturesDir]); + + $this->assertSamePath($fixturesDir.\DIRECTORY_SEPARATOR.$name.$suffix, $result); + } + /** * @runInSeparateProcess */ diff --git a/Tests/Fixtures/executable_with_added_suffix.foo b/Tests/Fixtures/executable_with_added_suffix.foo new file mode 100755 index 00000000..471a493a --- /dev/null +++ b/Tests/Fixtures/executable_with_added_suffix.foo @@ -0,0 +1 @@ +See \Symfony\Component\Process\Tests\ExecutableFinderTest::testFindWithAddedSuffixes() diff --git a/Tests/Fixtures/executable_without_suffix b/Tests/Fixtures/executable_without_suffix new file mode 100755 index 00000000..9bf8b4db --- /dev/null +++ b/Tests/Fixtures/executable_without_suffix @@ -0,0 +1 @@ +See \Symfony\Component\Process\Tests\ExecutableFinderTest::testFindWithoutSuffix() diff --git a/Tests/Messenger/RunProcessMessageHandlerTest.php b/Tests/Messenger/RunProcessMessageHandlerTest.php index 049da77a..e095fa09 100644 --- a/Tests/Messenger/RunProcessMessageHandlerTest.php +++ b/Tests/Messenger/RunProcessMessageHandlerTest.php @@ -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; } diff --git a/Tests/ProcessFailedExceptionTest.php b/Tests/ProcessFailedExceptionTest.php index 259ffd63..d05beb85 100644 --- a/Tests/ProcessFailedExceptionTest.php +++ b/Tests/ProcessFailedExceptionTest.php @@ -80,8 +80,8 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput( $exception = new ProcessFailedException($process); - $this->assertEquals( - "The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}", + $this->assertStringMatchesFormat( + "The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}", str_replace("'php'", 'php', $exception->getMessage()) ); } @@ -126,9 +126,9 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput() $exception = new ProcessFailedException($process); - $this->assertEquals( - "The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}", - str_replace("'php'", 'php', $exception->getMessage()) + $this->assertStringMatchesFormat( + "The command \"%s\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}", + $exception->getMessage() ); } } diff --git a/Tests/ProcessTest.php b/Tests/ProcessTest.php index e9c7527c..00d06208 100644 --- a/Tests/ProcessTest.php +++ b/Tests/ProcessTest.php @@ -66,6 +66,23 @@ public function testInvalidCwd() $cmd->run(); } + /** + * @dataProvider invalidProcessProvider + */ + public function testInvalidCommand(Process $process) + { + // An invalid command should not fail during start + $this->assertSame('\\' === \DIRECTORY_SEPARATOR ? 1 : 127, $process->run()); + } + + public static function invalidProcessProvider(): array + { + return [ + [new Process(['invalid'])], + [Process::fromShellCommandline('invalid')], + ]; + } + /** * @group transient-on-windows */ @@ -168,7 +185,7 @@ public function testAllOutputIsActuallyReadOnTermination() // another byte which will never be read. $expectedOutputSize = PipesInterface::CHUNK_SIZE * 2 + 2; - $code = sprintf('echo str_repeat(\'*\', %d);', $expectedOutputSize); + $code = \sprintf('echo str_repeat(\'*\', %d);', $expectedOutputSize); $p = $this->getProcessForCode($code); $p->start(); @@ -305,11 +322,13 @@ public function testSetInputWhileRunningThrowsAnException() /** * @dataProvider provideInvalidInputValues */ - public function testInvalidInput($value) + public function testInvalidInput(array|object $value) { + $process = $this->getProcess('foo'); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('"Symfony\Component\Process\Process::setInput" only accepts strings, Traversable objects or stream resources.'); - $process = $this->getProcess('foo'); + $process->setInput($value); } @@ -324,7 +343,7 @@ public static function provideInvalidInputValues() /** * @dataProvider provideInputValues */ - public function testValidInput($expected, $value) + public function testValidInput(?string $expected, float|string|null $value) { $process = $this->getProcess('foo'); $process->setInput($value); @@ -359,7 +378,7 @@ public static function chainedCommandsOutputProvider() */ public function testChainedCommandsOutput($expected, $operator, $input) { - $process = $this->getProcess(sprintf('echo %s %s echo %s', $input, $operator, $input)); + $process = $this->getProcess(\sprintf('echo %s %s echo %s', $input, $operator, $input)); $process->run(); $this->assertEquals($expected, $process->getOutput()); } @@ -584,8 +603,10 @@ public function testSuccessfulMustRunHasCorrectExitCode() public function testMustRunThrowsException() { - $this->expectException(ProcessFailedException::class); $process = $this->getProcess('exit 1'); + + $this->expectException(ProcessFailedException::class); + $process->mustRun(); } @@ -966,9 +987,11 @@ public function testExitCodeIsAvailableAfterSignal() public function testSignalProcessNotRunning() { + $process = $this->getProcess('foo'); + $this->expectException(LogicException::class); $this->expectExceptionMessage('Cannot send signal on a non running process.'); - $process = $this->getProcess('foo'); + $process->signal(1); // SIGHUP } @@ -980,7 +1003,7 @@ public function testMethodsThatNeedARunningProcess($method) $process = $this->getProcess('foo'); $this->expectException(LogicException::class); - $this->expectExceptionMessage(sprintf('Process must be started before calling "%s()".', $method)); + $this->expectExceptionMessage(\sprintf('Process must be started before calling "%s()".', $method)); $process->{$method}(); } @@ -1056,20 +1079,24 @@ public function testDisableOutputDisablesTheOutput() public function testDisableOutputWhileRunningThrowsException() { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Disabling output while the process is running is not possible.'); $p = $this->getProcessForCode('sleep(39);'); $p->start(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Disabling output while the process is running is not possible.'); + $p->disableOutput(); } public function testEnableOutputWhileRunningThrowsException() { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('Enabling output while the process is running is not possible.'); $p = $this->getProcessForCode('sleep(40);'); $p->disableOutput(); $p->start(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Enabling output while the process is running is not possible.'); + $p->enableOutput(); } @@ -1085,19 +1112,23 @@ public function testEnableOrDisableOutputAfterRunDoesNotThrowException() public function testDisableOutputWhileIdleTimeoutIsSet() { - $this->expectException(LogicException::class); - $this->expectExceptionMessage('Output cannot be disabled while an idle timeout is set.'); $process = $this->getProcess('foo'); $process->setIdleTimeout(1); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Output cannot be disabled while an idle timeout is set.'); + $process->disableOutput(); } public function testSetIdleTimeoutWhileOutputIsDisabled() { - $this->expectException(LogicException::class); - $this->expectExceptionMessage('timeout cannot be set while the output is disabled.'); $process = $this->getProcess('foo'); $process->disableOutput(); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('timeout cannot be set while the output is disabled.'); + $process->setIdleTimeout(1); } @@ -1113,11 +1144,13 @@ public function testSetNullIdleTimeoutWhileOutputIsDisabled() */ public function testGetOutputWhileDisabled($fetchMethod) { - $this->expectException(LogicException::class); - $this->expectExceptionMessage('Output has been disabled.'); $p = $this->getProcessForCode('sleep(41);'); $p->disableOutput(); $p->start(); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Output has been disabled.'); + $p->{$fetchMethod}(); } @@ -1475,7 +1508,7 @@ public function testEscapeArgument($arg) public function testRawCommandLine() { - $p = Process::fromShellCommandline(sprintf('"%s" -r %s "a" "" "b"', self::$phpBin, escapeshellarg('print_r($argv);'))); + $p = Process::fromShellCommandline(\sprintf('"%s" -r %s "a" "" "b"', self::$phpBin, escapeshellarg('print_r($argv);'))); $p->run(); $expected = "Array\n(\n [0] => -\n [1] => a\n [2] => \n [3] => b\n)\n"; @@ -1522,17 +1555,21 @@ public function testPreparedCommandWithQuoteInIt() public function testPreparedCommandWithMissingValue() { + $p = Process::fromShellCommandline('echo "${:abc}"'); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Command line is missing a value for parameter "abc": echo "${:abc}"'); - $p = Process::fromShellCommandline('echo "${:abc}"'); + $p->run(null, ['bcd' => 'BCD']); } public function testPreparedCommandWithNoValues() { + $p = Process::fromShellCommandline('echo "${:abc}"'); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Command line is missing a value for parameter "abc": echo "${:abc}"'); - $p = Process::fromShellCommandline('echo "${:abc}"'); + $p->run(null, []); } @@ -1642,6 +1679,50 @@ public function testNotTerminableInputPipe() $this->assertFalse($process->isRunning()); } + public function testIgnoringSignal() + { + if (!\function_exists('pcntl_signal')) { + $this->markTestSkipped('pnctl extension is required.'); + } + + $process = $this->getProcess(['sleep', '10']); + $process->setIgnoredSignals([\SIGTERM]); + + $process->start(); + $process->stop(timeout: 0.2); + + $this->assertNotSame(\SIGTERM, $process->getTermSignal()); + } + + // This test ensure that the previous test is reliable, in case of the sleep command ignoring the SIGTERM signal + public function testNotIgnoringSignal() + { + if (!\function_exists('pcntl_signal')) { + $this->markTestSkipped('pnctl extension is required.'); + } + if (\PHP_VERSION_ID < 80300 && isset($_SERVER['GITHUB_ACTIONS'])) { + $this->markTestSkipped('Transient on GHA with PHP < 8.3'); + } + + $process = $this->getProcess(['sleep', '10']); + + $process->start(); + $process->stop(timeout: 0.2); + + $this->assertSame(\SIGTERM, $process->getTermSignal()); + } + + public function testPathResolutionOnWindows() + { + if ('\\' !== \DIRECTORY_SEPARATOR) { + $this->markTestSkipped('This test is for Windows platform only'); + } + + $process = $this->getProcess(['where']); + + $this->assertSame('C:\\Windows\\system32\\where.EXE', $process->getCommandLine()); + } + private function getProcess(string|array $commandline, ?string $cwd = null, ?array $env = null, mixed $input = null, ?int $timeout = 60): Process { if (\is_string($commandline)) { diff --git a/Tests/SignalListener.php b/Tests/SignalListener.php index 618be740..7a351858 100644 --- a/Tests/SignalListener.php +++ b/Tests/SignalListener.php @@ -9,7 +9,10 @@ * file that was distributed with this source code. */ -pcntl_signal(\SIGUSR1, function () { echo 'SIGUSR1'; exit; }); +pcntl_signal(\SIGUSR1, function () { + echo 'SIGUSR1'; + exit; +}); echo 'Caught '; diff --git a/composer.json b/composer.json index 317c07e7..dda5575e 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ } ], "require": { - "php": ">=8.1" + "php": ">=8.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Process\\": "" },