[HttpKernel] Fix response always private and overwritten if session is started

[stixx]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#26769
License	MIT

This PR resolves the bug where the response caching directives where always overwritten in the master request by the listener even if the controller response was trying to set caching directives. This fix adds an additional check in the listener to check if the response was cacheable at first.

[nicolas-grekas]

This has been solved differently in #26681: the current behavior is considered correct (see links in linked PR), yet in very specific cases (e.g. session groups) it's OK to cache session accessing data. Thus 4.1 will provide a way to force caching session-using responses, as a new feature.

[stixx]

That solution isn't available yet in Symfony 3.4 as far as I can see. What do you require for this solution to come for 3.4?:

• Upgrade to Symfony 4.1 and maintain current behaviour;
• Use a decorator in my own project as proposed in Allow to easily ask Symfony not to set a response to private automatically #26681;
• Implement new behaviour from Allow to easily ask Symfony not to set a response to private automatically #26681 for Symfony 3.4 in a (new) PR;

[nicolas-grekas]

#26681 is a new feature, so cannot go on 3.4. Options 1. or 2. are the ones.

[HypeMC]

@nicolas-grekas I'm really disappointed to hear this. I just upgraded Symfony from 3.3 to 3.4 & ESI caching completely stopped working. I understand that no new features will be added to 3.4 anymore, but this seems to go against the BC promise. Would be great if some fix was provided for 3.x. Dunno, maybe I'm wrong about the BC promise, but it looks like a big change for a minor version.

[EmmanuelVella]

@nicolas-grekas From symfony 3.4, current doc won't work if the firewall is enabled on theses actions (http://symfony.com/doc/3.4/http_cache/esi.html).

So what is the way to make it work in this case ? Excluding corresponding actions in the security.yml file ? This would be hard to maintain when having, for example, many actions and using i18n routes.

[nicolas-grekas]

@EmmanuelVella if you mean having cached fragments while using HttpCache, there is a PR fixing it in #27467 when the fragments don't use the session. If you mean having cached fragments behind a stateful firewall, then most of the time these are not cacheable. At least they cannot be considered cacheable by default yes.