bug #32516 [FrameworkBundle][Config] Ignore exceptions thrown during …

…reflection classes autoload (fancyweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle][Config] Ignore exceptions thrown during reflection classes autoload

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32499 with PHP 7.3+
| License       | MIT
| Doc PR        | -

The behavior when an exception is thrown in a class loader changed in PHP 7.3 (cf https://3v4l.org/OQPk9). That means that the `throwOnRequiredClass` trick that is done in the parent class of these cache warmers (`AbstractPhpFileCacheWarmer`) does not work anymore with PHP7.3+.

Commits
-------

dbd9b75 [FrameworkBundle][Config] Ignore exeptions thrown during reflection classes autoload

src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php

@@ -16,6 +16,7 @@
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
 use Symfony\Component\Cache\Adapter\ProxyAdapter;
+use Symfony\Component\Config\Resource\ClassExistenceResource;
 use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
 
 /**
@@ -54,13 +55,13 @@ public function warmUp($cacheDir)
     {
         $arrayAdapter = new ArrayAdapter();
 
-        spl_autoload_register([PhpArrayAdapter::class, 'throwOnRequiredClass']);
+        spl_autoload_register([ClassExistenceResource::class, 'throwOnRequiredClass']);
         try {
             if (!$this->doWarmUp($cacheDir, $arrayAdapter)) {
                 return;
             }
         } finally {
-            spl_autoload_unregister([PhpArrayAdapter::class, 'throwOnRequiredClass']);
+            spl_autoload_unregister([ClassExistenceResource::class, 'throwOnRequiredClass']);
         }
 
         // the ArrayAdapter stores the values serialized
@@ -82,6 +83,17 @@ protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array
         $phpArrayAdapter->warmUp($values);
     }
 
+    /**
+     * @internal
+     */
+    final protected function ignoreAutoloadException($class, \Exception $exception)
+    {
+        try {
+            ClassExistenceResource::throwOnRequiredClass($class, $exception);
+        } catch (\ReflectionException $e) {
+        }
+    }
+
     /**
      * @param string       $cacheDir
      * @param ArrayAdapter $arrayAdapter

src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php

@@ -64,17 +64,8 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
             }
             try {
                 $this->readAllComponents($reader, $class);
-            } catch (\ReflectionException $e) {
-                // ignore failing reflection
-            } catch (AnnotationException $e) {
-                /*
-                 * Ignore any AnnotationException to not break the cache warming process if an Annotation is badly
-                 * configured or could not be found / read / etc.
-                 *
-                 * In particular cases, an Annotation in your code can be used and defined only for a specific
-                 * environment but is always added to the annotations.map file by some Symfony default behaviors,
-                 * and you always end up with a not found Annotation.
-                 */
+            } catch (\Exception $e) {
+                $this->ignoreAutoloadException($class, $e);
             }
         }
 
@@ -84,14 +75,32 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
     private function readAllComponents(Reader $reader, $class)
     {
         $reflectionClass = new \ReflectionClass($class);
-        $reader->getClassAnnotations($reflectionClass);
+
+        try {
+            $reader->getClassAnnotations($reflectionClass);
+        } catch (AnnotationException $e) {
+            /*
+             * Ignore any AnnotationException to not break the cache warming process if an Annotation is badly
+             * configured or could not be found / read / etc.
+             *
+             * In particular cases, an Annotation in your code can be used and defined only for a specific
+             * environment but is always added to the annotations.map file by some Symfony default behaviors,
+             * and you always end up with a not found Annotation.
+             */
+        }
 
         foreach ($reflectionClass->getMethods() as $reflectionMethod) {
-            $reader->getMethodAnnotations($reflectionMethod);
+            try {
+                $reader->getMethodAnnotations($reflectionMethod);
+            } catch (AnnotationException $e) {
+            }
         }
 
         foreach ($reflectionClass->getProperties() as $reflectionProperty) {
-            $reader->getPropertyAnnotations($reflectionProperty);
+            try {
+                $reader->getPropertyAnnotations($reflectionProperty);
+            } catch (AnnotationException $e) {
+            }
         }
     }
 }

src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php

@@ -56,10 +56,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
             foreach ($loader->getMappedClasses() as $mappedClass) {
                 try {
                     $metadataFactory->getMetadataFor($mappedClass);
-                } catch (\ReflectionException $e) {
-                    // ignore failing reflection
                 } catch (AnnotationException $e) {
                     // ignore failing annotations
+                } catch (\Exception $e) {
+                    $this->ignoreAutoloadException($mappedClass, $e);
                 }
             }
         }

src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php

@@ -61,10 +61,10 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
                     if ($metadataFactory->hasMetadataFor($mappedClass)) {
                         $metadataFactory->getMetadataFor($mappedClass);
                     }
-                } catch (\ReflectionException $e) {
-                    // ignore failing reflection
                 } catch (AnnotationException $e) {
                     // ignore failing annotations
+                } catch (\Exception $e) {
+                    $this->ignoreAutoloadException($mappedClass, $e);
                 }
             }
         }

src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php

@@ -86,6 +86,54 @@ public function testAnnotationsCacheWarmerWithDebugEnabled()
         $reader->getPropertyAnnotations($refClass->getProperty('cacheDir'));
     }
 
+    /**
+     * Test that the cache warming process is not broken if a class loader
+     * throws an exception (on class / file not found for example).
+     */
+    public function testClassAutoloadException()
+    {
+        $this->assertFalse(class_exists($annotatedClass = 'C\C\C', false));
+
+        file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
+        $warmer = new AnnotationsCacheWarmer(new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
+            if ($class === $annotatedClass) {
+                throw new \DomainException('This exception should be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp($this->cacheDir);
+
+        spl_autoload_unregister($classLoader);
+    }
+
+    /**
+     * Test that the cache warming process is broken if a class loader throws an
+     * exception but that is unrelated to the class load.
+     */
+    public function testClassAutoloadExceptionWithUnrelatedException()
+    {
+        $this->expectException(\DomainException::class);
+        $this->expectExceptionMessage('This exception should not be caught by the warmer.');
+
+        $this->assertFalse(class_exists($annotatedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_AnnotationsCacheWarmerTest', false));
+
+        file_put_contents($this->cacheDir.'/annotations.map', sprintf('<?php return %s;', var_export([$annotatedClass], true)));
+        $warmer = new AnnotationsCacheWarmer(new AnnotationReader(), tempnam($this->cacheDir, __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classLoader = function ($class) use ($annotatedClass) {
+            if ($class === $annotatedClass) {
+                eval('class '.$annotatedClass.'{}');
+                throw new \DomainException('This exception should not be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp($this->cacheDir);
+
+        spl_autoload_unregister($classLoader);
+    }
+
     /**
      * @return MockObject|Reader
      */

src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php

@@ -77,4 +77,58 @@ public function testWarmUpWithoutLoader()
         $this->assertIsArray($values);
         $this->assertCount(0, $values);
     }
+
+    /**
+     * Test that the cache warming process is not broken if a class loader
+     * throws an exception (on class / file not found for example).
+     */
+    public function testClassAutoloadException()
+    {
+        if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
+            $this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
+        }
+
+        $this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
+
+        $warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
+            if ($class === $mappedClass) {
+                throw new \DomainException('This exception should be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp('foo');
+
+        spl_autoload_unregister($classLoader);
+    }
+
+    /**
+     * Test that the cache warming process is broken if a class loader throws an
+     * exception but that is unrelated to the class load.
+     */
+    public function testClassAutoloadExceptionWithUnrelatedException()
+    {
+        $this->expectException(\DomainException::class);
+        $this->expectExceptionMessage('This exception should not be caught by the warmer.');
+
+        if (!class_exists(CacheClassMetadataFactory::class) || !method_exists(XmlFileLoader::class, 'getMappedClasses') || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {
+            $this->markTestSkipped('The Serializer default cache warmer has been introduced in the Serializer Component version 3.2.');
+        }
+
+        $this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest', false));
+
+        $warmer = new SerializerCacheWarmer([new YamlFileLoader(__DIR__.'/../Fixtures/Serialization/Resources/does_not_exist.yaml')], tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
+            if ($class === $mappedClass) {
+                eval('class '.$mappedClass.'{}');
+                throw new \DomainException('This exception should not be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp('foo');
+
+        spl_autoload_unregister($classLoader);
+    }
 }

src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php

@@ -107,4 +107,54 @@ public function testWarmUpWithoutLoader()
         $this->assertIsArray($values);
         $this->assertCount(0, $values);
     }
+
+    /**
+     * Test that the cache warming process is not broken if a class loader
+     * throws an exception (on class / file not found for example).
+     */
+    public function testClassAutoloadException()
+    {
+        $this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest', false));
+
+        $validatorBuilder = new ValidatorBuilder();
+        $validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/does_not_exist.yaml');
+        $warmer = new ValidatorCacheWarmer($validatorBuilder, tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classloader = function ($class) use ($mappedClass) {
+            if ($class === $mappedClass) {
+                throw new \DomainException('This exception should be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp('foo');
+
+        spl_autoload_unregister($classloader);
+    }
+
+    /**
+     * Test that the cache warming process is broken if a class loader throws an
+     * exception but that is unrelated to the class load.
+     */
+    public function testClassAutoloadExceptionWithUnrelatedException()
+    {
+        $this->expectException(\DomainException::class);
+        $this->expectExceptionMessage('This exception should not be caught by the warmer.');
+
+        $this->assertFalse(class_exists($mappedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest', false));
+
+        $validatorBuilder = new ValidatorBuilder();
+        $validatorBuilder->addYamlMapping(__DIR__.'/../Fixtures/Validation/Resources/does_not_exist.yaml');
+        $warmer = new ValidatorCacheWarmer($validatorBuilder, tempnam(sys_get_temp_dir(), __FUNCTION__), new ArrayAdapter());
+
+        spl_autoload_register($classLoader = function ($class) use ($mappedClass) {
+            if ($class === $mappedClass) {
+                eval('class '.$mappedClass.'{}');
+                throw new \DomainException('This exception should not be caught by the warmer.');
+            }
+        }, true, true);
+
+        $warmer->warmUp('foo');
+
+        spl_autoload_unregister($classLoader);
+    }
 }

src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml

@@ -0,0 +1 @@
+AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest: ~

src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Resources/does_not_exist.yaml

@@ -0,0 +1 @@
+AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest: ~

src/Symfony/Bundle/FrameworkBundle/composer.json

@@ -21,7 +21,7 @@
         "symfony/cache": "~3.4|~4.0",
         "symfony/class-loader": "~3.2",
         "symfony/dependency-injection": "^3.4.24|^4.2.5",
-        "symfony/config": "~3.4|~4.0",
+        "symfony/config": "^3.4.31|^4.3.4",
         "symfony/debug": "~2.8|~3.0|~4.0",
         "symfony/event-dispatcher": "~3.4|~4.0",
         "symfony/http-foundation": "^3.3.11|~4.0",

src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php

@@ -266,7 +266,7 @@ private function generateItems(array $keys)
     /**
      * @throws \ReflectionException When $class is not found and is required
      *
-     * @internal
+     * @internal to be removed in Symfony 5.0
      */
     public static function throwOnRequiredClass($class)
     {

src/Symfony/Component/Config/Resource/ClassExistenceResource.php

@@ -76,10 +76,14 @@ public function isFresh($timestamp)
 
             try {
                 $exists = class_exists($this->resource) || interface_exists($this->resource, false) || trait_exists($this->resource, false);
-            } catch (\ReflectionException $e) {
-                if (0 >= $timestamp) {
-                    unset(self::$existsCache[1][$this->resource]);
-                    throw $e;
+            } catch (\Exception $e) {
+                try {
+                    self::throwOnRequiredClass($this->resource, $e);
+                } catch (\ReflectionException $e) {
+                    if (0 >= $timestamp) {
+                        unset(self::$existsCache[1][$this->resource]);
+                        throw $e;
+                    }
                 }
             } finally {
                 self::$autoloadedClass = $autoloadedClass;
@@ -117,24 +121,57 @@ public function unserialize($serialized)
     }
 
     /**
-     * @throws \ReflectionException When $class is not found and is required
+     * Throws a reflection exception when the passed class does not exist but is required.
+     *
+     * A class is considered "not required" when it's loaded as part of a "class_exists" or similar check.
+     *
+     * This function can be used as an autoload function to throw a reflection
+     * exception if the class was not found by previous autoload functions.
+     *
+     * A previous exception can be passed. In this case, the class is considered as being
+     * required totally, so if it doesn't exist, a reflection exception is always thrown.
+     * If it exists, the previous exception is rethrown.
+     *
+     * @throws \ReflectionException
      *
      * @internal
      */
-    public static function throwOnRequiredClass($class)
+    public static function throwOnRequiredClass($class, \Exception $previous = null)
     {
-        if (self::$autoloadedClass === $class) {
+        // If the passed class is the resource being checked, we shouldn't throw.
+        if (null === $previous && self::$autoloadedClass === $class) {
+            return;
+        }
+
+        if (class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false)) {
+            if (null !== $previous) {
+                throw $previous;
+            }
+
             return;
         }
-        $e = new \ReflectionException("Class $class not found");
+
+        if ($previous instanceof \ReflectionException) {
+            throw $previous;
+        }
+
+        $e = new \ReflectionException("Class $class not found", 0, $previous);
+
+        if (null !== $previous) {
+            throw $e;
+        }
+
         $trace = $e->getTrace();
         $autoloadFrame = [
             'function' => 'spl_autoload_call',
             'args' => [$class],
         ];
-        $i = 1 + array_search($autoloadFrame, $trace, true);
 
-        if (isset($trace[$i]['function']) && !isset($trace[$i]['class'])) {
+        if (false === $i = array_search($autoloadFrame, $trace, true)) {
+            throw $e;
+        }
+
+        if (isset($trace[++$i]['function']) && !isset($trace[$i]['class'])) {
             switch ($trace[$i]['function']) {
                 case 'get_class_methods':
                 case 'get_class_vars':