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

Open
symfonyaml opened this issue Feb 7, 2024 · 1 comment · May be fixed by #54576
Open

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

symfonyaml opened this issue Feb 7, 2024 · 1 comment · May be 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
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