8000 bug #45851 [Console] Fix exit status on uncaught exception with negat… · symfony/symfony@7b43f0f · GitHub
[go: up one dir, main page]

Skip to content

Commit 7b43f0f

Browse files
committed
bug #45851 [Console] Fix exit status on uncaught exception with negative code (acoulton)
This PR was merged into the 4.4 branch. Discussion ---------- [Console] Fix exit status on uncaught exception with negative code | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #45850 | License | MIT | Doc PR | N/A 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 PR 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. IMO therefore applications using negative statuses are relying on an undocumented / incorrect implementation, and this is not a formal BC break. 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. Commits ------- e7072aa [Console] Fix exit status on uncaught exception with negative code
2 parents 56b428f + e7072aa commit 7b43f0f

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed

src/Symfony/Component/Console/Application.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public function run(InputInterface $input = null, OutputInterface $output = null
157157
$exitCode = $e->getCode();
158158
if (is_numeric($exitCode)) {
159159
$exitCode = (int) $exitCode;
160-
if (0 === $exitCode) {
160+
if ($exitCode <= 0) {
161161
$exitCode = 1;
162162
}
163163
} else {

src/Symfony/Component/Console/Tests/ApplicationTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,25 @@ public function testRunDispatchesExitCodeOneForExceptionCodeZero()
11751175
$this->assertTrue($passedRightValue, '-> exit code 1 was passed in the console.terminate event');
11761176
}
11771177

1178+
/**
1179+
* @testWith [-1]
1180+
* [-32000]
1181+
*/
1182+
public function testRunReturnsExitCodeOneForNegativeExceptionCode($exceptionCode)
1183+
{
1184+
$exception = new \Exception('', $exceptionCode);
1185+
1186+
$application = $this->getMockBuilder(Application::class)->setMethods(['doRun'])->getMock();
1187+
$application->setAutoExit(false);
1188+
$application->expects($this->once())
1189+
->method('doRun')
1190+
->willThrowException($exception);
1191+
1192+
$exitCode = $application->run(new ArrayInput([]), new NullOutput());
1193+
1194+
$this->assertSame(1, $exitCode, '->run() returns exit code 1 when exception code is '.$exceptionCode);
1195+
}
1196+
11781197
public function testAddingOptionWithDuplicateShortcut()
11791198
{
11801199
$this->expectException(\LogicException::class);

0 commit comments

Comments
 (0)
0