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] Misuse of ArgvInput with arrays needs a better error handling #53836

Closed
symfonyaml opened this issue Feb 7, 2024 · 1 comment · Fixed by #54576
Closed

[Console] Misuse of ArgvInput with arrays needs a better error handling #53836

symfonyaml opened this issue Feb 7, 2024 · 1 comment · Fixed by #54576

Comments

@symfonyaml
Copy link
Contributor

symfonyaml commented Feb 7, 2024

Symfony version(s) affected

5.4

Description

Misuse of ArgvInput with array in $argv returns different PHP fatal errors, instead of some DX with a nice exception explaining the situation.

The misuse :

$argv = [
    'script',
    ['array'], // <------ this is the misuse bit
];
$input = ArgvInput($argv);

Calling bind() method :

Uncaught TypeError: Argument 1 passed to ArgvInput::parseToken() must be of the type string, array given

Calling hasParameterOption() or getParameterOption() methods :

Uncaught TypeError: Argument 1 passed to str_starts_with() must be of the type string or null, array given

How to reproduce

  • Create a new directory
    mkdir ./reproduce-error-argv-input-bug
    cd ./reproduce-error-argv-input-bug
    
  • Install the symfony/console repo
    composer require symfony/console
    
  • Create the PHP script test.php to throw the error
    <?php
    require_once __DIR__.'/vendor/autoload.php';
    $argv = [
        'script',
        ['array'], // <------ this is the misuse bit
    ];
    $input = new \Symfony\Component\Console\Input\ArgvInput($argv);
    $inputDefinition = new \Symfony\Component\Console\Input\InputDefinition();
    // comment any of the following method calls to get the errors :
    // error 1 (Argument 1 passed to ArgvInput::parseToken() must be of the type string, array given)
    $input->bind($inputDefinition);
    // error 2 (Argument 1 passed to str_starts_with() must be of the type string or null, array given)
    $input->hasParameterOption(['--ansi'], true);
    $input->getParameterOption(['--ansi']);
  • Run the PHP script
    php test.php
    

Possible Solution

Maybe we should prevent to pass array values in the ArgvInput constructor like :

class ArgvInput extends Input
{
    public function __construct(array $argv = null, InputDefinition $definition = null)
    {
        $argv = $argv ?? $_SERVER['argv'] ?? [];

        if (array_filter($argv, 'is_array')) {
            throw new RuntimeException('Argument values expected to be scalars, got array.');
        }
    }
    // ...

Additional Context

There is a kind of related issue with ArrayInput by @niklaswolf : #52580

@derrabus
Copy link
Member

derrabus commented Mar 4, 2024

Maybe #54149 helps already?

fabpot added a commit that referenced this issue Mar 9, 2024
…abus)

This PR was merged into the 7.1 branch.

Discussion
----------

[Console] Document argv arrays for static analysis

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Part of #53836
| License       | MIT

This PR adds PHPDoc blocks for static analyzers that should make mistakes like the ones described in #53836 more discoverable.

Commits
-------

a84f39a [Console] Document argv arrays for static analysis
@fabpot fabpot closed this as completed in 4f5d618 Jun 3, 2024
symfony-splitter pushed a commit to symfony/console that referenced this issue Jun 3, 2024
… with arrays (symfonyaml)

This PR was squashed before being merged into the 7.2 branch.

Discussion
----------

[Console] Better error handling when misuse of `ArgvInput` with arrays

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #53836
| License       | MIT

### Issue
When we don't use `ArgvInput` correclty, and use array in $argv values, it returns different PHP fatal errors.
See all details and how to reproduce it in the issue symfony/symfony#53836

### Solution
In this PR
 - Add some DX with an exception explaining the problem, to avoid PHP fatal errors
 - Add tests**

_____
Note : Old PR #54147 was targeting 5.4, see [this comment](symfony/symfony#54147 (comment)) for more details

Commits
-------

6f64cf4f80 [Console] Better error handling when misuse of `ArgvInput` with arrays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants