diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index 665b0e7b3..8248aee9a 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/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; use Symfony\Component\Process\Exception\RuntimeException; @@ -455,7 +456,15 @@ class ProcessExecutor } /** - * 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 * * @param ?string $argument * @@ -463,30 +472,34 @@ class ProcessExecutor */ 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); + } - /** - * @param string $arg - * @param string $char - * @return bool - */ - private static function isSurroundedBy($arg, $char) - { - return 2 < strlen($arg) && $char === $arg[0] && $char === $arg[strlen($arg) - 1]; + return $argument; } } diff --git a/tests/Composer/Test/Downloader/FossilDownloaderTest.php b/tests/Composer/Test/Downloader/FossilDownloaderTest.php index 69a810977..41611a459 100644 --- a/tests/Composer/Test/Downloader/FossilDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FossilDownloaderTest.php @@ -15,7 +15,6 @@ namespace Composer\Test\Downloader; use Composer\Downloader\FossilDownloader; use Composer\Test\TestCase; use Composer\Util\Filesystem; -use Composer\Util\Platform; use Composer\Test\Mock\ProcessExecutorMock; class FossilDownloaderTest extends TestCase @@ -165,9 +164,4 @@ class FossilDownloaderTest extends TestCase $this->assertEquals('source', $downloader->getInstallationSource()); } - - private function getCmd($cmd) - { - return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd; - } } diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 248ee8074..6a63c5d7f 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -645,7 +645,7 @@ composer https://github.com/old/url (push) $cmd = str_replace('cd ', 'cd /D ', $cmd); $cmd = str_replace('composerPath', getcwd().'/composerPath', $cmd); - return strtr($cmd, "'", '"'); + return $this->getCmd($cmd); } return $cmd; diff --git a/tests/Composer/Test/Downloader/HgDownloaderTest.php b/tests/Composer/Test/Downloader/HgDownloaderTest.php index e31262b1d..19b5b723c 100644 --- a/tests/Composer/Test/Downloader/HgDownloaderTest.php +++ b/tests/Composer/Test/Downloader/HgDownloaderTest.php @@ -15,7 +15,6 @@ namespace Composer\Test\Downloader; use Composer\Downloader\HgDownloader; use Composer\Test\TestCase; use Composer\Util\Filesystem; -use Composer\Util\Platform; use Composer\Test\Mock\ProcessExecutorMock; class HgDownloaderTest extends TestCase @@ -157,9 +156,4 @@ class HgDownloaderTest extends TestCase $this->assertEquals('source', $downloader->getInstallationSource()); } - - private function getCmd($cmd) - { - return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd; - } } diff --git a/tests/Composer/Test/EventDispatcher/EventDispatcherTest.php b/tests/Composer/Test/EventDispatcher/EventDispatcherTest.php index 400f4dec9..b6ef9a5d7 100644 --- a/tests/Composer/Test/EventDispatcher/EventDispatcherTest.php +++ b/tests/Composer/Test/EventDispatcher/EventDispatcherTest.php @@ -426,7 +426,7 @@ class EventDispatcherTest extends TestCase $dispatcher->dispatch('helloWorld', new ScriptEvent('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()); diff --git a/tests/Composer/Test/TestCase.php b/tests/Composer/Test/TestCase.php index 157d9dbdc..00a01a838 100644 --- a/tests/Composer/Test/TestCase.php +++ b/tests/Composer/Test/TestCase.php @@ -17,6 +17,7 @@ use Composer\Package\RootPackageInterface; use Composer\Package\PackageInterface; use Composer\Semver\Constraint\Constraint; use Composer\Util\Filesystem; +use Composer\Util\Platform; use Composer\Util\Silencer; use Symfony\Component\Process\ExecutableFinder; use Composer\Package\Loader\ArrayLoader; @@ -154,4 +155,24 @@ abstract class TestCase extends PolyfillTestCase 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; + } } diff --git a/tests/Composer/Test/Util/ProcessExecutorTest.php b/tests/Composer/Test/Util/ProcessExecutorTest.php index ca2a8112b..166fd4174 100644 --- a/tests/Composer/Test/Util/ProcessExecutorTest.php +++ b/tests/Composer/Test/Util/ProcessExecutorTest.php @@ -128,4 +128,90 @@ class ProcessExecutorTest extends TestCase $end = microtime(true); $this->assertTrue($end - $start < 0.5, 'Canceling took longer than it should, lasted '.($end - $start)); } + + /** + * 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 must be quoted + 'ws space' => array('a b c', '"a b c"', "'a b c'"), + + // whitespace 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('<>&|()^', '"<>&|()^"', "'<>&|()^'"), + ); + } } diff --git a/tests/Composer/Test/Util/SvnTest.php b/tests/Composer/Test/Util/SvnTest.php index 8e1db31ec..8d38c567b 100644 --- a/tests/Composer/Test/Util/SvnTest.php +++ b/tests/Composer/Test/Util/SvnTest.php @@ -14,7 +14,6 @@ namespace Composer\Test\Util; use Composer\Config; use Composer\IO\NullIO; -use Composer\Util\Platform; use Composer\Util\Svn; use Composer\Test\TestCase; @@ -47,7 +46,7 @@ class SvnTest extends TestCase 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 @@ class SvnTest extends TestCase $this->assertEquals($this->getCmd(" --no-auth-cache --username 'foo' --password 'bar' "), $reflMethod->invoke($svn)); } - - private function getCmd($cmd) - { - return Platform::isWindows() ? strtr($cmd, "'", '"') : $cmd; - } }