Skip to content

Commit a93da9b

Browse files
milodg
authored andcommitted
PhpInterpreter: pass command to proc_open() as array
- PHP escapes arguments itself - subshell call is bypassed on Linux too
1 parent 8010162 commit a93da9b

3 files changed

Lines changed: 22 additions & 21 deletions

File tree

src/Runner/Job.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,15 @@ public function run(bool $async = false): void
9595
$this->duration = -microtime(as_float: true);
9696
$this->proc = proc_open(
9797
$this->interpreter
98-
->withArguments(['-d register_argc_argv=on', $this->test->getFile(), ...$args])
99-
->getCommandLine(),
98+
->withArguments(['-d', 'register_argc_argv=on', $this->test->getFile(), ...$args])
99+
->getCommand(),
100100
[
101101
['pipe', 'r'],
102102
['pipe', 'w'],
103103
$this->stderrFile ? ['file', $this->stderrFile, 'w'] : ['pipe', 'w'],
104104
],
105105
$pipes,
106106
dirname($this->test->getFile()),
107-
null,
108-
['bypass_shell' => true],
109107
) ?: throw new \RuntimeException('Cannot start test process.');
110108

111109
foreach (array_keys($this->envVars) as $name) {

src/Runner/PhpInterpreter.php

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,16 @@
88
namespace Tester\Runner;
99

1010
use Tester\Helpers;
11-
use function count, in_array;
11+
use function array_map, count, explode, implode, in_array, str_contains;
1212

1313

1414
/**
1515
* Wraps a PHP executable and its resolved version, extensions, and command-line options.
1616
*/
1717
class PhpInterpreter
1818
{
19-
private string $commandLine;
19+
/** @var list<string> */
20+
private array $commandLine;
2021
private bool $cgi;
2122
private \stdClass $info;
2223
private string $error;
@@ -25,14 +26,10 @@ class PhpInterpreter
2526
/** @param list<string> $args */
2627
public function __construct(string $path, array $args = [])
2728
{
28-
$this->commandLine = Helpers::escapeArg($path);
2929
$proc = @proc_open( // @ is escalated to exception
30-
$this->commandLine . ' --version',
30+
[$path, '--version'],
3131
[['pipe', 'r'], ['pipe', 'w'], ['pipe', 'w']],
3232
$pipes,
33-
null,
34-
null,
35-
['bypass_shell' => true],
3633
);
3734
if ($proc === false) {
3835
throw new \Exception("Cannot run PHP interpreter $path. Use -p option.");
@@ -42,20 +39,16 @@ public function __construct(string $path, array $args = [])
4239
$output = stream_get_contents($pipes[1]);
4340
proc_close($proc);
4441

45-
$args = ' ' . implode(' ', array_map([Helpers::class, 'escapeArg'], $args));
4642
if (str_contains($output, 'phpdbg')) {
47-
$args = ' -qrrb -S cli' . $args;
43+
$args = ['-qrrb', '-S', 'cli', ...$args];
4844
}
4945

50-
$this->commandLine .= rtrim($args);
46+
$this->commandLine = [$path, ...$args];
5147

5248
$proc = proc_open(
53-
$this->commandLine . ' -d register_argc_argv=on ' . Helpers::escapeArg(__DIR__ . '/info.php') . ' serialized',
49+
[...$this->commandLine, '-d', 'register_argc_argv=on', __DIR__ . '/info.php', 'serialized'],
5450
[['pipe', 'r'], ['pipe', 'w'], ['pipe', 'w']],
5551
$pipes,
56-
null,
57-
null,
58-
['bypass_shell' => true],
5952
) ?: throw new \Exception("Unable to run $path.");
6053

6154
$output = stream_get_contents($pipes[1]);
@@ -88,7 +81,7 @@ public function __construct(string $path, array $args = [])
8881
public function withArguments(array $args): static
8982
{
9083
$me = clone $this;
91-
$me->commandLine .= ' ' . implode(' ', array_map([Helpers::class, 'escapeArg'], $args));
84+
$me->commandLine = [...$me->commandLine, ...$args];
9285
return $me;
9386
}
9487

@@ -98,11 +91,18 @@ public function withArguments(array $args): static
9891
*/
9992
public function withPhpIniOption(string $name, ?string $value = null): static
10093
{
101-
return $this->withArguments(['-d ' . $name . ($value === null ? '' : "=$value")]);
94+
return $this->withArguments(['-d', $name . ($value === null ? '' : "=$value")]);
10295
}
10396

10497

10598
public function getCommandLine(): string
99+
{
100+
return implode(' ', array_map([Helpers::class, 'escapeArg'], $this->commandLine));
101+
}
102+
103+
104+
/** @return list<string> */
105+
public function getCommand(): array
106106
{
107107
return $this->commandLine;
108108
}

tests/Runner/PhpInterpreter.phpt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ Assert::true($interpreter->hasExtension('DaTe'));
1010
Assert::false($interpreter->hasExtension('foo-bar'));
1111

1212
Assert::contains(PHP_BINARY, $interpreter->getCommandLine());
13+
Assert::type('list', $interpreter->getCommand());
14+
Assert::same(PHP_BINARY, $interpreter->getCommand()[0]);
15+
Assert::same([...$interpreter->getCommand(), '-v'], $interpreter->withArguments(['-v'])->getCommand());
1316
Assert::same(PHP_VERSION, $interpreter->getVersion());
1417
Assert::same(str_contains(PHP_SAPI, 'cgi'), $interpreter->isCgi());
1518

@@ -35,6 +38,6 @@ Assert::count($count, $engines);
3538

3639
// createInterpreter() uses same php.ini as parent
3740
if (!$interpreter->isCgi()) {
38-
$output = shell_exec($interpreter->withArguments(['-r echo php_ini_loaded_file();'])->getCommandLine());
41+
$output = shell_exec($interpreter->withArguments(['-r', 'echo php_ini_loaded_file();'])->getCommandLine());
3942
Assert::same(php_ini_loaded_file(), $output);
4043
}

0 commit comments

Comments
 (0)