Skip to content

Commit

Permalink
Do not use a shell in proc_open if not really needed
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored and sebastianbergmann committed Mar 28, 2024
1 parent 5e80471 commit fbf391a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 16 deletions.
37 changes: 22 additions & 15 deletions src/Util/PHP/AbstractPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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';

Check warning on line 188 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L188

Added line #L188 was not covered by tests

if (!$file) {
$command .= 's=';
$command[] = 's=';

Check warning on line 191 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L191

Added line #L191 was not covered by tests
}
}

if ($file) {
$command .= ' ' . escapeshellarg($file);
$command[] = '-f';
$command[] = $file;

Check warning on line 197 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L196-L197

Added lines #L196 - L197 were not covered by tests
}

if ($this->arguments) {
if (!$file) {
$command .= ' --';
$command[] = '--';

Check warning on line 202 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L202

Added line #L202 was not covered by tests
}
$command .= ' ' . $this->arguments;
}

if ($this->stderrRedirection) {
$command .= ' 2>&1';
foreach (explode(' ', $this->arguments) as $arg) {
$command[] = trim($arg);

Check warning on line 206 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L205-L206

Added lines #L205 - L206 were not covered by tests
}
}

return $command;
Expand All @@ -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<string>
*/
protected function settingsToParameters(array $settings): array
{
$buffer = '';
$buffer = [];

foreach ($settings as $setting) {
$buffer .= ' -d ' . escapeshellarg($setting);
$buffer[] = '-d';
$buffer[] = $setting;
}

return $buffer;
Expand Down
10 changes: 10 additions & 0 deletions src/Util/PHP/DefaultPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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);

Check failure on line 125 in src/Util/PHP/DefaultPhpProcess.php

View workflow job for this annotation

GitHub Actions / Type Checker

InvalidArgument

src/Util/PHP/DefaultPhpProcess.php:125:28: InvalidArgument: Argument 1 of time_nanosleep expects int<1, max>, but 0 provided (see https://psalm.dev/004)
}

$stderr = $stdout = '';

if (isset($pipes[1])) {
Expand Down
2 changes: 1 addition & 1 deletion src/Util/PHP/WindowsPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ protected function getHandles(): array

protected function useTemporaryFile(): bool
{
return true;
return false;

Check warning on line 41 in src/Util/PHP/WindowsPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/WindowsPhpProcess.php#L41

Added line #L41 was not covered by tests
}
}

0 comments on commit fbf391a

Please sign in to comment.