[Process] Fix escaping on Windows

Windows escaping is broken since the last merges.
symfony · Mar 18, 2014 · 0f65f90 · 0f65f90
1 parent 02088bc
commit 0f65f90
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 12 deletions.
diff --git a/src/Symfony/Component/Process/ProcessUtils.php b/src/Symfony/Component/Process/ProcessUtils.php
@@ -47,20 +47,18 @@ public static function escapeArgument($argument)
 
             $escapedArgument = '';
             $quote =  false;
-            foreach (preg_split('/([%"])/i', $argument, -1, PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE) as $part) {
+            foreach (preg_split('/(")/i', $argument, -1, PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE) as $part) {
                 if ('"' === $part) {
                     $escapedArgument .= '\\"';
-                } elseif ('%' === $part) {
-                    $escapedArgument .= '^%';
+                } elseif (self::isSurroundedBy($part, '%')) {
+                    // Avoid environment variable expansion
+                    $escapedArgument .= '^%"'.substr($part, 1, -1).'"^%';
                 } else {
+                    // escape trailing backslash
                     if ('\\' === substr($part, -1)) {
                         $part .= '\\';
                     }
-                    $part = escapeshellarg($part);
-                    if ('"' === $part[0] && '"' === $part[strlen($part) - 1]) {
-                        $part = substr($part, 1, -1);
-                        $quote = true;
-                    }
+                    $quote = true;
                     $escapedArgument .= $part;
                 }
             }
@@ -73,4 +71,9 @@ public static function escapeArgument($argument)
 
         return escapeshellarg($argument);
     }
+
+    private static function isSurroundedBy($arg, $char)
+    {
+        return 2 < strlen($arg) && $char === $arg[0] && $char === $arg[strlen($arg) - 1];
+    }
 }
diff --git a/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php b/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
@@ -142,13 +142,13 @@ public function testPrefixIsPrependedToAllGeneratedProcess()
 
     public function testShouldEscapeArguments()
     {
-        $pb = new ProcessBuilder(array('%path%', 'foo " bar'));
+        $pb = new ProcessBuilder(array('%path%', 'foo " bar', '%baz%baz'));
         $proc = $pb->getProcess();
 
         if (defined('PHP_WINDOWS_VERSION_BUILD')) {
-            $this->assertSame('^%"path"^% "foo "\\"" bar"', $proc->getCommandLine());
+            $this->assertSame('^%"path"^% "foo \\" bar" "%baz%baz"', $proc->getCommandLine());
         } else {
-            $this->assertSame("'%path%' 'foo \" bar'", $proc->getCommandLine());
+            $this->assertSame("'%path%' 'foo \" bar' '%baz%baz'", $proc->getCommandLine());
         }
     }
 

diff --git a/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php b/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php
@@ -30,7 +30,7 @@ public function dataArguments()
                 array('"\"php\" \"-v\""', '"php" "-v"'),
                 array('"foo bar"', 'foo bar'),
                 array('^%"path"^%', '%path%'),
-                array('"<|>"\\"" "\\""\'f"', '<|>" "\'f'),
+                array('"<|>\\" \\"\'f"', '<|>" "\'f'),
                 array('""', ''),
                 array('"with\trailingbs\\\\"', 'with\trailingbs\\'),
             );