8000 [HttpKernel] PreviousHandler possible wrong assignment · Issue #33024 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpKernel] PreviousHandler possible wrong assignment #33024

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.

Sign up for GitHub

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

Jump to bottom
Closed
kaznovac opened this issue Aug 7, 2019 · 1 comment
Closed

[HttpKernel] PreviousHandler possible wrong assignment #33024

kaznovac opened this issue Aug 7, 2019 · 1 comment
Labels
HttpKernel

Comments

@kaznovac
Copy link
Contributor
kaznovac commented Aug 7, 2019

Symfony version(s) affected: 4.3.x

Description
this piece of code looks wrong, can someone please take a look
https://github.com/symfony/symfony/blob/4.3/src/Symfony/Component/HttpKernel/Kernel.php#L508-L512

            $previousHandler = \defined('PHPUNIT_COMPOSER_INSTALL');
            $previousHandler = $previousHandler ?: set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
                if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) {
                    return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;
                }

defined returns true or false

  • if true ?: ... => $previousHandler = true;

  • if false this might happen

                    return $previousHandler ? $previousHandler($type, $message, $file, $line) : false;

as $previousHandler=false this returns false;
luckily, otherwise we might try to execute true as a function :)

Possible Solution
this is probably wrong assignment

            $previousHandler = \defined('PHPUNIT_COMPOSER_INSTALL');
@fancyweb fancyweb mentioned this issue Aug 8, 2019
Merged
@fancyweb
Copy link
Contributor
fancyweb commented Aug 8, 2019

Same as #32758

nicolas-grekas added a commit that referenced this issue Aug 8, 2019
@nicolas-grekas
minor #33031 [HttpKernel] Clarify error handler restoring process aga…
e48d2c1 
…in (fancyweb)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Clarify error handler restoring process again

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

Commits
-------

4ee54f0 [HttpKernel] Clarify error handler restoring process again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
HttpKernel
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kaznovac @nicolas-grekas @xabbuh @fancyweb
0