bug #25408 [Debug] Fix catching fatal errors in case of nested error …

…handlers (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[Debug] Fix catching fatal errors in case of nested error handlers

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8703, #16980
| License       | MIT
| Doc PR        | -

Fixing a bug from 2013 :)

Commits
-------

27dc9a6 [Debug] Fix catching fatal errors in case of nested error handlers

src/Symfony/Component/Debug/ErrorHandler.php

@@ -485,6 +485,7 @@ public function handleException($exception, array $error = null)
             $exception = new FatalThrowableError($exception);
         }
         $type = $exception instanceof FatalErrorException ? $exception->getSeverity() : E_ERROR;
+        $handlerException = null;
 
         if (($this->loggedErrors & $type) || $exception instanceof FatalThrowableError) {
             $e = array(
@@ -529,18 +530,20 @@ public function handleException($exception, array $error = null)
                 }
             }
         }
-        if (empty($this->exceptionHandler)) {
-            throw $exception; // Give back $exception to the native handler
-        }
         try {
-            call_user_func($this->exceptionHandler, $exception);
+            if (null !== $this->exceptionHandler) {
+                return \call_user_func($this->exceptionHandler, $exception);
+            }
+            $handlerException = $handlerException ?: $exception;
         } catch (\Exception $handlerException) {
         } catch (\Throwable $handlerException) {
         }
-        if (isset($handlerException)) {
-            $this->exceptionHandler = null;
-            $this->handleException($handlerException);
+        $this->exceptionHandler = null;
+        if ($exception === $handlerException) {
+            self::$reservedMemory = null; // Disable the fatal error handler
+            throw $exception; // Give back $exception to the native handler
         }
+        $this->handleException($handlerException);
     }
 
     /**
@@ -556,15 +559,30 @@ public static function handleFatalError(array $error = null)
             return;
         }
 
-        self::$reservedMemory = null;
+        $handler = self::$reservedMemory = null;
+        $handlers = array();
 
-        $handler = set_error_handler('var_dump');
-        $handler = is_array($handler) ? $handler[0] : null;
-        restore_error_handler();
+        while (!is_array($handler) || !$handler[0] instanceof self) {
+            $handler = set_exception_handler('var_dump');
+            restore_exception_handler();
 
-        if (!$handler instanceof self) {
+            if (!$handler) {
+                break;
+            }
+            restore_exception_handler();
+            array_unshift($handlers, $handler);
+        }
+        foreach ($handlers as $h) {
+            set_exception_handler($h);
+        }
+        if (!$handler) {
             return;
         }
+        if ($handler !== $h) {
+            $handler[0]->setExceptionHandler($h);
+        }
+        $handler = $handler[0];
+        $handlers = array();
 
         if ($exit = null === $error) {
             $error = error_get_last();

src/Symfony/Component/Debug/Tests/phpt/exception_rethrown.phpt

@@ -0,0 +1,36 @@
+--TEST--
+Test rethrowing in custom exception handler
+--FILE--
+<?php
+
+namespace Symfony\Component\Debug;
+
+$vendor = __DIR__;
+while (!file_exists($vendor.'/vendor')) {
+    $vendor = dirname($vendor);
+}
+require $vendor.'/vendor/autoload.php';
+
+if (true) {
+    class TestLogger extends \Psr\Log\AbstractLogger
+    {
+        public function log($level, $message, array $context = array())
+        {
+            echo $message, "\n";
+        }
+    }
+}
+
+set_exception_handler(function ($e) { echo 123; throw $e; });
+ErrorHandler::register()->setDefaultLogger(new TestLogger());
+ini_set('display_errors', 1);
+
+throw new \Exception('foo');
+
+?>
+--EXPECTF--
+Uncaught Exception: foo
+123
+Fatal error: Uncaught %s:25
+Stack trace:
+%a

src/Symfony/Component/Debug/Tests/phpt/fatal_with_nested_handlers.phpt

@@ -0,0 +1,40 @@
+--TEST--
+Test catching fatal errors when handlers are nested
+--FILE--
+<?php
+
+namespace Symfony\Component\Debug;
+
+$vendor = __DIR__;
+while (!file_exists($vendor.'/vendor')) {
+    $vendor = dirname($vendor);
+}
+require $vendor.'/vendor/autoload.php';
+
+Debug::enable();
+ini_set('display_errors', 0);
+
+$eHandler = set_error_handler('var_dump');
+$xHandler = set_exception_handler('var_dump');
+
+var_dump(array(
+    $eHandler[0] === $xHandler[0] ? 'Error and exception handlers do match' : 'Error and exception handlers are different',
+));
+
+$eHandler[0]->setExceptionHandler('print_r');
+
+if (true) {
+    class Broken implements \Serializable {};
+}
+
+?>
+--EXPECTF--
+array(1) {
+  [0]=>
+  string(37) "Error and exception handlers do match"
+}
+object(Symfony\Component\Debug\Exception\FatalErrorException)#4 (8) {
+  ["message":protected]=>
+  string(199) "Error: Class Symfony\Component\Debug\Broken contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (Serializable::serialize, Serializable::unserialize)"
+%a
+}