Skip to content
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.

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

Console\Input\StringInput doesn't handle escaped single quotes correctly #32182

Closed
bohwaz opened this issue Jun 26, 2019 · 9 comments
Closed

Comments

@bohwaz
Copy link

bohwaz commented Jun 26, 2019

Symfony version(s) affected: 2.8-4.3

Description

Symfony\Component\Console\Input\StringInput doesn't work with arguments escaped by escapeshellarg.

escapeshellarg on a string with single quotes will produce a closing single quote, an escaped single quote and an opening single quote:

$arg = 'Jenny\'s';
print($arg . PHP_EOL);
// Jenny's
print(escapeshellarg($arg) . PHP_EOL);
// Jenny'\''s

This is correct as per bash documentation: https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#Single-Quotes

A single quote may not occur between single quotes, even when preceded by a backslash.

But this breaks StringInput as it breaks up the string in multiple tokens.

How to reproduce

$arg = escapeshellarg('Jenny\'s');
$cmd = 'command --arg=' . $arg;

var_dump($cmd);
var_dump(StringInput::tokenize($cmd));

Output:

string(26) "command --arg='Jenny'\''s'"
string(26) "command --arg='Jenny'\''s'"
array(4) {
  [0]=>
  string(7) "command"
  [1]=>
  string(11) "--arg=Jenny"
  [2]=>
  string(1) "\"
  [3]=>
  string(1) "s"
}

Possible Solution

StringInput should handle escaped characters outside of single quotes part of the string.

@nicolas-grekas
Copy link
Member

That's not StringInput's job: the shell should have already parsed the string so that PHP/StringInput should be given the already resolved string. What you describe is not what StringInput::tokenize() is for.

@bohwaz
Copy link
Author

bohwaz commented Jun 26, 2019

That's not what StringInputTest suggests?

    public function testToString()
    {
        $input = new StringInput('-f foo');
        $this->assertEquals('-f foo', (string) $input);

        $input = new StringInput('-f --bar=foo "a b c d"');
        $this->assertEquals('-f --bar=foo '.escapeshellarg('a b c d'), (string) $input);

        $input = new StringInput('-f --bar=foo \'a b c d\' '."'A\nB\\'C'");
        $this->assertEquals('-f --bar=foo '.escapeshellarg('a b c d').' '.escapeshellarg("A\nB'C"), (string) $input);
    }

@Chi-teck
Copy link
Contributor

I've got a similar issue without using escapeshellarg.
echo new StringInput('--foo="bar"'); prints --for=bar instead of --foo="bar".
That causes a problem when you pass encoded JSON string as command line argument.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@bohwaz
Copy link
Author

bohwaz commented Dec 18, 2020

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

Yes, still relevant.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@bohwaz
Copy link
Author

bohwaz commented Jun 20, 2021

Yes, still relevant, stop trying to avoid issues.

@carsonbot carsonbot removed the Stalled label Jun 20, 2021
@wouterj
Copy link
Member

wouterj commented Jun 21, 2021

@bohwaz are you up to fixing the issue?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 20, 2022

@Chi-teck the issue is on your side: you need to escape the json content. Eg echo new StringInput('--foo=\'"bar"\'');

@bohwaz see #45088

@fabpot fabpot closed this as completed Jan 22, 2022
fabpot added a commit that referenced this issue Jan 22, 2022
…s-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Console] fix parsing escaped chars in StringInput

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #32182
| License       | MIT
| Doc PR        | -

Commits
-------

f0a89ec [Console] fix parsing escaped chars in StringInput
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants