Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpCache] Prefer using public, maxage over s-maxage #12934

Closed
mpdude opened this issue Jan 11, 2020 · 1 comment
Closed

[HttpCache] Prefer using public, maxage over s-maxage #12934

mpdude opened this issue Jan 11, 2020 · 1 comment
Milestone

Comments

@mpdude
Copy link
Contributor

mpdude commented Jan 11, 2020

When describing the HTTP Cache "Expiration" model (at least at https://symfony.com/doc/current/http_cache.html#expiration-caching, https://symfony.com/doc/current/http_cache/expiration.html#expiration-with-the-cache-control-header) we always use s-maxage/setSharedMaxAge(), making it look like a convenient shortcut for public, maxage=..., and suggesting that s-maxage would just provide another maxage value for the shared cache.

In fact, s-maxage has an additional implication: It prevents re-using a response in a stale-if-error scenario (see symfony/symfony#35305).

Here's the relevant parts of the spec.

RFC 7234 Section 5.2.2.9

The "s-maxage" response directive indicates that, in shared caches,
the maximum age specified by this directive overrides the maximum age
specified by either the max-age directive or the Expires header
field. The s-maxage directive also implies the semantics of the
proxy-revalidate response directive.

... where proxy-revalidate in turn implies must-revalidate which says ...

The "must-revalidate" response directive indicates that once it has
become stale, a cache MUST NOT use the response to satisfy subsequent
requests without successful validation on the origin server.

Also, Section 4.2.4 summarizes this as

A cache MUST NOT generate a stale response if it is prohibited by an
explicit in-protocol directive (e.g., by a "no-store" or "no-cache"
cache directive, a "must-revalidate" cache-response-directive, or an
applicable "s-maxage" or "proxy-revalidate" cache-response-directive;
see Section 5.2.2).

My suggestion is to update the documentation to always show the combination of public and maxage (which gives shared expiration caching plus stale-if-error behavior) instead of s-maxage examples.

I can try to make the actual PR, although I am really slow when trying to mangle .rst. But in general – do you understand the specs the same way and is that change a good idea?

@wouterj
Copy link
Member

wouterj commented Jan 21, 2020

Hi @mpdude!

Thanks for your detailed issue description. I would say: Please make the change. HTTP Caching is one of those topics that none of the doc members has much knowledge about, so we rely mostly on community members with expert knowledge to maintain good docs. This exact piece of code was introduced at the very beginning of the documentation (da66900 ), so there is also not much knowledge on why setSharedMaxAge() is used in the first place.

Also, with my little knowledge about HTTP caching, I've come around public, maxage=... way more often than s-maxage. So I would say using public, maxage=... is both more correct (reading the quoted specs) and easier to understand for the normal PHP developer.

If you need any help with the rst syntax, feel free to ask help on slack (#documentation) and don't feel bad if you have to do changes (we can also make the minor fixes while merging if you wish to save time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants