From 19c41a3e1f5aea8f314a8a7aede57b94ceb8f0ab Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 22:20:00 +0100 Subject: [PATCH 1/4] Do not use a shell in proc_open if not really needed --- src/Util/PHP/AbstractPhpProcess.php | 37 +++++++++++++++++------------ src/Util/PHP/DefaultPhpProcess.php | 10 ++++++++ src/Util/PHP/WindowsPhpProcess.php | 2 +- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/Util/PHP/AbstractPhpProcess.php b/src/Util/PHP/AbstractPhpProcess.php index ddc909b0006..835d9148cd2 100644 --- a/src/Util/PHP/AbstractPhpProcess.php +++ b/src/Util/PHP/AbstractPhpProcess.php @@ -13,7 +13,7 @@ use function array_keys; use function array_merge; use function assert; -use function escapeshellarg; +use function explode; use function file_exists; use function file_get_contents; use function ini_get_all; @@ -156,12 +156,15 @@ public function runTestJob(string $job, Test $test, string $processResultFile): /** * Returns the command based into the configurations. + * + * @return string[] */ - public function getCommand(array $settings, ?string $file = null): string + public function getCommand(array $settings, ?string $file = null): array { $runtime = new Runtime; - $command = $runtime->getBinary(); + $command = []; + $command[] = $runtime->getRawBinary(); if ($runtime->hasPCOV()) { $settings = array_merge( @@ -179,29 +182,29 @@ public function getCommand(array $settings, ?string $file = null): string ); } - $command .= $this->settingsToParameters($settings); + $command = array_merge($command, $this->settingsToParameters($settings)); if (PHP_SAPI === 'phpdbg') { - $command .= ' -qrr'; + $command[] = '-qrr'; if (!$file) { - $command .= 's='; + $command[] = 's='; } } if ($file) { - $command .= ' ' . escapeshellarg($file); + $command[] = '-f'; + $command[] = $file; } if ($this->arguments) { if (!$file) { - $command .= ' --'; + $command[] = '--'; } - $command .= ' ' . $this->arguments; - } - if ($this->stderrRedirection) { - $command .= ' 2>&1'; + foreach (explode(' ', $this->arguments) as $arg) { + $command[] = trim($arg); + } } return $command; @@ -212,12 +215,16 @@ public function getCommand(array $settings, ?string $file = null): string */ abstract public function runJob(string $job, array $settings = []): array; - protected function settingsToParameters(array $settings): string + /** + * @return list + */ + protected function settingsToParameters(array $settings): array { - $buffer = ''; + $buffer = []; foreach ($settings as $setting) { - $buffer .= ' -d ' . escapeshellarg($setting); + $buffer[] = '-d'; + $buffer[] = $setting; } return $buffer; diff --git a/src/Util/PHP/DefaultPhpProcess.php b/src/Util/PHP/DefaultPhpProcess.php index 658c0ce7fba..16568274c4d 100644 --- a/src/Util/PHP/DefaultPhpProcess.php +++ b/src/Util/PHP/DefaultPhpProcess.php @@ -16,11 +16,13 @@ use function is_array; use function is_resource; use function proc_close; +use function proc_get_status; use function proc_open; use function rewind; use function stream_get_contents; use function sys_get_temp_dir; use function tempnam; +use function time_nanosleep; use function unlink; use PHPUnit\Framework\Exception; @@ -95,6 +97,10 @@ protected function runProcess(string $job, array $settings): array 2 => $handles[2] ?? ['pipe', 'w'], ]; + if ($this->stderrRedirection) { + $pipeSpec[2] = ['redirect', 1]; + } + $process = proc_open( $this->getCommand($settings, $this->tempFile), $pipeSpec, @@ -115,6 +121,10 @@ protected function runProcess(string $job, array $settings): array fclose($pipes[0]); + while (proc_get_status($process)['running'] === true) { + time_nanosleep(0, 100000); + } + $stderr = $stdout = ''; if (isset($pipes[1])) { diff --git a/src/Util/PHP/WindowsPhpProcess.php b/src/Util/PHP/WindowsPhpProcess.php index 74524996d36..35ac11aa8b8 100644 --- a/src/Util/PHP/WindowsPhpProcess.php +++ b/src/Util/PHP/WindowsPhpProcess.php @@ -38,6 +38,6 @@ protected function getHandles(): array protected function useTemporaryFile(): bool { - return true; + return false; } } From 932073191341a41409ea2d29193302c228e3a00b Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 28 Mar 2024 09:22:18 +0100 Subject: [PATCH 2/4] Remove windows stdout workarounds --- src/Util/PHP/DefaultPhpProcess.php | 39 +++--------------------------- src/Util/PHP/WindowsPhpProcess.php | 20 --------------- 2 files changed, 3 insertions(+), 56 deletions(-) diff --git a/src/Util/PHP/DefaultPhpProcess.php b/src/Util/PHP/DefaultPhpProcess.php index 16568274c4d..eaa05a6827a 100644 --- a/src/Util/PHP/DefaultPhpProcess.php +++ b/src/Util/PHP/DefaultPhpProcess.php @@ -16,13 +16,10 @@ use function is_array; use function is_resource; use function proc_close; -use function proc_get_status; use function proc_open; -use function rewind; use function stream_get_contents; use function sys_get_temp_dir; use function tempnam; -use function time_nanosleep; use function unlink; use PHPUnit\Framework\Exception; @@ -57,14 +54,6 @@ public function runJob(string $job, array $settings = []): array return $this->runProcess($job, $settings); } - /** - * Returns an array of file handles to be used in place of pipes. - */ - protected function getHandles(): array - { - return []; - } - /** * Handles creating the child process and returning the STDOUT and STDERR. * @@ -75,8 +64,6 @@ protected function getHandles(): array */ protected function runProcess(string $job, array $settings): array { - $handles = $this->getHandles(); - $env = null; if ($this->env) { @@ -92,9 +79,9 @@ protected function runProcess(string $job, array $settings): array } $pipeSpec = [ - 0 => $handles[0] ?? ['pipe', 'r'], - 1 => $handles[1] ?? ['pipe', 'w'], - 2 => $handles[2] ?? ['pipe', 'w'], + 0 => ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], ]; if ($this->stderrRedirection) { @@ -121,10 +108,6 @@ protected function runProcess(string $job, array $settings): array fclose($pipes[0]); - while (proc_get_status($process)['running'] === true) { - time_nanosleep(0, 100000); - } - $stderr = $stdout = ''; if (isset($pipes[1])) { @@ -139,22 +122,6 @@ protected function runProcess(string $job, array $settings): array fclose($pipes[2]); } - if (isset($handles[1])) { - rewind($handles[1]); - - $stdout = stream_get_contents($handles[1]); - - fclose($handles[1]); - } - - if (isset($handles[2])) { - rewind($handles[2]); - - $stderr = stream_get_contents($handles[2]); - - fclose($handles[2]); - } - proc_close($process); $this->cleanup(); diff --git a/src/Util/PHP/WindowsPhpProcess.php b/src/Util/PHP/WindowsPhpProcess.php index 35ac11aa8b8..75fa9ab909b 100644 --- a/src/Util/PHP/WindowsPhpProcess.php +++ b/src/Util/PHP/WindowsPhpProcess.php @@ -9,9 +9,6 @@ */ namespace PHPUnit\Util\PHP; -use function tmpfile; -use PHPUnit\Framework\Exception; - /** * @internal This class is not covered by the backward compatibility promise for PHPUnit * @@ -19,23 +16,6 @@ */ final class WindowsPhpProcess extends DefaultPhpProcess { - /** - * @throws Exception - * @throws PhpProcessException - */ - protected function getHandles(): array - { - if (false === $stdout_handle = tmpfile()) { - throw new PhpProcessException( - 'A temporary file could not be created; verify that your TEMP environment variable is writable', - ); - } - - return [ - 1 => $stdout_handle, - ]; - } - protected function useTemporaryFile(): bool { return false; From 982c7a0a1146073f7b4b1f5652ccefe51d2fcb24 Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Thu, 28 Mar 2024 09:59:12 +0100 Subject: [PATCH 3/4] Use PHP_BINARY instead of deprecated Runtime::getRawBinary() --- src/Util/PHP/AbstractPhpProcess.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Util/PHP/AbstractPhpProcess.php b/src/Util/PHP/AbstractPhpProcess.php index 835d9148cd2..feb0344661c 100644 --- a/src/Util/PHP/AbstractPhpProcess.php +++ b/src/Util/PHP/AbstractPhpProcess.php @@ -9,6 +9,7 @@ */ namespace PHPUnit\Util\PHP; +use const PHP_BINARY; use const PHP_SAPI; use function array_keys; use function array_merge; @@ -164,7 +165,7 @@ public function getCommand(array $settings, ?string $file = null): array $runtime = new Runtime; $command = []; - $command[] = $runtime->getRawBinary(); + $command[] = PHP_BINARY; if ($runtime->hasPCOV()) { $settings = array_merge( From 500149c212d6cec5707ec49f7eeb23d8997abdbd Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Thu, 28 Mar 2024 10:09:15 +0100 Subject: [PATCH 4/4] Update ChangeLog --- ChangeLog-10.5.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog-10.5.md b/ChangeLog-10.5.md index 70c05c9bd78..4608764922b 100644 --- a/ChangeLog-10.5.md +++ b/ChangeLog-10.5.md @@ -4,6 +4,10 @@ All notable changes of the PHPUnit 10.5 release series are documented in this fi ## [10.5.16] - 2024-MM-DD +### Changed + +* [#5766](https://github.com/sebastianbergmann/phpunit/pull/5766): Do not use a shell in `proc_open()` if not really needed + ### Fixed * [#5570](https://github.com/sebastianbergmann/phpunit/pull/5570): Windows does not support exclusive locks on stdout