Skip to content

Commit

Permalink
[Console] Fix exit status on uncaught exception with negative code
Browse files Browse the repository at this point in the history
As described in #45850, if an application threw an exception with
the `code` property set to a negative number this could in some
cases cause the process to appear to exit 'successfully' with a
zero exit status.

Exiting with any negative number produces potentially unexpected
results - the reported exit status will not appear to match the
value that was set. This is due to the binary handling /
truncation of exit codes.

This may theoretically break BC for applications that were
intentionally throwing exceptions with negative codes and
performing some action based on that status. However, given they
would have had to implement an algorithm to map e.g. `-10` in
PHP to `246` as the actual exit status, it seems unlikely this
is a common usage. It is certainly outside the defined behaviour
of POSIX exit codes.

Therefore I believe it is essentially safe to assume that exceptions
with negative codes are e.g. being thrown by lower-level components,
and are not intended to set a shell exit status.

Coalescing all negative numbers to 1 matches the existing behaviour
with other 'invalid' exception codes e.g. empty / zero / non-numeric.

This therefore feels the most robust fix and eliminates any potential
for confusion.

Fixes #45850
  • Loading branch information
acoulton committed Mar 26, 2022
1 parent 74236fc commit bdcc66f
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Application.php
Expand Up @@ -157,7 +157,7 @@ public function run(InputInterface $input = null, OutputInterface $output = null
$exitCode = $e->getCode();
if (is_numeric($exitCode)) {
$exitCode = (int) $exitCode;
if (0 === $exitCode) {
if ($exitCode <= 0) {
$exitCode = 1;
}
} else {
Expand Down
19 changes: 19 additions & 0 deletions Tests/ApplicationTest.php
Expand Up @@ -1175,6 +1175,25 @@ public function testRunDispatchesExitCodeOneForExceptionCodeZero()
$this->assertTrue($passedRightValue, '-> exit code 1 was passed in the console.terminate event');
}

/**
* @testWith [-1]
* [-32000]
*/
public function testRunReturnsExitCodeOneForNegativeExceptionCode($exceptionCode)
{
$exception = new \Exception('', $exceptionCode);

$application = $this->getMockBuilder(Application::class)->setMethods(['doRun'])->getMock();
$application->setAutoExit(false);
$application->expects($this->once())
->method('doRun')
->willThrowException($exception);

$exitCode = $application->run(new ArrayInput([]), new NullOutput());

$this->assertSame(1, $exitCode, '->run() returns exit code 1 when exception code is '.$exceptionCode);
}

public function testAddingOptionWithDuplicateShortcut()
{
$this->expectException(\LogicException::class);
Expand Down

0 comments on commit bdcc66f

Please sign in to comment.