Merge remote-tracking branch 'johnstevenson/escape' into 1.10

src/Composer/Util/ProcessExecutor.php

@@ -13,6 +13,7 @@
 namespace Composer\Util;
 
 use Composer\IO\IOInterface;
+use Composer\Util\Platform;
 use Symfony\Component\Process\Process;
 use Symfony\Component\Process\ProcessUtils;
 
@@ -155,33 +156,50 @@ public static function escape($argument)
     }
 
     /**
-     * Copy of Symfony's Process::escapeArgument() which is private
+     * Escapes a string to be used as a shell argument for Symfony Process.
+     *
+     * This method expects cmd.exe to be started with the /V:ON option, which
+     * enables delayed environment variable expansion using ! as the delimiter.
+     * If this is not the case, any escaped ^^!var^^! will be transformed to
+     * ^!var^! and introduce two unintended carets.
+     *
+     * Modified from https://github.com/johnstevenson/winbox-args
+     * MIT Licensed (c) John Stevenson <john-stevenson@blueyonder.co.uk>
      *
      * @param string $argument
      *
      * @return string
      */
     private static function escapeArgument($argument)
     {
-        if ('' === $argument || null === $argument) {
-            return '""';
+        if ('' === ($argument = (string) $argument)) {
+            return escapeshellarg($argument);
         }
-        if ('\\' !== \DIRECTORY_SEPARATOR) {
+
+        if (!Platform::isWindows()) {
             return "'".str_replace("'", "'\\''", $argument)."'";
         }
-        if (false !== strpos($argument, "\0")) {
-            $argument = str_replace("\0", '?', $argument);
+
+        // New lines break cmd.exe command parsing
+        $argument = strtr($argument, "\n", ' ');
+
+        $quote = strpbrk($argument, " \t") !== false;
+        $argument = preg_replace('/(\\\\*)"/', '$1$1\\"', $argument, -1, $dquotes);
+        $meta = $dquotes || preg_match('/%[^%]+%|![^!]+!/', $argument);
+
+        if (!$meta && !$quote) {
+            $quote = strpbrk($argument, '^&|<>()') !== false;
         }
-        if (!preg_match('/[\/()%!^"<>&|\s]/', $argument)) {
-            return $argument;
+
+        if ($quote) {
+            $argument = '"'.preg_replace('/(\\\\*)$/', '$1$1', $argument).'"';
         }
-        $argument = preg_replace('/(\\\\+)$/', '$1$1', $argument);
 
-        return '"'.str_replace(array('"', '^', '%', '!', "\n"), array('""', '"^^"', '"^%"', '"^!"', '!LF!'), $argument).'"';
-    }
+        if ($meta) {
+            $argument = preg_replace('/(["^&|<>()%])/', '^$1', $argument);
+            $argument = preg_replace('/(!)/', '^^$1', $argument);
+        }
 
-    private static function isSurroundedBy($arg, $char)
-    {
-        return 2 < strlen($arg) && $char === $arg[0] && $char === $arg[strlen($arg) - 1];
+        return $argument;
     }
 }

tests/Composer/Test/Downloader/FossilDownloaderTest.php

@@ -15,7 +15,6 @@
 use Composer\Downloader\FossilDownloader;
 use Composer\Test\TestCase;
 use Composer\Util\Filesystem;
-use Composer\Util\Platform;
 
 class FossilDownloaderTest extends TestCase
 {
@@ -168,9 +167,4 @@ public function testGetInstallationSource()
 
         $this->assertEquals('source', $downloader->getInstallationSource());
     }
-
-    private function getCmd($cmd)
-    {
-        return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd;
-    }
 }

tests/Composer/Test/Downloader/GitDownloaderTest.php

@@ -524,7 +524,7 @@ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails()
 
     public function testUpdateDoesntThrowsRuntimeExceptionIfGitCommandFailsAtFirstButIsAbleToRecover()
     {
-        $expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer -- \"\" && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- \"\"");
+        $expectedFirstGitUpdateCommand = $this->winCompat("(git remote set-url composer -- '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- ''");
         $expectedSecondGitUpdateCommand = $this->winCompat("(git remote set-url composer -- 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)) && git remote set-url composer -- 'https://github.com/composer/composer'");
 
         $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock();
@@ -701,7 +701,7 @@ private function winCompat($cmd)
             $cmd = str_replace('cd ', 'cd /D ', $cmd);
             $cmd = str_replace('composerPath', getcwd().'/composerPath', $cmd);
 
-            return strtr($cmd, "'", '"');
+            return $this->getCmd($cmd);
         }
 
         return $cmd;

tests/Composer/Test/Downloader/HgDownloaderTest.php

@@ -15,7 +15,6 @@
 use Composer\Downloader\HgDownloader;
 use Composer\Test\TestCase;
 use Composer\Util\Filesystem;
-use Composer\Util\Platform;
 
 class HgDownloaderTest extends TestCase
 {
@@ -157,9 +156,4 @@ public function testGetInstallationSource()
 
         $this->assertEquals('source', $downloader->getInstallationSource());
     }
-
-    private function getCmd($cmd)
-    {
-        return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd;
-    }
 }

tests/Composer/Test/EventDispatcher/EventDispatcherTest.php

@@ -401,7 +401,7 @@ public function testRecursionInScriptsNames()
 
         $dispatcher->dispatch('helloWorld', new CommandEvent('helloWorld', $composer, $io));
         $expected = "> helloWorld: @hello World".PHP_EOL.
-            "> hello: echo Hello " .escapeshellarg('World').PHP_EOL;
+            "> hello: echo Hello " .$this->getCmd("'World'").PHP_EOL;
 
         $this->assertEquals($expected, $io->getOutput());
     }

tests/Composer/Test/TestCase.php

@@ -16,6 +16,7 @@
 use Composer\Package\AliasPackage;
 use Composer\Semver\Constraint\Constraint;
 use Composer\Util\Filesystem;
+use Composer\Util\Platform;
 use Composer\Util\Silencer;
 use PHPUnit\Framework\TestCase as BaseTestCase;
 use Symfony\Component\Process\ExecutableFinder;
@@ -125,4 +126,24 @@ public function setExpectedException($exception, $message = null, $code = null)
             parent::setExpectedException($exception, $message, $code);
         }
     }
+
+    /**
+     * Transforms an escaped non-Windows command to match Windows escaping.
+     *
+     * @param string $cmd
+     *
+     * @return string The transformed command
+     */
+    protected function getCmd($cmd)
+    {
+        if (Platform::isWindows()) {
+            $cmd = preg_replace_callback("/('[^']*')/", function ($m) {
+                // Double-quotes are used only when needed
+                $char = (strpbrk($m[1], " \t^&|<>()") !== false || $m[1] === "''") ? '"' : '';
+                return str_replace("'", $char, $m[1]);
+            }, $cmd);
+        }
+
+        return $cmd;
+    }
 }

tests/Composer/Test/Util/ProcessExecutorTest.php

@@ -113,4 +113,90 @@ public function testConsoleIODoesNotFormatSymfonyConsoleStyle()
         $process->execute('php -r "echo \'<error>foo</error>\'.PHP_EOL;"');
         $this->assertSame('<error>foo</error>'.PHP_EOL, $output->fetch());
     }
+
+    /**
+     * Test various arguments are escaped as expected
+     *
+     * @dataProvider dataEscapeArguments
+     */
+    public function testEscapeArgument($argument, $win, $unix)
+    {
+        $expected = defined('PHP_WINDOWS_VERSION_BUILD') ? $win : $unix;
+        $this->assertSame($expected, ProcessExecutor::escape($argument));
+    }
+
+    /**
+     * Each named test is an array of:
+     *   argument, win-expected, unix-expected
+     */
+    public function dataEscapeArguments()
+    {
+        return array(
+            // empty argument - must be quoted
+            'empty'         => array('', '""', "''"),
+
+            // null argument - must be quoted
+            'empty null'    => array(null, '""', "''"),
+
+            // false argument - must be quoted
+            'empty false'    => array(false, '""', "''"),
+
+            // unix single-quote must be escaped
+            'unix-sq'       => array("a'bc", "a'bc", "'a'\\''bc'"),
+
+            // new lines must be replaced
+            'new lines'     => array("a\nb\nc", '"a b c"', "'a\nb\nc'"),
+
+            // whitespace <space> must be quoted
+            'ws space'      => array('a b c', '"a b c"', "'a b c'"),
+
+            // whitespace <tab> must be quoted
+            'ws tab'        => array("a\tb\tc", "\"a\tb\tc\"", "'a\tb\tc'"),
+
+            // no whitespace must not be quoted
+            'no-ws'         => array('abc', 'abc', "'abc'"),
+
+            // double-quotes must be backslash-escaped
+            'dq'            => array('a"bc', 'a\^"bc', "'a\"bc'"),
+
+            // double-quotes must be backslash-escaped with preceeding backslashes doubled
+            'dq-bslash'     => array('a\\"bc', 'a\\\\\^"bc', "'a\\\"bc'"),
+
+            // backslashes not preceeding a double-quote are treated as literal
+            'bslash'        => array('ab\\\\c\\', 'ab\\\\c\\', "'ab\\\\c\\'"),
+
+            // trailing backslashes must be doubled up when the argument is quoted
+            'bslash dq'     => array('a b c\\\\', '"a b c\\\\\\\\"', "'a b c\\\\'"),
+
+            // meta: outer double-quotes must be caret-escaped as well
+            'meta dq'       => array('a "b" c', '^"a \^"b\^" c^"', "'a \"b\" c'"),
+
+            // meta: percent expansion must be caret-escaped
+            'meta-pc1'      => array('%path%', '^%path^%', "'%path%'"),
+
+            // meta: expansion must have two percent characters
+            'meta-pc2'      => array('%path', '%path', "'%path'"),
+
+            // meta: expansion must have have two surrounding percent characters
+            'meta-pc3'      => array('%%path', '%%path', "'%%path'"),
+
+            // meta: bang expansion must be double caret-escaped
+            'meta-bang1'    => array('!path!', '^^!path^^!', "'!path!'"),
+
+            // meta: bang expansion must have two bang characters
+            'meta-bang2'    => array('!path', '!path', "'!path'"),
+
+            // meta: bang expansion must have two surrounding ang characters
+            'meta-bang3'    => array('!!path', '!!path', "'!!path'"),
+
+            // meta: caret-escaping must escape all other meta chars (triggered by double-quote)
+            'meta-all-dq'   => array('<>"&|()^', '^<^>\^"^&^|^(^)^^', "'<>\"&|()^'"),
+
+            // other meta: no caret-escaping when whitespace in argument
+            'other meta'    => array('<> &| ()^', '"<> &| ()^"', "'<> &| ()^'"),
+
+            // other meta: quote escape chars when no whitespace in argument
+            'other-meta'    => array('<>&|()^', '"<>&|()^"', "'<>&|()^'"),
+        );
+    }
 }

tests/Composer/Test/Util/SvnTest.php

@@ -14,7 +14,6 @@
 
 use Composer\Config;
 use Composer\IO\NullIO;
-use Composer\Util\Platform;
 use Composer\Util\Svn;
 use Composer\Test\TestCase;
 
@@ -47,7 +46,7 @@ public function urlProvider()
         return array(
             array('http://till:test@svn.example.org/', $this->getCmd(" --username 'till' --password 'test' ")),
             array('http://svn.apache.org/', ''),
-            array('svn://johndoe@example.org', $this->getCmd(" --username 'johndoe' --password \"\" ")),
+            array('svn://johndoe@example.org', $this->getCmd(" --username 'johndoe' --password '' ")),
         );
     }
 
@@ -130,9 +129,4 @@ public function testCredentialsFromConfigWithCacheCredentialsFalse()
 
         $this->assertEquals($this->getCmd(" --no-auth-cache --username 'foo' --password 'bar' "), $reflMethod->invoke($svn));
     }
-
-    private function getCmd($cmd)
-    {
-        return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd;
-    }
 }