-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Check response for avoid inject debugbar on json ajax #1558
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: master
Are you sure you want to change the base?
Conversation
@erikn69 Thanks a lot for the quick reaction with commit. This method is clearly the place to fix the issue, but I'm a bit confused by the if-else tree here. This injection also happens on plain text responses. I don't have a test case right now, but that's how I've stumbled unto this undesired inject issue. |
Not possible, look laravel-debugbar/src/LaravelDebugbar.php Lines 791 to 794 in 6fd181a
return response('Hello World', 200)
->header('Content-Type', 'text/plain'); Anyway, if you show me a way to reproduce the bug, I will try to upload a fix |
Thanks again for the followup erik69. Your example should work as expected. Thanks again. |
I'm not sure if this is the best way. Maybe we can use https://www.php.net/manual/en/function.json-validate.php when available? |
Yeah, but it would be best if people just set the json header.
would be the alternative to json_validate pre PHP8.3 |
an alternative would be to ONLY show the debugbar on actual HTML (eg if it includes HTML or something), but that's also not really ideal. |
b323fb2
to
e7763a5
Compare
Yes, but what happens in the case that request does not expect |
a11602d
to
8a8138a
Compare
Yeah I'm a bit worried about the validate impact. Do we really need to be sure it's json, or just looks like json? |
It is an extreme case, it happened to me with some plugin, Also, if some of the previous conditions are false, this condition never gets executed |
Closes #1275