symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.72k stars 9.45k forks source link

[HttpKernel] HttpCache works with cloned request object #23546

Closed leofeyer closed 7 years ago

leofeyer commented 7 years ago
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? yes
Symfony version 3.3.4

In the HttpCache::fetch() method does a $subRequest = clone $request; and passes the $subRequest variable to the forward() method, so all the request attributes end up on the $subRequest object instead of the $request object.

    protected function fetch(Request $request, $catch = false)
    {
        $subRequest = clone $request;

        // send no head requests because we want content
        if ('HEAD' === $request->getMethod()) {
            $subRequest->setMethod('GET');
        }

        // avoid that the backend sends no content
        $subRequest->headers->remove('if_modified_since');
        $subRequest->headers->remove('if_none_match');

        $response = $this->forward($subRequest, $catch);  // <-- $subRequest instead of $request

        if ($response->isCacheable()) {
            $this->store($request, $response);
        }

        return $response;
    }

This causes the request object to be empty when passed to $kernel->terminate($request, $response).

Here is the request object without using the HTTP cache:

Is this the expected behavior when using the HTTP cache? Because some of our kernel.terminate event listeners need to read the request attributes. Also, it seems kind of inconsistent to me.

nicolas-grekas commented 7 years ago

I think it's the expected behavior: the cache middleware shouldn't have access to the parameters set by the app. That's the behavior of using eg Varnish as another middleware caching layer, we should not diverge from this reference.

fabpot commented 7 years ago

Being as close as possible as a reverse proxy that would be in front of an app is indeed the goal here.