From 9f238988f99c103850d0147c668d1a6e25e9da6f Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Fri, 12 Jul 2019 11:05:21 +0200 Subject: [PATCH] [FrameworkBundle][CacheWarmer] Ignore exeptions thrown during reflection classes autoload --- .../AbstractPhpFileCacheWarmer.php | 5 +- .../CacheWarmer/AnnotationsCacheWarmer.php | 41 +++++++++----- .../CacheWarmer/SerializerCacheWarmer.php | 8 +++ .../CacheWarmer/ValidatorCacheWarmer.php | 8 +++ .../AnnotationsCacheWarmerTest.php | 48 +++++++++++++++++ .../CacheWarmer/SerializerCacheWarmerTest.php | 54 +++++++++++++++++++ .../CacheWarmer/ValidatorCacheWarmerTest.php | 50 +++++++++++++++++ .../Resources/does_not_exist.yaml | 1 + .../Validation/Resources/does_not_exist.yaml | 1 + .../Bundle/FrameworkBundle/composer.json | 2 +- .../Cache/Adapter/PhpArrayAdapter.php | 2 +- .../Resource/ClassExistenceResource.php | 45 ++++++++++++---- 12 files changed, 240 insertions(+), 25 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Resources/does_not_exist.yaml diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index 43cbb7968a8ad..08191ddff7732 100644 --- 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 diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php index 3c32cb1c4a33a..cdbb653998960 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -17,6 +17,7 @@ use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\DoctrineProvider; +use Symfony\Component\Config\Resource\ClassExistenceResource; /** * Warms up annotation caches for classes found in composer's autoload class map @@ -66,15 +67,13 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) $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) { + try { + ClassExistenceResource::throwOnRequiredClass($class, $e); + + throw $e; + } catch (\ReflectionException $e) { + } } } @@ -84,14 +83,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 index 917ea47235fd7..431af5ab78ccf 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -14,6 +14,7 @@ use Doctrine\Common\Annotations\AnnotationException; use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Config\Resource\ClassExistenceResource; use Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory; use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory; use Symfony\Component\Serializer\Mapping\Loader\LoaderChain; @@ -60,6 +61,13 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) // ignore failing reflection } catch (AnnotationException $e) { // ignore failing annotations + } catch (\Exception $e) { + try { + ClassExistenceResource::throwOnRequiredClass($mappedClass, $e); + + throw $e; + } catch (\ReflectionException $e) { + } } } } diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php index 8ac38133c06ee..f262ab945611a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -15,6 +15,7 @@ use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\PhpArrayAdapter; +use Symfony\Component\Config\Resource\ClassExistenceResource; use Symfony\Component\Validator\Mapping\Cache\Psr6Cache; use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory; use Symfony\Component\Validator\Mapping\Loader\LoaderChain; @@ -65,6 +66,13 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter) // ignore failing reflection } catch (AnnotationException $e) { // ignore failing annotations + } catch (\Exception $e) { + try { + ClassExistenceResource::throwOnRequiredClass($mappedClass, $e); + + throw $e; + } catch (\ReflectionException $e) { + } } } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php index 9cc3bfa2d2f91..fb2545b7fa786 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php @@ -85,6 +85,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('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. + * + * @expectedException \DomainException + * @expectedExceptionMessage This exception should not be caught by the warmer. + */ + public function testClassAutoloadExceptionWithUnrelatedException() + { + $this->assertFalse(class_exists($annotatedClass = 'AClassThatDoesNotExist_FWB_CacheWarmer_AnnotationsCacheWarmerTest', false)); + + file_put_contents($this->cacheDir.'/annotations.map', sprintf('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 \PHPUnit_Framework_MockObject_MockObject|Reader */ diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php index 50a5abf0ae98b..5a8f99fc17b42 100644 --- 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->assertInternalType('array', $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. + * + * @expectedException \DomainException + * @expectedExceptionMessage This exception should not be caught by the warmer. + */ + public function testClassAutoloadExceptionWithUnrelatedException() + { + 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 index 47c88f1a206af..8cfe2bd253e36 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php @@ -102,4 +102,54 @@ public function testWarmUpWithoutLoader() $this->assertInternalType('array', $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. + * + * @expectedException \DomainException + * @expectedExceptionMessage This exception should not be caught by the warmer. + */ + public function testClassAutoloadExceptionWithUnrelatedException() + { + $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/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Serialization/Resources/does_not_exist.yaml new file mode 100644 index 0000000000000..061691224619a --- /dev/null +++ b/src/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 new file mode 100644 index 0000000000000..69b1635e47063 --- /dev/null +++ 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 index 40cff711ae8ad..f215983ef4ee8 100644 --- 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.30|~4.2.11|^4.3.3", "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 index 42a4142099385..e1361581a3f11 100644 --- 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 index 5554f4c672524..c25d6ca7b39d7 100644 --- a/src/Symfony/Component/Config/Resource/ClassExistenceResource.php +++ b/src/Symfony/Component/Config/Resource/ClassExistenceResource.php @@ -69,7 +69,7 @@ public function isFresh($timestamp) $exists = $exists || $loaded; } elseif (!$exists = $loaded) { if (!self::$autoloadLevel++) { - spl_autoload_register(__CLASS__.'::throwOnRequiredClass'); + spl_autoload_register(__CLASS__.'::_throwOnRequiredClass'); } $autoloadedClass = self::$autoloadedClass; self::$autoloadedClass = $this->resource; @@ -77,14 +77,23 @@ 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]); + handle_reflection_exception: + if (0 >= $timestamp) { + unset(self::$existsCache[1][$this->resource]); + throw $e; + } + } catch (\Exception $e) { + try { + self::throwOnRequiredClass($this->resource, $e); + throw $e; + } catch (\ReflectionException $e) { + goto handle_reflection_exception; } } finally { self::$autoloadedClass = $autoloadedClass; if (!--self::$autoloadLevel) { - spl_autoload_unregister(__CLASS__.'::throwOnRequiredClass'); + spl_autoload_unregister(__CLASS__.'::_throwOnRequiredClass'); } } } @@ -121,20 +130,38 @@ public function unserialize($serialized) * * @internal */ - public static function throwOnRequiredClass($class) + public static function _throwOnRequiredClass($class) { if (self::$autoloadedClass === $class) { return; } - $e = new \ReflectionException("Class $class not found"); - $trace = $e->getTrace(); + + self::throwOnRequiredClass($class, null); + } + + /** + * @throws \ReflectionException When $class is not found and is required + * + * @internal + */ + public static function throwOnRequiredClass($class, \Exception $previous = null) + { + if (class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false)) { + return; + } + + $e = new \ReflectionException("Class $class not found", 0, $previous); + $trace = $previous instanceof \Exception ? $previous->getTrace() : $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 (!$previous instanceof \Exception && isset($trace[++$i]['function']) && !isset($trace[$i]['class'])) { switch ($trace[$i]['function']) { case 'get_class_methods': case 'get_class_vars':