[HttpCache] Caches cookies by default

[Toflar]

Symfony version(s) affected: all

Description

HttpCache currently caches responses with cookies by default. This is not wrong as per HTTP specification, responses with Set-Cookie headers are allowed to be cached. However, in most cases we use cookies to identify with a single person only so I guess it's fairly safe to say that the vast majority of developers would expect that cookies can safely contain any user-specific data and it won't be shared no matter the Cache-Control: public and max-age or s-maxage set on the response. HttpCache, however, currently caches a response when the caching headers are set no matter if there are cookies or not. In my opinion, this is a dangerous default setting.
Also given the fact that Symfony ensures a response becomes private automatically when the session is started. This feels inconsistent to me because the session is essentially just a cookie too. I don't see why this should be handled differently for other cookies but I'm happy to hear about the use cases.

How to reproduce

1. composer create-project symfony/skeleton .
2. Edit public/index.php like so:

$kernel = new Kernel($env, $debug);
+$kernel = new \Symfony\Bundle\FrameworkBundle\HttpCache\HttpCache($kernel, $kernel->getCacheDir() . '/http_cache');

3. Add the following controller to src/Controller/CacheAction.php:

<?php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Response;

class CacheAction
{
    public function __invoke(): Response
    {
        $response = new Response('hi');
        $response->setSharedMaxAge(500);
        $response->headers->setCookie(new Cookie('cookie-name', 'i-am-data-that-belongs-to-a-user-only-and-not-in-the-shared-cache-of-a-proxy'));
        return $response;
    }
}

4. Edit the config/routes.yaml as follows:

cache:
    path: /cache
    controller: App\Controller\CacheAction

5. Run the webserver: php -S 127.0.0.1:8000 -t public
6. Visit the new page: http://127.0.0.1:8000/cache
7. You can now inspect var/cache/<dev|prod>/http_cache and see that the cookie is stored alongside the other information and will be shared with any other request.

Possible Solution
I think HttpCache should not cache any response if it contains cookies by default. This should be a sane default behaviour.

Additional context
Nothing really, just wanted to take the opportunity to say thank you to all the people involved in Symfony Flex. Look at how easy it was to create a full application to set up a case for other people to reproduce! 😄 ❤️

/cc @leofeyer @aschempp

[leofeyer]

In my opinion, this is a dangerous default setting.

I agree. That is probably why Varnish never caches requests with a Set-Cookie header, unless explicitly configured (see https://varnish-cache.org/docs/3.0/tutorial/cookies.html).

[nicolas-grekas]

I think it makes sense to have the same defaults as Varnish. Would you like to submit a PR against master? (don't forget to put a note in the UPGRADING files)
I would not make this configurable to keep things simple personally (others might disagree?)

[mpdude]

You are explicitly setting your response to be a shared one. IMO you need to think carefully about your use case before you do that. For the same reason, private is the default.

One case where Cookie and public make sense together is when the 🍪 identifies A/B testing groups.

Side note: Does the HttpCache correctly Support Vary headers?

Update to clarify the use case: You might have a cookie like a-b-testing-group: A and then render two different versions of a page depending on that. You'd want each version to have Cache-Control: s-maxage=300 to keep it in a shared cache (the page is the same for all users in each respective group), but also add Vary: Cookie to make sure the HttpCache keeps two different versions of the page and serves the one that matches a user's current cookie value.

[rpkamp]

The more I think about this, the more I agree with @mpdude that if you explicitly set a response with cookies to public you should be aware of the consequences and HttpCache should just cache the response.

If we don't cache responses with cookies we might as well remove all code concering cookies from HttpCache, and that doesn't really make sense to me.

Adding it as a kind of safe guard option might make sense, but I'm not sure it's needed. I tend to think it's not.

[Toflar]

Side note: Does the HttpCache correctly Support Vary headers?

Yes it does.

So how do we continue here. I can see that having the HttpCache ignore responses with cookie headers is the easy fix but then we have a different handling for regular cookies and the session cookie. I would like to aim for a more consistent solution here. Wdyt?

[mpdude]

IMO you cannot magically tell which cookie is a session cookie and which one is not (not all session cookies need to be managed by Symfony?).

I'd guess it comes down to education and raising awareness for the problem? Maybe update the docs with some helpful hints?

[nicolas-grekas]

Closing as it seems this has been resolved as "nothing to fix".