Skip to content

[Console] Fix input validation when required arguments are missing #15533

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

Merged
merged 2 commits into from
Sep 27, 2015
Merged

[Console] Fix input validation when required arguments are missing #15533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f12a4c1
[Console] Fix input validation when required arguments are missing
jakzal Aug 13, 2015
4982b02
[Console] Add the command name to input arguments if it's missing
jakzal Sep 15, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/Symfony/Component/Console/Command/Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,13 @@ public function run(InputInterface $input, OutputInterface $output)
$this->interact($input, $output);
}

// The command name argument is often omitted when a command is executed directly with its run() method.
// It would fail the validation if we didn't make sure the command argument is present,
// since it's required by the application.
if ($input->hasArgument('command') && null === $input->getArgument('command')) {
$input->setArgument('command', $this->getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain when this happens? I think it deserves a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion. I'll add:

        // The command name argument is often omitted when a command is executed directly with its run() method.
        // It will fail the validation since the command argument is required by the application.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing this comment gave me another idea: is the command argument really a required argument? What if we made it optional instead of setting it here?

}

$input->validate();

if ($this->code) {
Expand Down
11 changes: 9 additions & 2 deletions src/Symfony/Component/Console/Input/Input.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,15 @@ abstract protected function parse();
*/
public function validate()
{
if (count($this->arguments) < $this->definition->getArgumentRequiredCount()) {
throw new \RuntimeException('Not enough arguments.');
$definition = $this->definition;
$givenArguments = $this->arguments;

$missingArguments = array_filter(array_keys($definition->getArguments()), function ($argument) use ($definition, $givenArguments) {
return !array_key_exists($argument, $givenArguments) && $definition->getArgument($argument)->isRequired();
});

if (count($missingArguments) > 0) {
throw new \RuntimeException(sprintf('Not enough arguments (missing: "%s").', implode(', ', $missingArguments)));
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Console/Tests/Command/CommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,15 @@ public function testRunReturnsIntegerExitCode()
$this->assertSame(2, $exitCode, '->run() returns integer exit code (casts numeric to int)');
}

public function testRunWithApplication()
{
$command = new \TestCommand();
$command->setApplication(new Application());
$exitCode = $command->run(new StringInput(''), new NullOutput());

$this->assertSame(0, $exitCode, '->run() returns an integer exit code');
}

public function testRunReturnsAlwaysInteger()
{
$command = new \TestCommand();
Expand Down
13 changes: 12 additions & 1 deletion src/Symfony/Component/Console/Tests/Input/InputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function testGetInvalidArgument()

/**
* @expectedException \RuntimeException
* @expectedExceptionMessage Not enough arguments.
* @expectedExceptionMessage Not enough arguments (missing: "name").
*/
public function testValidateWithMissingArguments()
{
Expand All @@ -103,6 +103,17 @@ public function testValidateWithMissingArguments()
$input->validate();
}

/**
* @expectedException \RuntimeException
* @expectedExceptionMessage Not enough arguments (missing: "name").
*/
public function testValidateWithMissingRequiredArguments()
{
$input = new ArrayInput(array('bar' => 'baz'));
$input->bind(new InputDefinition(array(new InputArgument('name', InputArgument::REQUIRED), new InputArgument('bar', InputArgument::OPTIONAL))));
$input->validate();
}

public function testValidate()
{
$input = new ArrayInput(array('name' => 'foo'));
Expand Down