feature #10201 [Debug] error stacking + fatal screaming + case testing (nicolas-grekas)

This PR was merged into the 2.5-dev branch.

Discussion
----------

[Debug] error stacking + fatal screaming + case testing

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | no
| License       | MIT
| Doc PR        | no

Ported from https://github.com/nicolas-grekas/Patchwork/

Three enhancements for Symfony debug mode:

- detect case mismatches between loaded class name, its declared name and its source file name
- dismiss hard to debug blank pages related to non-catchable-@-silenced fatal errors (```@(new Toto) + parse error in Toto.php``` => enjoy debugging)
- work around https://bugs.php.net/42098 / https://bugs.php.net/54054 / https://bugs.php.net/60149 / https://bugs.php.net/65322 (fixed in 5.5.5)

An other thing I didn't port is scope isolation: the current `require` in autoloaders is done in their scope, so local $vars / $this (with access to private props/methods) is easily accessible from required files. Shouldn't proper separation prevent that?

Commits
-------

6de362b [Debug] error stacking+fatal screaming+case testing

src/Symfony/Component/Debug/DebugClassLoader.php

@@ -14,46 +14,67 @@ namespace Symfony\Component\Debug;
 /**
  * Autoloader checking if the class is really defined in the file found.
  *
- * The ClassLoader will wrap all registered autoloaders providing a
- * findFile method and will throw an exception if a file is found but does
+ * The ClassLoader will wrap all registered autoloaders
+ * and will throw an exception if a file is found but does
  * not declare the class.
  *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Christophe Coevoet <stof@notk.org>
+ * @author Nicolas Grekas <p@tchwork.com>
  *
  * @api
  */
 class DebugClassLoader
 {
-    private $classFinder;
+    private $classLoader;
+    private $isFinder;
+    private $wasFinder;
 
     /**
      * Constructor.
      *
-     * @param object $classFinder
+     * @param callable|object $classLoader
      *
      * @api
+     * @deprecated since 2.5, passing an object is deprecated and support for it will be removed in 3.0
      */
-    public function __construct($classFinder)
+    public function __construct($classLoader)
     {
-        $this->classFinder = $classFinder;
+        $this->wasFinder = is_object($classLoader) && method_exists($classLoader, 'findFile');
+
+        if ($this->wasFinder) {
+            $this->classLoader = array($classLoader, 'loadClass');
+            $this->isFinder = true;
+        } else {
+            $this->classLoader = $classLoader;
+            $this->isFinder = is_array($classLoader) && method_exists($classLoader[0], 'findFile');
+        }
     }
 
     /**
      * Gets the wrapped class loader.
      *
-     * @return object a class loader instance
+     * @return callable|object a class loader
+     *
+     * @deprecated since 2.5, returning an object is deprecated and support for it will be removed in 3.0
      */
     public function getClassLoader()
     {
-        return $this->classFinder;
+        if ($this->wasFinder) {
+            return $this->classLoader[0];
+        } else {
+            return $this->classLoader;
+        }
     }
 
     /**
-     * Replaces all autoloaders implementing a findFile method by a DebugClassLoader wrapper.
+     * Wraps all autoloaders
      */
     public static function enable()
     {
+        // Ensures we don't hit https://bugs.php.net/42098
+        class_exists(__NAMESPACE__.'\ErrorHandler', true);
+
         if (!is_array($functions = spl_autoload_functions())) {
             return;
         }
@@ -63,8 +84,8 @@ class DebugClassLoader
         }
 
         foreach ($functions as $function) {
-            if (is_array($function) && !$function[0] instanceof self && method_exists($function[0], 'findFile')) {
-                $function = array(new static($function[0]), 'loadClass');
+            if (!is_array($function) || !$function[0] instanceof self) {
+                $function = array(new static($function), 'loadClass');
             }
 
             spl_autoload_register($function);
@@ -86,7 +107,7 @@ class DebugClassLoader
 
         foreach ($functions as $function) {
             if (is_array($function) && $function[0] instanceof self) {
-                $function[0] = $function[0]->getClassLoader();
+                $function = $function[0]->getClassLoader();
             }
 
             spl_autoload_register($function);
@@ -99,10 +120,14 @@ class DebugClassLoader
      * @param string $class A class name to resolve to file
      *
      * @return string|null
+     *
+     * @deprecated Deprecated since 2.5, to be removed in 3.0.
      */
     public function findFile($class)
     {
-        return $this->classFinder->findFile($class);
+        if ($this->wasFinder) {
+            return $this->classLoader[0]->findFile($class);
+        }
     }
 
     /**
@@ -116,10 +141,55 @@ class DebugClassLoader
      */
     public function loadClass($class)
     {
-        if ($file = $this->classFinder->findFile($class)) {
-            require $file;
+        ErrorHandler::stackErrors();
 
-            if (!class_exists($class, false) && !interface_exists($class, false) && (!function_exists('trait_exists') || !trait_exists($class, false))) {
+        try {
+            if ($this->isFinder) {
+                if ($file = $this->classLoader[0]->findFile($class)) {
+                    require $file;
+                }
+            } else {
+                call_user_func($this->classLoader, $class);
+                $file = false;
+            }
+        } catch (\Exception $e) {
+            ErrorHandler::unstackErrors();
+
+            throw $e;
+        }
+
+        ErrorHandler::unstackErrors();
+
+        $exists = class_exists($class, false) || interface_exists($class, false) || (function_exists('trait_exists') && trait_exists($class, false));
+
+        if ($exists) {
+            $name = new \ReflectionClass($class);
+            $name = $name->getName();
+
+            if ($name !== $class) {
+                throw new \RuntimeException(sprintf('Case mismatch between loaded and declared class names: %s vs %s', $class, $name));
+            }
+        }
+
+        if ($file) {
+            if ('\\' == $class[0]) {
+                $class = substr($class, 1);
+            }
+
+            $i = -1;
+            $tail = str_replace('\\', DIRECTORY_SEPARATOR, $class).'.php';
+            $len = strlen($tail);
+
+            do {
+                $tail = substr($tail, $i+1);
+                $len -= $i+1;
+
+                if (! substr_compare($file, $tail, -$len, $len, true) && substr_compare($file, $tail, -$len, $len, false)) {
+                    throw new \RuntimeException(sprintf('Case mismatch between class and source file names: %s vs %s', $class, $file));
+                }
+            } while (false !== $i = strpos($tail, '\\'));
+
+            if (! $exists) {
                 if (false !== strpos($class, '/')) {
                     throw new \RuntimeException(sprintf('Trying to autoload a class with an invalid name "%s". Be careful that the namespace separator is "\" in PHP, not "/".', $class));
                 }

src/Symfony/Component/Debug/ErrorHandler.php

@@ -25,6 +25,7 @@ use Symfony\Component\Debug\FatalErrorHandler\FatalErrorHandlerInterface;
  *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Konstantin Myakshin <koc-dp@yandex.ru>
+ * @author Nicolas Grekas <p@tchwork.com>
  */
 class ErrorHandler
 {
@@ -57,6 +58,10 @@ class ErrorHandler
      */
     private static $loggers = array();
 
+    private static $stackedErrors = array();
+
+    private static $stackedErrorLevels = array();
+
     /**
      * Registers the error handler.
      *
@@ -121,45 +126,46 @@ class ErrorHandler
 
         if ($level & (E_USER_DEPRECATED | E_DEPRECATED)) {
             if (isset(self::$loggers['deprecation'])) {
-                if (version_compare(PHP_VERSION, '5.4', '<')) {
-                    $stack = array_map(
-                        function ($row) {
-                            unset($row['args']);
-
-                            return $row;
-                        },
-                        array_slice(debug_backtrace(false), 0, 10)
-                    );
+                if (self::$stackedErrorLevels) {
+                    self::$stackedErrors[] = func_get_args();
                 } else {
-                    $stack = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 10);
-                }
+                    if (version_compare(PHP_VERSION, '5.4', '<')) {
+                        $stack = array_map(
+                            function ($row) {
+                                unset($row['args']);
 
-                self::$loggers['deprecation']->warning($message, array('type' => self::TYPE_DEPRECATION, 'stack' => $stack));
+                                return $row;
+                            },
+                            array_slice(debug_backtrace(false), 0, 10)
+                        );
+                    } else {
+                        $stack = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 10);
+                    }
+
+                    self::$loggers['deprecation']->warning($message, array('type' => self::TYPE_DEPRECATION, 'stack' => $stack));
+                }
             }
 
             return true;
         }
 
         if ($this->displayErrors && error_reporting() & $level && $this->level & $level) {
-            // make sure the ContextErrorException class is loaded (https://bugs.php.net/bug.php?id=65322)
-            if (!class_exists('Symfony\Component\Debug\Exception\ContextErrorException')) {
-                require __DIR__.'/Exception/ContextErrorException.php';
-            }
-
-            $exception = new ContextErrorException(sprintf('%s: %s in %s line %d', isset($this->levels[$level]) ? $this->levels[$level] : $level, $message, $file, $line), 0, $level, $file, $line, $context);
-
             // Exceptions thrown from error handlers are sometimes not caught by the exception
             // handler, so we invoke it directly (https://bugs.php.net/bug.php?id=54275)
-            $exceptionHandler = set_exception_handler(function () {});
+            $exceptionHandler = set_exception_handler('var_dump');
             restore_exception_handler();
 
             if (is_array($exceptionHandler) && $exceptionHandler[0] instanceof ExceptionHandler) {
-                $exceptionHandler[0]->handle($exception);
+                if (self::$stackedErrorLevels) {
+                    self::$stackedErrors[] = func_get_args();
 
-                if (!class_exists('Symfony\Component\Debug\Exception\DummyException')) {
-                    require __DIR__.'/Exception/DummyException.php';
+                    return true;
                 }
 
+                $exception = sprintf('%s: %s in %s line %d', isset($this->levels[$level]) ? $this->levels[$level] : $level, $message, $file, $line);
+                $exception = new ContextErrorException($exception, 0, $level, $file, $line, $context);
+                $exceptionHandler[0]->handle($exception);
+
                 // we must stop the PHP script execution, as the exception has
                 // already been dealt with, so, let's throw an exception that
                 // will be caught by a dummy exception handler
@@ -179,13 +185,61 @@ class ErrorHandler
         return false;
     }
 
+    /**
+     * Configure the error handler for delayed handling.
+     * Ensures also that non-catchable fatal errors are never silenced.
+     *
+     * As shown by http://bugs.php.net/42098 and http://bugs.php.net/60724
+     * PHP has a compile stage where it behaves unusually. To workaround it,
+     * we plug an error handler that only stacks errors for later.
+     *
+     * The most important feature of this is to prevent
+     * autoloading until unstackErrors() is called.
+     */
+    public static function stackErrors()
+    {
+        self::$stackedErrorLevels[] = error_reporting(error_reporting() | E_PARSE | E_ERROR | E_CORE_ERROR | E_COMPILE_ERROR);
+    }
+
+    /**
+     * Unstacks stacked errors and forwards to the regular handler
+     */
+    public static function unstackErrors()
+    {
+        $level = array_pop(self::$stackedErrorLevels);
+
+        if (null !== $level) {
+            error_reporting($level);
+        }
+
+        if (empty(self::$stackedErrorLevels)) {
+            $errors = self::$stackedErrors;
+            self::$stackedErrors = array();
+
+            $errorHandler = set_error_handler('var_dump');
+            restore_error_handler();
+
+            if ($errorHandler) {
+                foreach ($errors as $e) {
+                    call_user_func_array($errorHandler, $e);
+                }
+            }
+        }
+    }
+
     public function handleFatal()
     {
-        if (null === $error = error_get_last()) {
+        $this->reservedMemory = '';
+        $error = error_get_last();
+
+        while (self::$stackedErrorLevels) {
+            static::unstackErrors();
+        }
+
+        if (null === $error) {
             return;
         }
 
-        $this->reservedMemory = '';
         $type = $error['type'];
         if (0 === $this->level || !in_array($type, array(E_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_PARSE))) {
             return;
@@ -206,7 +260,7 @@ class ErrorHandler
         }
 
         // get current exception handler
-        $exceptionHandler = set_exception_handler(function () {});
+        $exceptionHandler = set_exception_handler('var_dump');
         restore_exception_handler();
 
         if (is_array($exceptionHandler) && $exceptionHandler[0] instanceof ExceptionHandler) {

src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php

@@ -12,46 +12,134 @@
 namespace Symfony\Component\Debug\Tests;
 
 use Symfony\Component\Debug\DebugClassLoader;
+use Symfony\Component\Debug\ErrorHandler;
 
 class DebugClassLoaderTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var int Error reporting level before running tests.
+     */
+    private $errorReporting;
+
     private $loader;
 
     protected function setUp()
     {
+        $this->errorReporting = error_reporting(E_ALL | E_STRICT);
         $this->loader = new ClassLoader();
         spl_autoload_register(array($this->loader, 'loadClass'));
+        DebugClassLoader::enable();
     }
 
     protected function tearDown()
     {
+        DebugClassLoader::disable();
         spl_autoload_unregister(array($this->loader, 'loadClass'));
+        error_reporting($this->errorReporting);
     }
 
     public function testIdempotence()
     {
         DebugClassLoader::enable();
-        DebugClassLoader::enable();
 
         $functions = spl_autoload_functions();
         foreach ($functions as $function) {
             if (is_array($function) && $function[0] instanceof DebugClassLoader) {
                 $reflClass = new \ReflectionClass($function[0]);
-                $reflProp = $reflClass->getProperty('classFinder');
+                $reflProp = $reflClass->getProperty('classLoader');
                 $reflProp->setAccessible(true);
 
                 $this->assertNotInstanceOf('Symfony\Component\Debug\DebugClassLoader', $reflProp->getValue($function[0]));
 
-                DebugClassLoader::disable();
-
                 return;
             }
         }
 
-        DebugClassLoader::disable();
-
         $this->fail('DebugClassLoader did not register');
     }
+
+    public function testUnsilencing()
+    {
+        ob_start();
+        $bak = array(
+            ini_set('log_errors', 0),
+            ini_set('display_errors', 1),
+        );
+
+        // See below: this will fail with parse error
+        // but this should not be @-silenced.
+        @ class_exists(__NAMESPACE__.'\TestingUnsilencing', true);
+
+        ini_set('log_errors', $bak[0]);
+        ini_set('display_errors', $bak[1]);
+        $output = ob_get_clean();
+
+        $this->assertStringMatchesFormat('%aParse error%a', $output);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Debug\Exception\DummyException
+     */
+    public function testStacking()
+    {
+        // the ContextErrorException must not be loaded to test the workaround
+        // for https://bugs.php.net/65322.
+        if (class_exists('Symfony\Component\Debug\Exception\ContextErrorException', false)) {
+            $this->markTestSkipped('The ContextErrorException class is already loaded.');
+        }
+
+        $exceptionHandler = $this->getMock('Symfony\Component\Debug\ExceptionHandler', array('handle'));
+        set_exception_handler(array($exceptionHandler, 'handle'));
+
+        $that = $this;
+        $exceptionCheck = function ($exception) use ($that) {
+            $that->assertInstanceOf('Symfony\Component\Debug\Exception\ContextErrorException', $exception);
+            $that->assertEquals(E_STRICT, $exception->getSeverity());
+            $that->assertStringStartsWith(__FILE__, $exception->getFile());
+            $that->assertRegexp('/^Runtime Notice: Declaration/', $exception->getMessage());
+        };
+
+        $exceptionHandler->expects($this->once())
+            ->method('handle')
+            ->will($this->returnCallback($exceptionCheck));
+        ErrorHandler::register();
+
+        try {
+            // Trigger autoloading + E_STRICT at compile time
+            // which in turn triggers $errorHandler->handle()
+            // that again triggers autoloading for ContextErrorException.
+            // Error stacking works around the bug above and everything is fine.
+
+            eval('
+                namespace '.__NAMESPACE__.';
+                class ChildTestingStacking extends TestingStacking { function foo($bar) {} }
+            ');
+        } catch (\Exception $e) {
+            restore_error_handler();
+            restore_exception_handler();
+
+            throw $e;
+        }
+
+        restore_error_handler();
+        restore_exception_handler();
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     */
+    public function testNameCaseMismatch()
+    {
+        class_exists(__NAMESPACE__.'\TestingCaseMismatch', true);
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     */
+    public function testFileCaseMismatch()
+    {
+        class_exists(__NAMESPACE__.'\Fixtures\CaseMismatch', true);
+    }
 }
 
 class ClassLoader
@@ -62,5 +150,14 @@ class ClassLoader
 
     public function findFile($class)
     {
+        if (__NAMESPACE__.'\TestingUnsilencing' === $class) {
+            eval('-- parse error --');
+        } elseif (__NAMESPACE__.'\TestingStacking' === $class) {
+            eval('namespace '.__NAMESPACE__.'; class TestingStacking { function foo() {} }');
+        } elseif (__NAMESPACE__.'\TestingCaseMismatch' === $class) {
+            eval('namespace '.__NAMESPACE__.'; class TestingCaseMisMatch {}');
+        } elseif (__NAMESPACE__.'\Fixtures\CaseMismatch' === $class) {
+            return __DIR__ . '/Fixtures/casemismatch.php';
+        }
     }
 }

src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php

@@ -44,65 +44,6 @@ class ErrorHandlerTest extends \PHPUnit_Framework_TestCase
         error_reporting($this->errorReporting);
     }
 
-    public function testCompileTimeError()
-    {
-        // the ContextErrorException must not be loaded to test the workaround
-        // for https://bugs.php.net/bug.php?id=65322.
-        if (class_exists('Symfony\Component\Debug\Exception\ContextErrorException', false)) {
-            $this->markTestSkipped('The ContextErrorException class is already loaded.');
-        }
-
-        $exceptionHandler = $this->getMock('Symfony\Component\Debug\ExceptionHandler', array('handle'));
-
-        // the following code forces some PHPUnit classes to be loaded
-        // so that they will be available in the exception handler
-        // as they won't be autoloaded by PHP
-        class_exists('PHPUnit_Framework_MockObject_Invocation_Object');
-        $this->assertInstanceOf('stdClass', new \stdClass());
-        $this->assertEquals(1, 1);
-        $this->assertStringStartsWith('foo', 'foobar');
-        $this->assertArrayHasKey('bar', array('bar' => 'foo'));
-
-        $that = $this;
-        $exceptionCheck = function ($exception) use ($that) {
-            $that->assertInstanceOf('Symfony\Component\Debug\Exception\ContextErrorException', $exception);
-            $that->assertEquals(E_STRICT, $exception->getSeverity());
-            $that->assertEquals(2, $exception->getLine());
-            $that->assertStringStartsWith('Runtime Notice: Declaration of _CompileTimeError::foo() should be compatible with', $exception->getMessage());
-            $that->assertArrayHasKey('bar', $exception->getContext());
-        };
-
-        $exceptionHandler->expects($this->once())
-            ->method('handle')
-            ->will($this->returnCallback($exceptionCheck))
-        ;
-
-        ErrorHandler::register();
-        set_exception_handler(array($exceptionHandler, 'handle'));
-
-        // dummy variable to check for in error handler.
-        $bar = 123;
-
-        // trigger compile time error
-        try {
-            eval(<<<'PHP'
-class _BaseCompileTimeError { function foo() {} }
-class _CompileTimeError extends _BaseCompileTimeError { function foo($invalid) {} }
-PHP
-            );
-        } catch (DummyException $e) {
-            // if an exception is thrown, the test passed
-        } catch (\Exception $e) {
-            restore_error_handler();
-            restore_exception_handler();
-
-            throw $e;
-        }
-
-        restore_error_handler();
-        restore_exception_handler();
-    }
-
     public function testNotice()
     {
         $exceptionHandler = $this->getMock('Symfony\Component\Debug\ExceptionHandler', array('handle'));
@@ -112,7 +53,6 @@ PHP
         $exceptionCheck = function ($exception) use ($that) {
             $that->assertInstanceOf('Symfony\Component\Debug\Exception\ContextErrorException', $exception);
             $that->assertEquals(E_NOTICE, $exception->getSeverity());
-            $that->assertEquals(__LINE__ + 44, $exception->getLine());
             $that->assertEquals(__FILE__, $exception->getFile());
             $that->assertRegexp('/^Notice: Undefined variable: (foo|bar)/', $exception->getMessage());
             $that->assertArrayHasKey('foobar', $exception->getContext());

src/Symfony/Component/Debug/Tests/Fixtures/casemismatch.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\Debug\Tests\Fixtures;
+
+class CaseMismatch
+{
+}