Skip to content

Commit

Permalink
bug #34848 [Process] change the syntax of portable command lines (nic…
Browse files Browse the repository at this point in the history
…olas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] change the syntax of portable command lines

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34838
| License       | MIT
| Doc PR        | symfony/symfony-docs#12772

An alternative to #34845

Right now, portable command lines use `"$FOO"` for placeholders.
But because we validate that a corresponding variable exists before running the command, this fails with `Command line is missing a value for key "$FOO"` when `FOO` is not defined.

This PR proposes to use `"${:FOO}"` instead. The difference with the previous syntax is that this cannot collide with existing shell scripts as it is invalid for them.

When this is merged, we'll have to update https://symfony.com/blog/new-in-symfony-4-1-prepared-commands too.

Commits
-------

3c7b775 [Process] change the syntax of portable prepared command lines
  • Loading branch information
nicolas-grekas committed Dec 10, 2019
2 parents b81f428 + 3c7b775 commit 2e70324
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
6 changes: 3 additions & 3 deletions src/Symfony/Component/Process/Process.php
Expand Up @@ -1643,12 +1643,12 @@ private function escapeArgument(?string $argument): string

private function replacePlaceholders(string $commandline, array $env)
{
return preg_replace_callback('/"\$([_a-zA-Z]++[_a-zA-Z0-9]*+)"/', function ($matches) use ($commandline, $env) {
return preg_replace_callback('/"\$\{:([_a-zA-Z]++[_a-zA-Z0-9]*+)\}"/', function ($matches) use ($commandline, $env) {
if (!isset($env[$matches[1]]) || false === $env[$matches[1]]) {
throw new InvalidArgumentException(sprintf('Command line is missing a value for key %s: %s.', $matches[0], $commandline));
throw new InvalidArgumentException(sprintf('Command line is missing a value for parameter "%s": %s.', $matches[1], $commandline));
}

return '\\' === \DIRECTORY_SEPARATOR ? $this->escapeArgument($env[$matches[1]]) : $matches[0];
return $this->escapeArgument($env[$matches[1]]);
}, $commandline);
}

Expand Down
14 changes: 7 additions & 7 deletions src/Symfony/Component/Process/Tests/ProcessTest.php
Expand Up @@ -1463,23 +1463,23 @@ public function provideEscapeArgument()

public function testPreparedCommand()
{
$p = Process::fromShellCommandline('echo "$abc"DEF');
$p = Process::fromShellCommandline('echo "${:abc}"DEF');
$p->run(null, ['abc' => 'ABC']);

$this->assertSame('ABCDEF', rtrim($p->getOutput()));
}

public function testPreparedCommandMulti()
{
$p = Process::fromShellCommandline('echo "$abc""$def"');
$p = Process::fromShellCommandline('echo "${:abc}""${:def}"');
$p->run(null, ['abc' => 'ABC', 'def' => 'DEF']);

$this->assertSame('ABCDEF', rtrim($p->getOutput()));
}

public function testPreparedCommandWithQuoteInIt()
{
$p = Process::fromShellCommandline('php -r "$code" "$def"');
$p = Process::fromShellCommandline('php -r "${:code}" "${:def}"');
$p->run(null, ['code' => 'echo $argv[1];', 'def' => '"DEF"']);

$this->assertSame('"DEF"', rtrim($p->getOutput()));
Expand All @@ -1488,16 +1488,16 @@ public function testPreparedCommandWithQuoteInIt()
public function testPreparedCommandWithMissingValue()
{
$this->expectException('Symfony\Component\Process\Exception\InvalidArgumentException');
$this->expectExceptionMessage('Command line is missing a value for key "$abc": echo "$abc".');
$p = Process::fromShellCommandline('echo "$abc"');
$this->expectExceptionMessage('Command line is missing a value for parameter "abc": echo "${:abc}".');
$p = Process::fromShellCommandline('echo "${:abc}"');
$p->run(null, ['bcd' => 'BCD']);
}

public function testPreparedCommandWithNoValues()
{
$this->expectException('Symfony\Component\Process\Exception\InvalidArgumentException');
$this->expectExceptionMessage('Command line is missing a value for key "$abc": echo "$abc".');
$p = Process::fromShellCommandline('echo "$abc"');
$this->expectExceptionMessage('Command line is missing a value for parameter "abc": echo "${:abc}".');
$p = Process::fromShellCommandline('echo "${:abc}"');
$p->run(null, []);
}

Expand Down

0 comments on commit 2e70324

Please sign in to comment.