Do not use a shell in proc_open if not really needed

sebastianbergmann · Mar 28, 2024 · 4e96332 · 4e96332
1 parent f8941b6
commit 4e96332
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 16 deletions.
diff --git 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<string>
+     */
+    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
@@ -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
@@ -38,6 +38,6 @@ protected function getHandles(): array
 
     protected function useTemporaryFile(): bool
     {
-        return true;
+        return false;
     }
 }