8000 Process has inconsistent escaping behavior array/string · Issue #27540 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Process has inconsistent escaping behavior array/string #27540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
rtek opened this issue Jun 7, 2018 · 2 comments
Closed

Process has inconsistent escaping behavior array/string #27540

rtek opened this issue Jun 7, 2018 · 2 comments
Labels
Bug Process Status: Needs Review

Comments

@rtek
Copy link
Contributor
rtek commented Jun 7, 2018

Symfony version(s) affected: symfony/process 4.1.0

Description
May not be a bug but the docs imply that the string form for Process command is the equivalent to the array form:

When a string command is passed to new Process() the arguments are not escaped, but when the array form is used they are correctly escaped. This example is on windows so ^ is an escape char.

As a result the string form of the same command is not portable whereas the array form is.

<?php

namespace Test;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Process\Process;

require 'vendor/autoload.php';

class Test extends TestCase
{
    public function testStringsNotEscaped()
    {
        $px = new Process('composer require ^7.1');
        $py = new Process(['composer', 'require', '^7.1']);

        $this->assertSame($px->getCommandLine(), $py->getCommandLine());
    }
}
$ vendor/bin/phpunit Test.php
PHPUnit 7.2.4 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 46 ms, Memory: 2.00MB

There was 1 failure:

1) Test\Test::testStringsNotEscaped
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'composer require ^7.1'
+'composer require ""^^"7.1"'

C:\server\root\oss\process\Test.php:17

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.
@stof
Copy link
Member
stof commented Jun 7, 2018

that's a feature, not a bug.

if you pass a single string, that command is executed as is. Any escaping has to be done on your own when building the command.

When passing an array, Symfony escape each argument for your (as it can know what the different parts of the command are as they are passed separately). and on Windows, ^ is a special char, so some escaping it needed (try running composer require ^7.1 in a cmd shell, and you will see that it will require 7.1, not ^7.1 as the ^ would be swallowed by cmd itself).

That's precisely the reason why the array syntax was added in 3.4 (older versions could rely on the ProcessBuilder to perform some escaping, but that was not working entirely fine, as it was performing this escaping too early to be able to handle 100% of cases properly).

If the doc implies that the 2 forms are equivalent, that's a documentation issue, and should be reported at https://github.com/symfony/symfony-docs

@rtek
Copy link
Contributor Author
rtek commented Jun 7, 2018

Thanks for the clarification.

@rtek rtek closed this as completed Jun 7, 2018
@rtek rtek mentioned this issue Jun 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Process Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stof @xabbuh @rtek @carsonbot
0