[pp-3] Bubbling of elements' max-age to the page's headers and the page cache

This patch is based upon #131, but ignores cacheable metadata's max-age if it is 0. This should work around the issues like suggested by @Berdir in #146

Turns out, skipping max-age of 0 (that is, handling max-age=0 as permanent in page cache) is not a good solution; if you use max-age to ensure a cached page is rebuild after a certain timestamp, and if a request happens on exactly the expiration time, the response must not be cached (as the max-age is 0). At least in such cases, page cache must not handle max-age=0 as cache permanently.

And since we (currently) cannot determine why max-age was set to 0, I think page cache should never cache responses with a max-age>=0. Even if this means that certain (or all) pages will no longer be cached by the page cache. Note this means that for example multilingual sites using the language switcher block ( 📌 Make language switcher block cacheable Postponed ) no longer have their pages cached by the page cache module, but at least the page cache won't be incorrectly caching pages permanently that have a max-age>=0. Also, those pages will probably still be cached by the dynamic page cache.

I think we should deprecate using the Expires header as the expire time for the page cache and just use the max-age of the cacheable metadata of the response. There are several places (like access checks) that don't have access to the final reponse and can only pass cacheable metadata.

I also think this issue is actually about two different issues:

1. Page cache incorrectly ignoring the max-age of the cacheable metadata of the response (patch #131)
2. The Cache-Control response header not containing/reflecting the max-age of the cacheable metadata of the response (patch #148 or something like cache contol override module)

For now, if your site depends on time-based cache invalidations you at least need the patch from 🐛 Stacked caches result in max-age drift in caches Active , even if your site is not using the page_cache module.

If you are using the page_cache module, you also need the patch from #131 as a complete solution.

If you are not using the page_cache module (but are using another pubic/managed caching solution, like a reverse proxy like Varnish), the patch from #148 might work.

The Cache Control Override module can be used additionally if you want to set a minimum and/or maximum on the max-age of the Cache-Control response header.

This patch is a combination of the patch in #131 and 🐛 Stacked caches result in max-age drift in caches Active for usage in sites <=D10.2

Patch #154 incorrectly did request_time - expire instead of expire - request_time to recalculate mag-age

I'm still subscribed to this issue 8 years later and I re-read the duplicate I created → and I'm still thinking about this:

There are existing issues for this, it's mostly by design and I'm not sure if it can be changed. It would result in many pages no longer being cacheable that currently are, for example as forms declare themself as uncacheable.

#2732129-4: FinishResponseSubscriber::setResponseCacheable() does not respect Cacheable Metadata for Cache-Control header →

And now I'm wondering.... so what? Like if a bunch of things become uncachable in Drupal 11 and we go and fix those things (individually) in 11.1+ that seems... fine? Am I missing something here?

maybe consider my comment once again ?
https://www.drupal.org/project/drupal/issues/2352009#comment-12814040 →

Committed #155 to an issuefork so tests actually run on that code.

i think you won't be able to solve such problems without client side javascript code that updates part of the page outside the context of the whole page caching .

That seems fine?

In my mind, undercaching is always prefered to overcaching. In an ideal world, it would be perfectly cached, but if the form (or whatever) declares that the whole page is uncachable because of it's inclusion, then.... it is what it is. The only thing you can do is refactor that element to be cacheable (i.e. generate the dynamic bits with JS and/or WebAssembly)

Or we figure out that we don't actually need to declare these things as completely uncachable anymore.

For instance, what if we decide to rely on SameSite=Lax rather than using our custom CSRF protection? This is what other projects like Next.js do and maybe that would be fine for us as well?

CSRF protection doesn't kick in unless you have a session, in which case page caching doesn't happen anyway. However if forms are uncacheable purely because they might have CSRF protection, then that would be breaking page caching for no reason then.

And now I'm wondering.... so what? Like if a bunch of things become uncachable in Drupal 11.0 and we go and fix those things (individually) in 11.1+ that seems... fine?

I have personally seen multiple sites that only stay online because of Drupal's internal page cache or sometimes because they cache pages in a CDN. If we suddenly make those sites uncacheable, they will go down. Something to test would be this patch + a site with anonymous comments enabled (and the form displayed on node pages) to see if those pages still go into the page cache. It would be OK if some poorly behaving contrib modules break the page cache after this lands, although we still might want to make the behaviour configurable as a first step, but not if lots of pages output by core do.

📌 Form tokens are now rendered lazily, allow forms to opt in to be cacheable Needs review would make forms render cacheable.

Committed #155 to an issuefork so tests actually run on that code.

Made outdated patches hidden, MR#8155 contains the latest changes _always_.

Read through chunks of this issue again and as far as I can tell two separate things are being discussed here and while both of them are related to the cache max-age and its (lack of) bubbling, only one of them seems to be the cause for the seeming "intractability" of this issue:

1. Things having an explicit nonzero, finite max-age
2. Things setting their max-age to 0 to declare themselves as uncacheable

While I would absolutely applaud also getting the second use-case somehow properly integrated with page cache it seems getting the first use-case to work is technically just as simple but does not have any of the repercussions in terms of page cache integration that the second one does.

So I would like to propose to solve that issue first, i.e. to bubble nonzero max-ages into the page cache and explicitly leave the "max-age = 0"-case as a todo for the future.

The only place in core we ever set a nonzero, finite max-age, as far as I know, is the time-based caching plugin for views, so the only affect this would have in core is those views actually being properly expired by page cache. But the benefit would be that any contrib or custom modules that either (knowingly or unknowingly) do not work with Page Cache or somehow have to employ hacks to set the response expiry manually will now work with Page Cache automatically (and without any hacks).

Would love to hear if people agree with the splitting up of this problem and if so, whether or not the first (nonzero) part should happen in this issue and the zero part be handled in a follow-up or if we should keep the zero part here and split out the nonzero part.

I do not have all the specific elements in mind, but generally, yes, I'm in favor of splitting this kind of big old issue in several smaller ones, so that they can be reviewed more easily. That's the kind of practice encouraged by core reviewers: https://www.youtube.com/watch?v=_uCfmFH4rUs

I think 📌 Make language switcher block cacheable Postponed could be handled without evaluating the render cache i think. Hoepfully we can get some activity there.

For this issue, we should try and keep the blockers as small as possible, seems it is a pretty short list.

This patch contains a snapshot of the current state of the MR so it can be safely used with composer-patches.