Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
PageCache determines which responses to cache by inspecting the Expires header. (This should be updated to use the Cache-Control header instead, but that's out of scope here.)
Responses that have Expires: Sun, 19 Nov 1978 05:00:00 GMT are meant to not be cached, because it's an expiration date in the past. Yet PageCache caches them permanently:
$expire = ($date > $request_time) ? $date : Cache::PERMANENT;
This has been the case for a very long time, even predating #2527126: Only send X-Drupal-Cache-Tags and -Contexts headers when developer explicitly enables them. Quite possibly the rationale was it's fine to cache uncacheable responses, because cache tags will ensure the responses are invalidated in a timely manner
.
Now, to make matters worse, even when you configure a non-zero "max age" at /admin/config/development/performance, \Drupal\Core\EventSubscriber\FinishResponseSubscriber::setResponseCacheable() will still cause that same Expires header to be set? Why? Because when \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onRespond() calls \Drupal\Core\EventSubscriber\FinishResponseSubscriber::setResponseCacheable(), the latter contains this code:
// HTTP/1.0 proxies do not support the Vary header, so prevent any caching
// by sending an Expires date in the past. HTTP/1.1 clients ignore the
// Expires header if a Cache-Control: max-age directive is specified (see
// RFC 2616, section 14.9.3).
if (!$response->headers->has('Expires')) {
$this->setExpiresNoCache($response);
}
In other words:
- Drupal 8 intentionally disables HTTP/1.0 proxies
- Drupal 8 ships with a HTTP/1.0 reverse proxy that is breaking the spec: Page Cache
Proposed resolution
Make \Drupal\page_cache\StackMiddleware\PageCache a HTTP/1.1 proxy: make it inspect Cache-Control rather than Expires
Remaining tasks
TBD
Remaining tasks
TBD
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2835068-13.patch | 1.06 KB | oknate |
| #12 | 2835068-11.patch | 903 bytes | oknate |
| #9 | 2835068-Pagecache-expires-9.patch | 774 bytes | Nigel Cunningham |
Issue fork drupal-2835068
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #6
Leo Pitt CreditAttribution: Leo Pitt commentedThis seems like quite a major issue - but no activity on it in the past two years ...?
Is it still a current issue?
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous at Ashday Interactive Systems commentedI think it is still current, yes. I continue to find that whenever I want a page to be entirely uncacheable for some reason, I have to both set the max-age to 0 and call the page cache kill switch, even though just setting the max-age should be enough; and when I want a page cache to expire after a particular amount of time, I've been needing to sort of do so with cron and custom cache tags since simply setting max-age doesn't do the trick. My understanding is that this ticket is to resolve this issue and get cache max-age to apply normally to the page cache, the same as cache tags and cache contexts already do.
Comment #9
Nigel CunninghamI'm testing the attached patch in combination with the cache control override module and a couple of patches to it (https://www.drupal.org/files/issues/2019-09-26/cache_control_override_ex... and https://www.drupal.org/files/issues/2020-01-09/cache_control_override-31...), to good effect.
Comment #11
mxr576Comment #12
oknateHere's a patch that turns off caching in the page cache if it's set to not cacheable in FinishResponseSubscriber::onRespond().
It's similar to patch #9, but is more explicit.
The only place that date is used in Drupal core is FinishResponseSubscriber.
UPDATE: the next patch I added adds a more clear note about this.
This will make it more obvious to developers that their caching isn't working properly, for example if they've disabled it!
I tried Wim's suggestion to check the max age, but it was set to a negative number in this case. I like the idea of checking the same date to which the FinishResponseSubscriber sets the expires.
I ran into this with the Sitewide Alert module. https://www.drupal.org/project/sitewide_alert/issues/3189144
If you set expires header on a CacheableJsonResponse returned in a controller, like this:
$response->setExpires($expireDate->getPhpDateTime());but you have set cache.page.max_age to 0 (
<no caching>) at /admin/config/development/performance, the CacheableJsonResponse gets mangled in \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onRespond() and set to not cacheable, but Page Cache caches it anyway with 'expire' set to -1. This breaks the scheduling feature. The solution in my case easy, to set the cache.page.max_age to something other than 0.I was also able to fix it with the kill switch. But I'm glad I figured out that had forgot to set my max-page in the performance settings.
Comment #13
oknateJust adding a little more info in the comment.
Wim, can you add a little more info on this suggestion:
Symfony has a
\Symfony\Component\HttpFoundation\Response::isCacheable()method that looks to me to be perfect here. I don't see it used in Drupal core at all.Perhaps instead of checking the expires we could check if it's cacheable using that method.
Comment #14
oknateWim, I don't think the second part of the description is still true:
When I was testing tonight, with a CacheableJsonResponse from a controller, this wasn't the case. I think as long as it the response is an instance of CacheableResponseInterface, and cache.page.max_age is greater than zero, the expires is not altered.
Comment #16
Anatolij Zajika CreditAttribution: Anatolij Zajika commentedHello everybody. Here is the module that solves this problem and has settings - https://www.drupal.org/project/custom_cache
Comment #20
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedComment #21
smustgrave CreditAttribution: smustgrave at Mobomo commented#13 had many failures and was not ready for review.
Which show this will need it's own test case and work for BC.