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.0] Remove Sanitize From Stub #7026

Merged
merged 1 commit into from
Jan 19, 2015
Merged

[5.0] Remove Sanitize From Stub #7026

merged 1 commit into from
Jan 19, 2015

Conversation

ytake
Copy link
Contributor

@ytake ytake commented Jan 16, 2015

No description provided.

@GrahamCampbell
Copy link
Member

👍

@GrahamCampbell GrahamCampbell changed the title remove sanitize [5.0] Remove Sanitize From Stub Jan 16, 2015
taylorotwell added a commit that referenced this pull request Jan 19, 2015
[5.0] Remove Sanitize From Stub
@taylorotwell taylorotwell merged commit d891733 into laravel:master Jan 19, 2015
taylorotwell pushed a commit that referenced this pull request Aug 9, 2021
…quired (#37921)

* Manually populate POST request body with JSON data only when required

This fixes a 6 year old bug introduced in #7026 where GET requests would have GET data populated in the POST body property leading to issues around Request::post() and $request->getParsedBody() returning GET values when called on GET requests.

This is a resubmit of #17087 & #36708, and fixes #22805. Credit to @dan-har for the initial solution and @mixlion for updating it for >=6.x.

The original PR was meant to support POST requests where their Content-type was set to application/json (instead of the typical application/x-www-form-urlencoded), but it introduced a subtle and dangerous bug because while $request->getInputSource() does return the JSON data for JSON requests, it also returns the GET data for GET requests. This commit solves the underlying issue without breaking compatibility with the original functionality.

* Add test for non-JSON GET requests

* Style fixes

* Extra space removal

* GitHub's editor needs some work
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.

3 participants