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:

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

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:

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