fix: Fix incorrect PHP executable escaping (#329)

Author: theofidry

src/ParallelExecutorFactory.php

@@ -24,7 +24,6 @@
 use Webmozarts\Console\Parallelization\Process\StandardSymfonyProcessFactory;
 use Webmozarts\Console\Parallelization\Process\SymfonyProcessLauncherFactory;
 use function chr;
-use function explode;
 use function is_string;
 use function Safe\getcwd;
 use function str_starts_with;
@@ -232,16 +231,12 @@ public function withProgressSymbol(string $progressSymbol): self
      * The path of the PHP executable. It is the executable that will be used
      * to spawn the child process(es).
      *
-     * @param string|list<string> $phpExecutable
+     * @param string|list<string> $phpExecutable e.g. ['/path/to/php', '-dmemory_limit=512M']
      */
     public function withPhpExecutable(string|array $phpExecutable): self
     {
-        $normalizedExecutable = is_string($phpExecutable)
-            ? explode(' ', $phpExecutable)
-            : $phpExecutable;
-
         $clone = clone $this;
-        $clone->phpExecutable = $normalizedExecutable;
+        $clone->phpExecutable = is_string($phpExecutable) ? [$phpExecutable] : $phpExecutable;
 
         return $clone;
     }

tests/Input/ChildCommandFactoryTest.php

@@ -270,6 +270,31 @@ public static function childProvider(): iterable
             ];
         })();
 
+        yield 'enriched PHP executable with space (array case)' => (static function () use (
+            $scriptPath,
+            $commandName
+        ) {
+            [$input, $commandDefinition] = self::createInput(
+                [],
+                [],
+            );
+
+            return [
+                ['/path/to/my php', '-dmemory_limit=1'],
+                $scriptPath,
+                $commandName,
+                $commandDefinition,
+                $input,
+                [
+                    '/path/to/my php',
+                    '-dmemory_limit=1',
+                    $scriptPath,
+                    $commandName,
+                    '--child',
+                ],
+            ];
+        })();
+
         yield 'no PHP executable or command' => (static function () use (
             $scriptPath
         ) {

tests/ParallelExecutorFactoryTest.php

@@ -16,6 +16,7 @@
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use ReflectionClass;
 use Symfony\Component\Console\Input\InputDefinition;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Process\Process;
@@ -111,8 +112,11 @@ public function test_it_can_create_a_configured_executor(): void
         self::assertEquals($expected, $executor);
     }
 
-    public function test_it_escapes_the_php_executable_when_necessary(): void
-    {
+    #[DataProvider('phpExecutableProvider')]
+    public function test_it_escapes_the_php_executable_when_necessary(
+        string|array $phpExecutable,
+        array $expected,
+    ): void {
         $commandName = 'import:items';
         $definition = new InputDefinition();
         $errorHandler = new FakeErrorHandler();
@@ -121,32 +125,60 @@ public function test_it_escapes_the_php_executable_when_necessary(): void
         $callable1 = self::createCallable(1);
         $callable2 = self::createCallable(2);
 
-        $executorWithStringPhpExecutable = ParallelExecutorFactory::create(
+        $factory = ParallelExecutorFactory::create(
             $callable0,
             $callable1,
             $callable2,
             $commandName,
             $definition,
             $errorHandler,
         )
-            ->withPhpExecutable('/path/to/php -dmemory_limit=128M')
-            ->build();
+            ->withPhpExecutable($phpExecutable);
 
-        $executorWithArrayPhpExecutable = ParallelExecutorFactory::create(
-            $callable0,
-            $callable1,
-            $callable2,
-            $commandName,
-            $definition,
-            $errorHandler,
-        )
-            ->withPhpExecutable([
-                '/path/to/php',
-                '-dmemory_limit=128M',
-            ])
-            ->build();
+        $phpExecutableReflection = (new ReflectionClass($factory::class))->getProperty('phpExecutable');
+        $phpExecutableReflection->setAccessible(true);
+
+        $actual = $phpExecutableReflection->getValue($factory);
+
+        self::assertSame($expected, $actual);
+    }
 
-        self::assertEquals($executorWithArrayPhpExecutable, $executorWithStringPhpExecutable);
+    public static function phpExecutableProvider(): iterable
+    {
+        yield 'simple path' => [
+            '/path/to/php',
+            ['/path/to/php'],
+        ];
+
+        yield 'path with space' => [
+            '/path/to/my php',
+            ['/path/to/my php'],
+        ];
+
+        yield 'path with space (array)' => [
+            ['/path/to/my php'],
+            ['/path/to/my php'],
+        ];
+
+        yield 'path with php setting (incorrect)' => [
+            '/path/to/php -dmemory_limit=128M',
+            ['/path/to/php -dmemory_limit=128M'],
+        ];
+
+        yield 'path with php setting (array)' => [
+            ['/path/to/php', '-dmemory_limit=128M'],
+            ['/path/to/php', '-dmemory_limit=128M'],
+        ];
+
+        yield 'path with space and with php setting (incorrect)' => [
+            '/path/to/my php -dmemory_limit=128M',
+            ['/path/to/my php -dmemory_limit=128M'],
+        ];
+
+        yield 'path with space and with php setting (array)' => [
+            ['/path/to/my php', '-dmemory_limit=128M'],
+            ['/path/to/my php', '-dmemory_limit=128M'],
+        ];
     }
 
     public function test_it_sets_the_batch_size_to_the_segment_size_by_default(): void
@@ -234,7 +266,7 @@ public function test_it_can_create_an_executor_with_default_values(
         string $expectedSymbol,
         string $expectedPhpExecutable,
         string $expectedScriptPath,
-        string $expectedWorkingDirectory
+        string $expectedWorkingDirectory,
     ): void {
         $cleanUpWorkingDirectory = self::moveToWorkingDirectory($workingDirectory);
         $cleanUpEnvironmentVariables = EnvironmentVariables::setVariables($environmentVariables);