Skip to content

[Console] Fix infinite loop on missing input #20377

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions src/Symfony/Component/Console/Exception/RuntimeException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Exception;

/**
* @author Jérôme Tamarelle <jerome@tamarelle.net>
*/
class RuntimeException extends \RuntimeException
{
}
9 changes: 6 additions & 3 deletions src/Symfony/Component/Console/Helper/QuestionHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\Console\Formatter\OutputFormatterStyle;
use Symfony\Component\Console\Question\Question;
use Symfony\Component\Console\Question\ChoiceQuestion;
use Symfony\Component\Console\Exception\RuntimeException;

/**
* The QuestionHelper class provides helpers to interact with the user.
Expand Down Expand Up @@ -134,7 +135,7 @@ public function doAsk(OutputInterface $output, Question $question)
if (false === $ret) {
$ret = fgets($inputStream, 4096);
if (false === $ret) {
throw new \RuntimeException('Aborted');
throw new RuntimeException('Aborted');
Copy link
Member

Choose a reason for hiding this comment

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

I know that this exception class does not exist in 2.7, but the bug is there as well, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is. Would you catch any \RuntimeException in 2.7?

Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't it doable to backport this exception class in 2.7?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, I think that would be best.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure it is the right way to proceed but I added a simple extension of \RuntimeException which is used where it's needed, and rebased this.

}
$ret = trim($ret);
}
Expand Down Expand Up @@ -354,7 +355,7 @@ private function getHiddenResponse(OutputInterface $output, $inputStream)
shell_exec(sprintf('stty %s', $sttyMode));

if (false === $value) {
throw new \RuntimeException('Aborted');
throw new RuntimeException('Aborted');
}

$value = trim($value);
Expand All @@ -372,7 +373,7 @@ private function getHiddenResponse(OutputInterface $output, $inputStream)
return $value;
}

throw new \RuntimeException('Unable to hide the response.');
throw new RuntimeException('Unable to hide the response.');
}

/**
Expand All @@ -397,6 +398,8 @@ private function validateAttempts($interviewer, OutputInterface $output, Questio

try {
return call_user_func($question->getValidator(), $interviewer());
} catch (RuntimeException $e) {
throw $e;
} catch (\Exception $error) {
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,37 @@ public function testChoiceOutputFormattingQuestionForUtf8Keys()
$dialog->ask($this->createInputInterfaceMock(), $output, $question);
}

/**
* @expectedException \Symfony\Component\Console\Exception\RuntimeException
* @expectedExceptionMessage Aborted
*/
public function testAskThrowsExceptionOnMissingInput()
{
$dialog = new QuestionHelper();
$dialog->setInputStream($this->getInputStream(''));

$dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), new Question('What\'s your name?'));
}

/**
* @expectedException \Symfony\Component\Console\Exception\RuntimeException
* @expectedExceptionMessage Aborted
*/
public function testAskThrowsExceptionOnMissingInputWithValidator()
{
$dialog = new QuestionHelper();
$dialog->setInputStream($this->getInputStream(''));
Copy link
Member Author

@chalasr chalasr Nov 1, 2016

Choose a reason for hiding this comment

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

Be careful when merging, this method has been deprecated so we need to remove this line and change $this->createInputInterfaceMock() by $this->createStreamableInputInterfaceMock($this->getInputStream('')) below. Same for the test added in SymfonyQuestionHelperTest.


$question = new Question('What\'s your name?');
$question->setValidator(function () {
if (!$value) {
throw new \Exception('A value is required.');
}
});

$dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), $question);
}

protected function getInputStream($input)
{
$stream = fopen('php://memory', 'r+', false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,18 @@ public function testAskEscapeLabel()
$this->assertOutputContains('Do you want a \?', $output);
}

/**
* @expectedException \Symfony\Component\Console\Exception\RuntimeException
* @expectedExceptionMessage Aborted
*/
public function testAskThrowsExceptionOnMissingInput()
{
$dialog = new SymfonyQuestionHelper();

$dialog->setInputStream($this->getInputStream(''));
$dialog->ask($this->createInputInterfaceMock(), $this->createOutputInterface(), new Question('What\'s your name?'));
}

protected function getInputStream($input)
{
$stream = fopen('php://memory', 'r+', false);
Expand Down