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.7] Don't run TransformsRequest twice on ?query= parameters #26366

Merged
merged 1 commit into from Nov 3, 2018

Conversation

Projects
None yet
2 participants
@derekmd
Contributor

derekmd commented Nov 3, 2018

Fixes #19920
A similar fix was already made on the request JSON ParameterBag: #18840

Illuminate\Http\Request::createFromBase() modifies the base Symfony request:

$request->request = $request->getInputSource();

which makes $request->query === $request->request for GET/HEAD requests. HTTP middleware extending TransformsRequest (TrimStrings, ConvertEmptyStringsToNull, and user-land extensions) will recursively traverse ?query= parameters twice.

The existing unit tests were naive (not setting HTTP method nor initializing each ParameterBag) so I corrected their setup to reproduce how Laravel's request lifecycle handles middleware. Test data and assertions for those two tests remain unchanged.

Don't run TransformsRequest twice on ?query=
For HTTP methods GET/HEAD, Laravel
assigns the Request@query ParameterBag
instance to Request@request. So
TransformsRequest middleware will run
the same transforms twice on
Request@query parameters.

Instead setup tests to form a Request
object in the same manner the framework
does before interacting with
middleware ConvertEmptyStringsToNull and
TrimStrings.

Illuminate\Http\Request@capture() calls
createFromBase() - do the same here and
explicitly test manipulations are run
once per parameter.

@taylorotwell taylorotwell merged commit 4a4249c into laravel:5.7 Nov 3, 2018

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@derekmd derekmd deleted the derekmd:remove-transforms-request-redundant-traversal branch Nov 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment