Skip to content
Permalink
Browse files

bug #30327 [HttpKernel] Fix possible infinite loop of exceptions (enu…

…mag)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Fix possible infinite loop of exceptions

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

I ran into an [issue](php-enqueue/enqueue-dev#774) in the enqueue library which copied this part of code from Symfony. I'm now starting to understand what the problem is and it should most likely be fixed in Symfony as well.

I didn't actually run into it in Symfony itself but it seems at least hypothetically possible. Imagine if [here](https://github.com/symfony/symfony/blob/8c3dc8254a508593aa0637445659e93e39d31dca/src/Symfony/Component/HttpKernel/EventListener/ExceptionListener.php#L77) `$e` is somehow the same (===) as `$exception`. The code [below](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/EventListener/ExceptionListener.php#L82-L92) will then find the last exception in the `getPrevious()` chain and assigns `$exception` as the previous. However in the off chance that `$exception` is actually `$e` (the first exception in the chain) then it creates an infinite loop of exceptions which is not good for monolog and exception handlers.

What do you think?

Commits
-------

3447222 [HttpKernel] Fix possible infinite loop of exceptions
  • Loading branch information...
nicolas-grekas committed Feb 22, 2019
2 parents 7b4f4bf + 3447222 commit 848a83078cc4b3927d614c9497456193bf4f5cea
Showing with 3 additions and 4 deletions.
  1. +3 −4 src/Symfony/Component/HttpKernel/EventListener/ExceptionListener.php
@@ -56,13 +56,12 @@ public function onKernelException(GetResponseForExceptionEvent $event)
} catch (\Exception $e) {
$this->logException($e, sprintf('Exception thrown when handling an exception (%s: %s at %s line %s)', \get_class($e), $e->getMessage(), $e->getFile(), $e->getLine()));
$wrapper = $e;
while ($prev = $wrapper->getPrevious()) {
$prev = $e;
do {
if ($exception === $wrapper = $prev) {
throw $e;
}
}
} while ($prev = $wrapper->getPrevious());
$prev = new \ReflectionProperty($wrapper instanceof \Exception ? \Exception::class : \Error::class, 'previous');
$prev->setAccessible(true);

0 comments on commit 848a830

Please sign in to comment.
You can’t perform that action at this time.