fix: Fix the double escaping done to options passed to the child process (#325)

Author: theofidry

src/Input/InputOptionsSerializer.php

@@ -22,18 +22,13 @@
 use function array_map;
 use function array_merge;
 use function is_array;
-use function is_string;
-use function preg_match;
 use function sprintf;
-use function str_replace;
 
 /**
  * @internal
  */
 final class InputOptionsSerializer
 {
-    private const string ESCAPE_TOKEN_PATTERN = '/[\s\W]/';
-
     private function __construct()
     {
     }
@@ -81,10 +76,7 @@ private static function serializeOption(
         return match (true) {
             $option->isNegatable() => sprintf('--%s%s', $value ? '' : 'no-', $name),
             !$option->acceptValue() => sprintf('--%s', $name),
-            self::isArray($option, $value) => array_map(
-                static fn ($item) => self::serializeOptionWithValue($name, $item),
-                $value,
-            ),
+            self::isArray($option, $value) => array_map(static fn ($item) => self::serializeOptionWithValue($name, $item), $value),
             default => self::serializeOptionWithValue($name, $value),
         };
     }
@@ -105,34 +97,6 @@ private static function serializeOptionWithValue(
         string $name,
         bool|float|int|string|null $value,
     ): string {
-        return sprintf(
-            '--%s=%s',
-            $name,
-            self::quoteOptionValue($value),
-        );
-    }
-
-    /**
-     * Ensure that an option value is quoted correctly before it is passed to a
-     * child process.
-     */
-    private static function quoteOptionValue(bool|float|int|string|null $value): bool|float|int|string|null
-    {
-        if (self::isValueRequiresQuoting($value)) {
-            return sprintf(
-                '"%s"',
-                str_replace('"', '\"', (string) $value),
-            );
-        }
-
-        return $value;
-    }
-
-    /**
-     * Validate whether a command option requires quoting.
-     */
-    private static function isValueRequiresQuoting(mixed $value): bool
-    {
-        return is_string($value) && 0 < preg_match(self::ESCAPE_TOKEN_PATTERN, $value);
+        return sprintf('--%s=%s', $name, $value);
     }
 }

tests/Input/InputOptionsSerializerTest.php

@@ -255,7 +255,11 @@ private static function optionSerializationProvider(): iterable
                 InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY,
             ),
             ['--opt' => ['v1', 'v2', 'v3']],
-            ['--opt=v1', '--opt=v2', '--opt=v3'],
+            [
+                '--opt=v1',
+                '--opt=v2',
+                '--opt=v3',
+            ],
         );
 
         if (!$isSymfony4) {
@@ -315,22 +319,22 @@ private static function escapedValuesProvider(): iterable
 
         yield $createSet(
             '"foo"',
-            '"\"foo\""',
+            '"foo"',
         );
 
         yield $createSet(
             '"o_id in(\'20\')"',
-            '"\"o_id in(\'20\')\""',
+            '"o_id in(\'20\')"',
         );
 
         yield $createSet(
             'a b c d',
-            '"a b c d"',
+            'a b c d',
         );
 
         yield $createSet(
             "A\nB'C",
-            "\"A\nB'C\"",
+            "A\nB'C",
         );
     }
 

tests/Integration/DebugChildProcessInputsTest.php

@@ -49,13 +49,19 @@ public function test_it_can_run_the_command_without_sub_processes(
         string $item,
         ?string $simpleOption,
         array $arrayOption,
-        string $expected,
+        ?string $expected = null,
     ): void {
         $logger = new DummyLogger();
 
         $this->command->setItem($item);
         $this->command->setLogger($logger);
 
+        $expected ??= DebugChildProcessCommand::createContent(
+            $item,
+            $simpleOption,
+            $arrayOption,
+        );
+
         $this->commandTester->execute(
             [
                 'command' => 'debug:process',
@@ -78,55 +84,30 @@ public static function inputProvider(): iterable
             'item',
             null,
             [],
-            DebugChildProcessCommand::createContent(
-                'item',
-                '',
-                [],
-            ),
         ];
 
         yield 'with values' => [
             'item',
             'option',
             ['option1', 'option2'],
-            DebugChildProcessCommand::createContent(
-                'item',
-                'option',
-                ['option1', 'option2'],
-            ),
         ];
 
         yield 'escaped string token' => [
             '"foo"',
             '"bar"',
             ['"option1"', '"option2"'],
-            DebugChildProcessCommand::createContent(
-                '"foo"',
-                '"\"bar\""',
-                ['"\"option1\""', '"\"option2\""'],
-            ),
         ];
 
         yield 'escaped string token with both types of quotes' => [
             '"o_id in(\'20\')"',
             '"p_id in(\'22\')"',
             ['"option in(\'1\')"', '"option in(\'2\')"'],
-            DebugChildProcessCommand::createContent(
-                '"o_id in(\'20\')"',
-                '"\"p_id in(\'22\')\""',
-                ['"\"option in(\'1\')\""', '"\"option in(\'2\')\""'],
-            ),
         ];
 
         yield 'with values with spaces' => [
             'a b c d',
             'd c b a',
             ['option 1', 'option 2'],
-            DebugChildProcessCommand::createContent(
-                'a b c d',
-                '"d c b a"',
-                ['"option 1"', '"option 2"'],
-            ),
         ];
     }
 }