Problem/Motivation
Many hosting providers strictly limit response header sizes. The consequence of exceeding such a limitation can be fairly catastrophic, in some cases causing a fatal error.
This module should gracefully handle run-away cache tag header sizes to ensure a uniform and predictable behavior.
Steps to reproduce
Simulate a massive page with enough tags that can exceed hosting infrastructure response header length limitations.
/**
* Implements hook_preprocess_page().
*/
function my_module_preprocess_page(&$variables) {
for ($i = 0; $i < 1000000; ++$i) {
$variables['page']['#cache'][] = "tag-$i";
}
}
Proposed resolution
Add a response subscriber that...
- Checks the
Cache-Tagresponse header length. - Removes the
Cache-Tagheader if it's greater than 16kb and sets a new header,Cloudflare-CDN-Cache-Controltono-cache, no-store
This should allow intermediary caching layers (i.e. Varnish) to still store cached responses, while ensuring the CDN cannot.
Remaining tasks
- Align on approach
- Implement
User interface changes
None
API changes
None
Data model changes
None
Issue fork cloudflare_purger-3607016
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:
Comments
Comment #2
catchI think rather than truncating, we might want to set a short max-age like 5 minutes or 30 minutes or similar. Truncating could in certain circumstances mean that a page stays in the cache a lot longer than it should. You'd hope that not many pages would go over this limit.
#2952277: Minify the cache tags sent in the header is open against purger and has some additional strategies to reduce the number of cache tags (like removing individual node tags when node_list is on the page). There's also https://www.drupal.org/project/cache_tags_simplify. We might want to check whether it's possible to use this module together with cache_tags_simplify so that more aggressive minification can happen.
Comment #3
luke.leberFollowing the chats in slack, I'm sort of leaning more towards matching the header length specification that cloudflare documents (16kb long headers, multiple headers are okay) instead of trying to meet the needs of all hosting providers in this module.
Host-specific header limits might be better to resolve holistically in a response subscriber if there are problems.
Comment #4
bkosborneI agree with #3. To implement that, I think we stick with one Cache-Tag header and ensure the total length of that header and tags is less than 16 KB. If it's greater, then I think we should clear out the header entirely and mark the response as non-cacheable or set to 1 minute. This is similar to what the cache_tags_simplify module does when the cache tags are over its configurable limit.
For #2 - We'd have to do both truncating and short max age wouldn't we? So might as well just clear out all the cache tags IMO. I prefer not creating a difficult to debug situation.
For minifying, we are already hashing the tags and truncating the hash. I think just ensuring compatibility with cache_tags_simplify is the best route here, rather than trying to incorporate further simplification methods in this module. That module uses a response subscriber to alter the cache tags on the response before Purge's subscriber. So we should be good there.
Comment #5
luke.leberOkay, so after some reading (https://developers.cloudflare.com/cache/concepts/cdn-cache-control/#head...)
I THINK that we should be able to simply set the
Cloudflare-CDN-Cache-Controlheader in the event of an overflow. In theory, that should still allow intermediary layers (i.e. a load balancer) to cache the response, but ensure that the CDN edge does not.Comment #6
luke.leberComment #7
luke.leberComment #8
bkosborneso you mean in an overflow, don't set the Cache-Tag header at all. Instead set Cloudflare-CDN-Cache-Control to indicate it’s not cacheable? I think that should work.
Comment #9
luke.leberYeah, exactly.
Comment #10
catchI still need to manually test, but because cache tags simplify uses a response subscriber, I think it will transparently cut down the number of tags on the response before purger and by extension this module sees anything, so we should be 100% compatible, however I'll also try to manually verify that this actually works in practice.
Comment #12
luke.leberI opened a PR, but will have to give it a real world test before moving this to NR. I can do that this weekend on personal hosting, as I'm not allowed to host drupal anywhere but Acquia at work.
Comment #13
luke.leberSliding over to NR.
I was able to confirm that overflows that set the
Cloudflare-CDN-Cache-Controlheader result in aCf-Cache-Statusof BYPASS and the upstreamX-Cache-Hitsfrom Varnish increases on every request, meaning the intermediary layer is working as expected.Comment #14
catchI wasn't sure about max-age: 0 vs the custom cloudflare header, but then thought about it some more and e.g. the custom cloudflare header would allow sites to run the internal page cache module + cloudlfare so that at least they get internal page cache hits if something is uncacheable in cloudflare. So definitely makes sense to only affect cloudflare here.
I double checked the listener priority against upstream purge and cache_tag_simplify issues and it should be fine both before and after. RTBC for me.
Comment #15
bkosborneNot just internal page cache module, but something like Varnish as well.