Let's assume you have
a) REST enabled for nodes
b) a hook_node_grants() implementation running on your site
You'll get gazillions of The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: %s. in your logs.
#2626298: REST module must cache only responses to GET requests would solve many of those cases for a).
On top of that there is a general problem. Everytime you load an entity via ::loadByProperties() (which happens in validation and saving of an entity) it adds cache contexts to the render context.
Let's not add those access query tags in ::loadByProperties(). It doesn't make sense from a conceptual point of view.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2729137.patch | 707 bytes | catch |
Comments
Comment #2
dawehnerComment #3
catchIs this all it needs?
Comment #4
dawehnerTheoretically yes, but @berdir argued that existing code might already rely on the access checking.
I disagree fundamentally though. For me loadByProperties is NOT a wrapper for entity query. For me this is just an implementation detail.
Comment #5
catchYes I agree with #4, it's an implementation detail and skipping the access check was forgotten in the implementation.
We probably want a change record, but I think it's far more likely that people are running into data issues (not finding nodes or users they should be then creating duplicates or similar) than relying on the access check.
Comment #7
dawehnerThis was a random failure. Ideally we would backup this with a test.
Comment #11
catchThis got fixed by #2878483: loadEntityByUuid() should skip access checks.