diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php index e593002e22..28e26cb3d1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php @@ -14,6 +14,7 @@ namespace Symfony\Bundle\FrameworkBundle\CacheWarmer; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\NullAdapter; use Symfony\Component\Cache\Adapter\PhpArrayAdapter; +use Symfony\Component\Config\Resource\ClassExistenceResource; use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; /** @@ -46,13 +47,13 @@ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface { $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 @@ -68,6 +69,17 @@ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface $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 index 523d5a34b4..78c118a999 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php @@ -68,17 +68,8 @@ class AnnotationsCacheWarmer extends AbstractPhpFileCacheWarmer } 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); } } @@ -88,14 +79,32 @@ class AnnotationsCacheWarmer extends AbstractPhpFileCacheWarmer private function readAllComponents(Reader $reader, string $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 41a8aaa04d..61f1967cf0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php @@ -58,10 +58,10 @@ class SerializerCacheWarmer extends AbstractPhpFileCacheWarmer 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 index 36e4bb185d..5aae19d5f6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php +++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php @@ -67,10 +67,10 @@ class ValidatorCacheWarmer extends AbstractPhpFileCacheWarmer 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 index a82f4a846f..35c2893511 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/AnnotationsCacheWarmerTest.php @@ -72,6 +72,54 @@ class AnnotationsCacheWarmerTest extends TestCase $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. + */ + 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('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 index ccaa64931b..0d4c8f5812 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/SerializerCacheWarmerTest.php @@ -60,4 +60,58 @@ class SerializerCacheWarmerTest extends TestCase $this->assertFileExists($file); } + + /** + * 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 index 5af2cc2ac5..8dfb0ab39a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/CacheWarmer/ValidatorCacheWarmerTest.php @@ -81,4 +81,54 @@ class ValidatorCacheWarmerTest extends TestCase $this->assertFileExists($file); } + + /** + * 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/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 0000000000..0616912246 --- /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 0000000000..69b1635e47 --- /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 ebefb2c208..c1e16c0a9e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -19,7 +19,7 @@ "php": "^7.1.3", "ext-xml": "*", "symfony/cache": "^4.4|^5.0", - "symfony/config": "^4.2|^5.0", + "symfony/config": "^4.3.4|^5.0", "symfony/dependency-injection": "^4.4|^5.0", "symfony/error-renderer": "^4.4|^5.0", "symfony/http-foundation": "^4.3|^5.0", diff --git a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php index 84d8675c4a..2259240653 100644 --- a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php @@ -288,7 +288,7 @@ class PhpArrayAdapter implements AdapterInterface, CacheInterface, PruneableInte /** * @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 51154cfd6a..25d2725ed4 100644 --- a/src/Symfony/Component/Config/Resource/ClassExistenceResource.php +++ b/src/Symfony/Component/Config/Resource/ClassExistenceResource.php @@ -76,10 +76,14 @@ class ClassExistenceResource implements SelfCheckingResourceInterface 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; @@ -109,24 +113,57 @@ class ClassExistenceResource implements SelfCheckingResourceInterface } /** - * @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; } - $e = new \ReflectionException("Class $class not found"); + + if (class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false)) { + if (null !== $previous) { + throw $previous; + } + + return; + } + + 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':