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

[HttpFoundation] Support for RFC9213 Targeted HTTP Cache Control #47288

Open
alexander-schranz opened this issue Aug 16, 2022 · 4 comments
Open

Comments

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Aug 16, 2022

Description

There is currently a new RFC published for cache-control headers specific for CDN's. It was created by 3 authors working at cloudflare, akamai and fastly.

https://datatracker.ietf.org/doc/rfc9213/

I think it would be great if the HttpFoundation make it easier to control this headers so we have the same functionality like we have now for the Cache-Control header in the Response Class.

Some more links beside the rfc above:

Example

I think the following methods of the HeaderBag would be effect by an "optional" targeted header variable (CDN-Cache-Control, Fastly-Cache-Control, Cloudflare-Cache-Control):

  • addCacheControlDirective
  • removeCacheControlDirective
  • getCacheControlDirective
  • hasCacheControlDirective

The following method on the Response would also be effected by "optional" header name:

  • setMaxAge
  • setStaleIfError
  • setStaleWhileRevalidate
  • setCache
  • setPrivate
  • setPublic
  • isCacheable
  • setImmutable?

Why at first way it looks like it make sense the following I think have no effect on -cache-control header:

  • setSharedMaxAge (cas aching is set via setMaxAge)

Also the Symfony Cache itself should keep the "-Cache-Control" header in mind. In general it is mostly a CDN-Cache-Control and an own name in case of Symfony it would make sense to call it Symfony-Cache-Control.

This headers make it clearer if the response itself is cachable by a server side cache but maybe not by a client side cache (including private ESI's) currently this kind of controlling was not very transparent and FOSHttpCache did workaround by a custom X-Reverse-Proxy-TTL which did not support other things like stale-if-error, statle-while-revalidate, ... Original discussion started there some time ago before the RFC was available: FriendsOfSymfony/FOSHttpCache#455.

PS: As seen above the RFC goes with <Targeted>-Cache-Control while it maybe would first make sense to give only <targeted> into the methods above. It would be better go give the whole header name into it like CDN-Cache-Control this make it possible to support none standardized headers like Surrogate-Control (fastly) and Edge-Control (akamai) also.

Mention some cache related persons: @dbu, @andrerom, @Toflar

@Toflar
Copy link
Contributor

Toflar commented Aug 16, 2022

Looks like cache control header handling should become its own class with an optional prefix. So you can work with multiple CacheControl objects, one having no prefix (default) and others having the vendor prefix (Fastly, Cloudflare...). The Response should then accept and work with a collection of those CacheControl objects. Probably?

@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Aug 16, 2022

@Toflar would be possible to move it to such an object but also array $this->cacheControls[] would work. If we go with an object I personally would keep it @internal and not expose it. But this are implementation details, in general I think you agree on adding RFC9213 functionality to Symfony HttpFoundation itself :)

@Toflar
Copy link
Contributor

Toflar commented Aug 16, 2022

If that RFC becomes a standard, then yes, Symfony should support having vendor specific cache control headers. However, we should try to keep the current API as simple as it is now.

@dbu
Copy link
Contributor

dbu commented Aug 16, 2022

according to https://datatracker.ietf.org/doc/html/rfc7127, a "proposed standard" level RFC is already pretty solid.

i prefer Toflars suggestion to extract the CacheControl into its own class. the HeaderBag mixes the 2 concerns of being a container for all headers and of managing the cache control header. if we separate that, i think it would be cleaner already for the existing functionality. and would allow to handle the targeted cache control headers nicely.

and HttpCache should also implement support for targeted cache control headers, as it is a surrogate cache and not a generic cache.

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

No branches or pull requests

4 participants