[HttpKernel] Remove _path from query parameters when fragment is a subrequest #17595
Conversation
|
We need a test reproducing this. |
|
And this seems the wrong fix IMO. the right fix would be to avoid generating |
|
The alternative would be patching |
|
The more I look at it, the more I think this is the appropriate fix. "_path" is in the query string specifically for the FragmentListener to parse; once it's been parsed, it's removed from the ParameterBag. It makes sense to remove it when finished with the listener, whether we use it or not, in order to keep behavior consistent. Suppressing "_path" during generation has a higher likelihood of side effects, and would make the generation code more complex: generateFragmentUri() then has to know whether the URI is intended to be consumed by a genuine ESI/Hinclude request, or whether it's being inlined. |
|
@cmenning Will you be able to add a test? |
|
@fabpot Tests added. |
…request attributes are already set Added tests for _path removal in FragmentListener
|
👍 |
|
👍 Status: reviewed |
|
Thank you @cmenning. |
…ent is a subrequest (cmenning)
This PR was merged into the 2.3 branch.
Discussion
----------
[HttpKernel] Remove _path from query parameters when fragment is a subrequest
| Q | A
| ------------- | ---
| Bug fix? | Yes
| New feature? | No
| BC breaks? | No
| Deprecations? | No
| Tests pass? | Yes
| Fixed tickets |
| License | MIT
| Doc PR |
Prior to 2.3.29, all requests to the ESI fragment path ("/_fragment" by default) would have the "_path" query parameter removed. This held true whether an external proxy (such as Varnish) handled the request as true ESI, or whether the Symfony kernel was mocking ESI behavior and inlining the subrequest.
Once the "_controller" check was added in 2.3.29, the "_path" query parameter was only removed on master requests (such as those coming from an external proxy) and not subrequests, leading to differing behavior in production and development settings.
Commits
-------
c374420 Remove _path from query parameters when fragment is a subrequest and request attributes are already set Added tests for _path removal in FragmentListener
Prior to 2.3.29, all requests to the ESI fragment path ("/_fragment" by default) would have the "_path" query parameter removed. This held true whether an external proxy (such as Varnish) handled the request as true ESI, or whether the Symfony kernel was mocking ESI behavior and inlining the subrequest.
Once the "_controller" check was added in 2.3.29, the "_path" query parameter was only removed on master requests (such as those coming from an external proxy) and not subrequests, leading to differing behavior in production and development settings.