Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Expand Up @@ -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);
}
}

Expand All @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another fix / improvement. Moving this catch on every calls allows to not fail totally on first fail but just on each failing cases.

}
}

foreach ($reflectionClass->getProperties() as $reflectionProperty) {
$reader->getPropertyAnnotations($reflectionProperty);
try {
$reader->getPropertyAnnotations($reflectionProperty);
} catch (AnnotationException $e) {
}
}
}
}
Expand Up @@ -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);
}
}
}
Expand Down
Expand Up @@ -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);
}
}
}
Expand Down
Expand Up @@ -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
*/
Expand Down
Expand Up @@ -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);
}
}
Expand Up @@ -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);
}
}
@@ -0,0 +1 @@
AClassThatDoesNotExist_FWB_CacheWarmer_SerializerCacheWarmerTest: ~
@@ -0,0 +1 @@
AClassThatDoesNotExist_FWB_CacheWarmer_ValidatorCacheWarmerTest: ~
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php
Expand Up @@ -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)
{
Expand Down
57 changes: 47 additions & 10 deletions src/Symfony/Component/Config/Resource/ClassExistenceResource.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
{
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':
Expand Down