Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
bug #27309 Fix surrogate not using original request (Toflar)
This PR was submitted for the 3.4 branch but it was squashed and merged into the 2.8 branch instead (closes #27309). Discussion ---------- Fix surrogate not using original request | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Warning: This might need some close attention. It took me hours to wrap my head around that issue :-) So the problem is that `HttpCache::forward()` (or essentially any part in your application) can modify the `$request` that is passed to the `HttpCache::handle()` and the surrogate can never access the original request using `HttpCache::getRequest()` anymore. Example: * Main request (GET `/foobar`) * It's not in the cache, so `HttpCache::forward()` modifies `REMOTE_ADDR` to `127.0.0.1` and adds the `X-Forwarded-For` header. * The request is sent to the application and any e.g. `kernel.request` listener might modify the `$request` further. * Now the `/foobar` route returns `text/html` that contains some `<esi src="=/fragment_path"` tag. * `HttpCache` has an instance of `SurrogateInterface` so (in our case `Esi`) will be asked to `process()` and then later on `handle()` the `/fragment_path` request. For that, `Esi` (or in fact `AbstractSurrogate` uses the following line to create a subrequest and pass it on to the application again: ```php $subRequest = Request::create($uri, Request::METHOD_GET, array(), $cache->getRequest()->cookies->all(), array(), $cache->getRequest()->server->all()); ``` What you can see here, is that it uses `$cache->getRequest()`. And here follows the problem: We did not duplicate (clone) the original request so essentially `$cache->getRequest()` is a reference to the current request that `HttpKernel::forward()` modified and probably any other part of the application did so too. So for example the original `REMOTE_ADDR` (client IP) got lost. What we should do instead is duplicate the original request so the surrogates can actually behave like a real reverse proxy such as Varnish would by keeping all the original request attributes. Commits ------- ab86f43 Fix surrogate not using original request
- Loading branch information