[Console] Application can exit 0 following an uncaught exception with specific negative `code` values #45850

acoulton · 2022-03-26T00:01:54Z

Symfony version(s) affected

At least v2+ through to HEAD

Description

A symfony console application can unexpectedly exit with an exit status of 0 if:

a command throws an uncaught exception
the code property of that exception has been populated with a negative integer that is an exact multiple of 256.

Additionally, commands exit with a potentially unexpected status for any other negative code - for example an exception code of -2 produces an exit status of 254. However, these are still nonzero so do not fundamentally change the shell's understanding of whether the command failed. They may just make it harder to understand e.g. logging output unless the developer is familiar with the internal implementation & behaviour of POSIX exit codes.

This happens because in Application::run if $e->getCode() returns any numeric, nonzero, value then it will attempt to use this as an exit status:

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

Lines 157 to 163 in 56b428f

    
           $exitCode = $e->getCode(); 
        
           if (is_numeric($exitCode)) { 
        
               $exitCode = (int) $exitCode; 
        
               if (0 === $exitCode) { 
        
                   $exitCode = 1; 
        
               } 
        
           } else {

Later in that class, the exit code is clamped to a maximum value of 255, but there is no minimum constraint:

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

Lines 182 to 188 in 56b428f

    
           if ($this->autoExit) { 
        
               if ($exitCode > 255) { 
        
                   $exitCode = 255; 
        
               } 
        
               exit($exitCode); 
        
           }

Therefore any negative values will be passed directly to the php exit() function.

In POSIX, only the least significant 8 bits of an exit status are made available when waiting for a child process. Therefore:

decimal exit	full binary	8 LSB as decimal
-1	1111 1111 1111 1111	255
-2	1111 1111 1111 1110	254
...	...	...
-255	1111 1111 0000 0001	1
-256	1111 1111 0000 0000	0
-257	1111 1110 1111 1111	255
...	...	...
512	1111 1110 0000 0000	0
...	...etc ad infinitum...	...

Although PHP error codes are usually positive, it's relatively common in c-based software for them to be negative.

We discovered the problem when trying to identify why our behat runs were occasionally "passing" despite a Chromium error causing the build to crash. It turned out when the driver threw an Exception, it was attaching the error code reported by Chrome, which happened to be -32000 (a multiple of 256). PHP simply documents Exception::$code as an int - it does not specify positive/negative, so it appears this is a valid value as far as the exception is concerned, but not as an exit status.

How to reproduce

Define a simple application:

<?php
// invalid-exit.php

use Symfony\Component\Console\Application;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

require_once __DIR__.'/vendor/autoload.php';
$app = new Application();

class ThrowExceptionCommand extends Command
{
    protected static $defaultName = 'throw-exception';

    protected function configure()
    {
        $this->addOption('exitcode', NULL, InputOption::VALUE_REQUIRED);
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $code = (int) $input->getOption('exitcode');
        throw new \RuntimeException('Quitting with exit code '.$code, $code);
    }

}

$app->add(new ThrowExceptionCommand);

$app->run();

And a simple wrapper script

#!/bin/bash

for throw_code in {1..-300}
do
  php ./invalid-exit.php throw-exception --exitcode=$throw_code >/dev/null 2>&1
  actual_code=$?
  echo "Exception code $throw_code => exited $actual_code"
done

Then run the script. Observe the output will be:

Exception code 1 => exited 1
Exception code 0 => exited 1
Exception code -1 => exited 255
Exception code -2 => exited 254
... continues descending
Exception code -255 => exited 1
Exception code -256 => exited 0
Exception code -257 => exited 255
Exception code -258 => exited 254
... continues descending

Possible Solution

There are two possible solutions.

IMO the best option would be to only use the code from the exception if it is a positive integer. However this may raise some BC concerns - although only for usecases that are likely to be extremely unusual. I will put together a PR based on this.

If that is unacceptable for compatibility then clamping values to -255 in the same way as they are clamped to 255 would solve the success/failure status which is the biggest part of the issue.

Additional Context

No response

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

8000

As described in symfony#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. 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.

As described in symfony#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 symfony#45850

…ive 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

As described in symfony#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 symfony#45850

acoulton added the Bug label Mar 26, 2022

carsonbot added Console Status: Needs Review labels Mar 26, 2022

acoulton mentioned this issue Mar 26, 2022

[Console] Fix exit status on uncaught exception with negative code #45851

Merged

fabpot closed this as completed Mar 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[Console] Application can exit 0 following an uncaught exception with specific negative `code` values #45850

[Console] Application can exit 0 following an uncaught exception with specific negative `code` values #45850

Uh oh!

[Console] Application can exit 0 following an uncaught exception with specific negative code values #45850

[Console] Application can exit 0 following an uncaught exception with specific negative code values #45850

Comments

Symfony version(s) affected

Description

How to reproduce

Possible Solution

Additional Context

[Console] Application can exit 0 following an uncaught exception with specific negative `code` values #45850

[Console] Application can exit 0 following an uncaught exception with specific negative `code` values #45850