Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #22551 [Process] Ecaping of CLI arguments containing slashes on W…
…indows (maryo)

This PR was squashed before being merged into the 3.3-dev branch (closes #22551).

Discussion
----------

[Process] Ecaping of CLI arguments containing slashes on Windows

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22549
| License       | MIT

Actually only the first argument - the command needs to be escaped but that would need another condition. I think it should be OK.

Commits
-------

0d07312 [Process] Ecaping of CLI arguments containing slashes on Windows
  • Loading branch information
fabpot committed Apr 29, 2017
2 parents 89979ca + 0d07312 commit 288d55f
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/Process/Process.php
Expand Up @@ -1717,7 +1717,7 @@ private function escapeArgument($argument)
if (false !== strpos($argument, "\0")) {
$argument = str_replace("\0", '?', $argument);
}
if (!preg_match('/[()%!^"<>&|\s]/', $argument)) {
if (!preg_match('/[\/()%!^"<>&|\s]/', $argument)) {
return $argument;
}
$argument = preg_replace('/(\\\\+)$/', '$1$1', $argument);
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
Expand Up @@ -91,14 +91,14 @@ public function testPrefixIsPrependedToAllGeneratedProcess()

$proc = $pb->setArguments(array('-v'))->getProcess();
if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php -v', $proc->getCommandLine());
$this->assertEquals('"/usr/bin/php" -v', $proc->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php' '-v'", $proc->getCommandLine());
}

$proc = $pb->setArguments(array('-i'))->getProcess();
if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php -i', $proc->getCommandLine());
$this->assertEquals('"/usr/bin/php" -i', $proc->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php' '-i'", $proc->getCommandLine());
}
Expand All @@ -111,14 +111,14 @@ public function testArrayPrefixesArePrependedToAllGeneratedProcess()

$proc = $pb->setArguments(array('-v'))->getProcess();
if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php composer.phar -v', $proc->getCommandLine());
$this->assertEquals('"/usr/bin/php" composer.phar -v', $proc->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php' 'composer.phar' '-v'", $proc->getCommandLine());
}

$proc = $pb->setArguments(array('-i'))->getProcess();
if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php composer.phar -i', $proc->getCommandLine());
$this->assertEquals('"/usr/bin/php" composer.phar -i', $proc->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php' 'composer.phar' '-i'", $proc->getCommandLine());
}
Expand Down Expand Up @@ -164,7 +164,7 @@ public function testShouldNotThrowALogicExceptionIfNoArgument()
->getProcess();

if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php', $process->getCommandLine());
$this->assertEquals('"/usr/bin/php"', $process->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php'", $process->getCommandLine());
}
Expand All @@ -176,7 +176,7 @@ public function testShouldNotThrowALogicExceptionIfNoPrefix()
->getProcess();

if ('\\' === DIRECTORY_SEPARATOR) {
$this->assertEquals('/usr/bin/php', $process->getCommandLine());
$this->assertEquals('"/usr/bin/php"', $process->getCommandLine());
} else {
$this->assertEquals("'/usr/bin/php'", $process->getCommandLine());
}
Expand Down
12 changes: 10 additions & 2 deletions src/Symfony/Component/Process/Tests/ProcessTest.php
Expand Up @@ -715,8 +715,8 @@ public function testRestart()
// Ensure that both processed finished and the output is numeric
$this->assertFalse($process1->isRunning());
$this->assertFalse($process2->isRunning());
$this->assertTrue(is_numeric($process1->getOutput()));
$this->assertTrue(is_numeric($process2->getOutput()));
$this->assertInternalType('numeric', $process1->getOutput());
$this->assertInternalType('numeric', $process2->getOutput());

// Ensure that restart returned a new process by check that the output is different
$this->assertNotEquals($process1->getOutput(), $process2->getOutput());
Expand Down Expand Up @@ -1446,6 +1446,14 @@ public function testInheritEnvDisabled()
$this->assertSame($expected, $env);
}

public function testGetCommandLine()
{
$p = new Process(array('/usr/bin/php'));

$expected = '\\' === DIRECTORY_SEPARATOR ? '"/usr/bin/php"' : "'/usr/bin/php'";
$this->assertSame($expected, $p->getCommandLine());
}

/**
* @dataProvider provideEscapeArgument
*/
Expand Down

0 comments on commit 288d55f

Please sign in to comment.