Skip to content

[HttpKernel] Prevent a fatal error when DebugHandlersListener is used with a kernel with no terminateWithException() method #17819

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

Merged
merged 1 commit into from
Feb 17, 2016

Conversation

jakzal
Copy link
Contributor

@jakzal jakzal commented Feb 16, 2016

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

I just suffered from this bug on a project that used HttpKernelInterface implementation with no `terminateWithException() method (which is not part of the interface).

@@ -93,7 +93,9 @@ public function configure(Event $event = null)
}
if (!$this->exceptionHandler) {
if ($event instanceof KernelEvent) {
$this->exceptionHandler = array($event->getKernel(), 'terminateWithException');
if (is_callable(array($event->getKernel(), 'terminateWithException'))) {
Copy link
Member

Choose a reason for hiding this comment

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

IHMO, method_exists would be more straightforward here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't it return true if the method was private?

Copy link
Member

Choose a reason for hiding this comment

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

yes, but I'd say we don't care, we already use method_exists in the code base and that doesn't cause any issue I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Updated.

Copy link
Member

Choose a reason for hiding this comment

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

and is_callable would return true as soon there is __call even though the implementation does not support it, so it is not a silver-bullet either.

… with a kernel with no terminateWithException() method
@jakzal jakzal force-pushed the http-kernel-debug-handlers-listener branch from b774445 to 2849152 Compare February 16, 2016 16:51
@nicolas-grekas
Copy link
Member

👍 failures look unrelated

@xabbuh
Copy link
Member

xabbuh commented Feb 16, 2016

👍

Status: Reviewed

@fabpot
Copy link
Member

fabpot commented Feb 17, 2016

Thank you @jakzal.

@fabpot fabpot merged commit 2849152 into symfony:2.7 Feb 17, 2016
fabpot added a commit that referenced this pull request Feb 17, 2016
…ner is used with a kernel with no terminateWithException() method (jakzal)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpKernel] Prevent a fatal error when DebugHandlersListener is used with a kernel with no terminateWithException() method

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

I just suffered from this bug on a project that used HttpKernelInterface implementation with no `terminateWithException() method (which is not part of the interface).

Commits
-------

2849152 [HttpKernel] Prevent a fatal error when DebugHandlersListener is used with a kernel with no terminateWithException() method
@jakzal jakzal deleted the http-kernel-debug-handlers-listener branch February 17, 2016 08:42
This was referenced Feb 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants