From 0f65f90b065c6131c88b8ee45fc4801e85fb081c Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 18 Mar 2014 14:37:22 +0100 Subject: [PATCH] [Process] Fix escaping on Windows Windows escaping is broken since the last merges. --- .../Component/Process/ProcessUtils.php | 19 +++++++++++-------- .../Process/Tests/ProcessBuilderTest.php | 6 +++--- .../Process/Tests/ProcessUtilsTest.php | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Symfony/Component/Process/ProcessUtils.php b/src/Symfony/Component/Process/ProcessUtils.php index 7c9887fdbe2c..5317cd088394 100644 --- 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 index 4c88b55ce110..d0611f0b415e 100644 --- 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 index 288d4f369d53..8ba94c114d26 100644 --- 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\\'), );