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

[5.6] Let Exception Handler do the reporting #24864

Merged
merged 3 commits into from Jul 16, 2018
Merged

[5.6] Let Exception Handler do the reporting #24864

merged 3 commits into from Jul 16, 2018

Conversation

deleugpn
Copy link
Contributor

@deleugpn deleugpn commented Jul 16, 2018

When using the API Guard on auth.php, Laravel will try to authenticate the user through the Authorization Header.
The User Provider can be Eloquent, in which case it will require a valid database connection.

The problem happens when the database connection configuration is wrong (username, password, database, etc) and the Auth Provider tries to reach for the database to authenticate the User.
Since the database is wrong, an exception will be thrown. However, the Handler will try to load the Context from the Auth Guard as stated here: https://github.com/laravel/framework/blob/5.6/src/Illuminate/Foundation/Exceptions/Handler.php#L151-L153.

The Auth Guard will try to reach for the Database to load the User, which again will throw an exception, which will try to load the Context, which will go to the Database until PHP Max Allowed Memory gets exhausted.

This pull request changes the ConnectionFactory to just rely on the default Exception Handler instead of trying to force-log an exception itself, thus avoiding an infinite loop.

deleugpn and others added 3 commits July 16, 2018 08:34
Co-authored-by: Abdala Cerqueira <abdala.cerqueira@gmail.com>
Co-authored-by: Abdala Cerqueira <abdala.cerqueira@gmail.com>
@deleugpn deleugpn changed the title Let Exception Handler do the reporting [5.6] Let Exception Handler do the reporting Jul 16, 2018
@taylorotwell taylorotwell merged commit 23f6279 into laravel:5.6 Jul 16, 2018
@sisve
Copy link
Contributor

sisve commented Jul 17, 2018

Does this fix #23415? Will this be backported to 5.5?

@GrahamCampbell
Copy link
Member

This test is totally broken if you don't happen to have a copy of mysql running on your local machine!

@GrahamCampbell
Copy link
Member

I'll PR a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants