Console application issue when command loader name key is not part of command object #38015

Xerkus · 2020-08-31T16:51:00Z

Symfony version(s) affected: 5.1.4

Description
Console Application::get() method could return null with undefined index notice when command loader is used but command name key does not match name or aliases from command object.

Application::find() is affected too because of

symfony/src/Symfony/Component/Console/Application.php

Lines 645 to 647 in d6468a9

    
           if ($this->has($name)) { 
        
               return $this->get($name); 
        
           }

How to reproduce

use Symfony\Component\Console\Application;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\CommandLoader\FactoryCommandLoader;

$app = new Application();
$loader = new FactoryCommandLoader([
    'test' => static fn() => new Command('test-command'),
]);

$app->setCommandLoader($loader);

var_export($app->has('test'));
var_export($app->get('test'));

Possible Solution
After command is obtained from command loader in Application::has(), assert that command is actually present in $this->commands.
I believe proper approach would be to raise an error when this occurs.

Ideally, application would transparently decorate command loader and decorator will then handle assertions that command object has matching name or alias:

CommandLoaderDecorator implements CommandLoaderInterface
{
...
    public function get(string $name) : Command
    {
        $command = $this->commandLoader->get($name);
        self::assertNamePresent($command, $name);
        return $command;
    }
}

Additional context
Command is loaded in has()

symfony/src/Symfony/Component/Console/Application.php

Lines 552 to 556 in d6468a9

    
           public function has(string $name) 
        
           { 
        
               $this->init(); 
        
               return isset($this->commands[$name]) || ($this->commandLoader && $this->commandLoader->has($name) && $this->add($this->commandLoader->get($name)));

The text was updated successfully, but these errors were encountered:

…e definition (chalasr) This PR was merged into the 3.4 branch. Discussion ---------- [Console] Fix undefined index for inconsistent command name definition | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fixes #38015 | License | MIT | Doc PR | - The issue happens when the command name is set via construct/setName() and is routed via a command loader under a different name, which causes `Application::get(): Command` to return null (return type violation) with a notice. This makes it throws a proper CommandNotFoundException as expected. Commits ------- d59140e Fix undefined index for inconsistent command name definition

Xerkus added the Bug label Aug 31, 2020

carsonbot added the Status: Needs Review label Aug 31, 2020

yceruto added the Console label Aug 31, 2020

symfony locked and limited conversation to collaborators Sep 1, 2020

symfony unlocked this conversation Sep 1, 2020

chalasr mentioned this issue Sep 1, 2020

[Console] Fix undefined index for inconsistent command name definition #38024

Merged

fabpot closed this as completed Sep 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Console application issue when command loader name key is not part of command object #38015

Console application issue when command loader name key is not part of command object #38015

Uh oh!

Console application issue when command loader name key is not part of command object #38015

Console application issue when command loader name key is not part of command object #38015

Comments