Active
Project:
Drupal core
Version:
main
Component:
theme system
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Jul 2015 at 16:58 UTC
Updated:
25 Jan 2022 at 16:03 UTC
Jump to comment: Most recent
Comments
Comment #1
dawehnerWon't those bits result into potential security issues when smart cache gets in? What happens if we miss some crucial cache context, due to that?
Comment #2
berdir90% of the templates are already cached in entities, blocks and views I'd say. smartcache only changes this for page, html and other templates that are not yet cached.
Not sure how this is supposed to be fixed?
page is a simple boolean. It's impossible for twig to figure out where it is coming from or what cacheability metadata is associated with it? Whatever code provides the value for it *has* to provide cacheability for it, if relevant.
Typical examples for this is a render array variable, e.g. the regions in page.html.twig. I can't imagine a case however, where this could be break, as we already need to have the cacheability of which blocks are in that region anyway. It gets more complicated, if you have something like page, e.g.
if (moon_phase = 'full') then {{ some_region }}.But again, there is absolutely nothing that can be done in a generic way, it's impossible to provide cacheability metadata for a non-render-array variable, whatever preprocess or other hook adds the value for it must provide cacheability for it and add it unconditionally, whether it is used or not.
The only thing I can imagine is collecting cacheability in an if if it is a render array and manually bubble it up, not sure if that is even possible. If not, then this is probably a works-as-designed (more like cant-do-anything-about-it)?
Comment #3
dawehnerCan't we collect all cacheability metadata of all the render arrays passed into a template?
Comment #5
xjmI'm trying to grok what the consequences of this issue are. A specific scenario and the results of that scenario would help.
<h1>and<h2>?secret_userinstead ofpage. Oris_promotion_season. Is there a risk of cache poisoning with output that is only for the secret user, or only for during the promotion season?My gut reaction is that a themer has no reason to expect that putting
{% if secret_user %}in their template would also result in the contents of that section being displayed to non-secret users when the cache is warm. So if that's the bug then it seems really bad, and like we would need to turn off caching for templates with conditionals like that, which would probably be an enormous performance hit.Even if
pageorsecret_useroris_promotion_seasonare simple booleans in the template itself, they are clearly being set in some earlier part of the render process based on things that have cacheability metadata associated with them. So can we somehow ensure that any renderable things we check in setting up the template variables get their metadata attached to the template such that the template's caching must vary and be invalidated by those things? And how do we make it so people remember to do that?Comment #6
lauriiiComment #7
alexpottDiscussed with @xjm, @Cottser, @joelpittet and @laurii. Given that this could cause information disclosure or at the very least displaying the wrong thing we felt this is a major issue. We also discussed whether it was possible for the Twig compiler to detect if and make the result uncacheable or somehow add something that will cause the cache to work as expected.
Comment #17
catch