Cache-Control public/private directives set when not always required
Symfony version(s) affected
2.0.0 and greater
Description
This commit introduced a change whereby if the value of the s-maxage directive is not set, the private directive is added to the Cache-Control header (snippet pasted below for reference).
// public if s-maxage is defined, private otherwise
if (!isset($this->cacheControl['s-maxage'])) {
return $header.', private';
}
Additionally, this commit introduced a change whereby if the s-maxage directive is set, the public directive is added as well.
According to RFC 9111, including the s-maxage directive "permits a shared cache to reuse a response to a request containing an Authorization header field subject to the above requirements on maximum age and revalidation". Also, according to the same RFC, the public directive "permits a shared cache to reuse a response to a request containing an Authorization header field".
We can see that as per the RFC, there is an overlap between the s-maxage and public directives and including s-maxage does not necessarily require the use of public. Similarly, the lack of the s-maxage directive does indicate that the response should not be cached by a shared cache.
Therefore, HttpFoundation should not force the use of either the public or private directives when s-maxage is missing or present.
How to reproduce
// First, run "composer require symfony/http-foundation"
// Then, execute this file:
<?php
require_once __DIR__.'/vendor/autoload.php';
use Symfony\Component\HttpFoundation\Response;
// in this scenario, the 'private' directive should not be automatically set:
$response = new Response();
$response->setCache([
'max_age' => 600,
]);
// expected: "max-age=600", got: "max-age=600, private"
var_dump($response->headers->get('Cache-Control'));
// in this scenario, the 'public' directive should not be automatically set:
$response = new Response();
$response->setCache([
's_maxage' => 600,
]);
// expected: "s-maxage=600", got: "public, s-maxage=600"
var_dump($response->headers->get('Cache-Control'));
Possible Solution
Remove the automatic addition of the public and private directives when not explicitly defined (either by calling the setPublic/setPrivate methods or by defining appropriate keys when calling setCache).
Whilst this may be a breaking change as other developers may be relying on this behaviour to exist, a potential configuration flag could be added to allow 'strict' RFC 9111 headers to be generated.
Additional Context
No response
Based on the behavior of the RFC, any response that has s-maxage is effectively public, even if you omit the explicit public keyword (simply check the effect for the cache of having one or the other and you'll see that public, s-maxage=600 and s-maxage=600 produce exactly the same behavior for spec-compliant systems).
However, it was found in the past that returning the public flag explicitly had better interoperability.
Adding private by default when you don't mark as response as public was done to avoid common footguns. The RFC changes the behavior of shared caches based on the presence of the Authorization header in the request. But most websites don't rely on this header for authentication but on session cookies instead.
Forcing all sites to remember to mark their responses as private when they don't intend them to be stored by shared caches would mean that we make it easier to write code with security issues (leaking content of another user that you should not be allowed to see) than secure code. We decided to make the default code secure.
It's true that the s-maxage for the most part is effectively public, though public is slightly more lenient. I suspected that this could have been the case due to interoperability issues with caches in the past.
I would not expect the cache control headers to be set every time a new response is instantiated but that guardrail is a good point. To give a bit more context, after setting cache headers through the HeaderBag, I was still seeing private tacked on to the end of the header value, it took a bit of scratching my head and browsing through the Symfony source code to find the root cause.
The public directive can be opted out of by calling $response->headers->removeCacheControlDirective('public'); after setting the cache options, however, the private directive has no opt-out mechanism due to its implementation. Would you be open to a PR where this would be brought in line with the public directive? So private would be added by default (as it is now) to avoid shooting yourself in the foot, but, you would still be able to opt out by calling say: $response->headers->removeCacheControlDirective('private');.
Do you have an actual use case where you don't want to mark as response as private without marking it as public ?
In consideration of the RFC's guidance, it appears that for the majority of scenarios where leveraging a shared cache for responses is desired, omitting the public directive aligns with best practices ("Note that it is unnecessary to add the public directive to a response that is already cacheable").
Streamlining cache control directives by employing solely max-age=600 for a caching duration of 600 seconds, rather than including max-age=600, s-maxage=600, public could present a more efficient and intuitive approach for developers.
max-age=600, public is enough. Symfony does not force you to set s-maxage when you don't need a different value than the max-age (see $response->setPublic().
And this does not answer my question: do you have an actual use case where using only max-age=600 is necessary ? Theoretical purity does not qualify as a use case.
No. This issue was raised in good faith in order to potentially improve the standards compliance of the Cache-Control header. However, if the Symfony team are happy with the current implementation, feel free to close this issue 👍 .
Hey, thanks for your report! There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?
Friendly reminder that this issue exists. If I don't hear anything I'll close this.
Hey,
I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!