[FrameworkBundle][Config] Ignore exeptions thrown during reflection c…

…lasses autoload
symfony · nicolas-grekas · Aug 7, 2019 · Jul 12, 2019 · Aug 6, 2019 · dbd9b75d8692e838fa4524d9e0f5fdafa2703bb0
commit dbd9b75d8692e838fa4524d9e0f5fdafa2703bb0
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/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

diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/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) {
+            }
        }
    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/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);
                }
            }
        }

diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/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);
                }
            }
        }

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php b/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
     */

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php b/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);
+    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php b/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);
+    }
 }
diff --git a/...Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml b/...Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml
@@ -0,0 +1 @@
+AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest: ~
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Resources/does_not_exist.yaml b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Resources/does_not_exist.yaml
@@ -0,0 +1 @@
+AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest: ~
diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/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",

diff --git a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php b/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)
    {

diff --git a/src/Symfony/Component/Config/Resource/ClassExistenceResource.php b/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':
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest: ~
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest: ~