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
The machine name element passes the entire data from #machine_name
to the frontend via drupalSettings. This exposes unnecessary data e.g. the php object or class which is used for the exists-check and it leads to very large objects in the cache (stored by AssetResolver).
Proposed resolution
Whitelist the required properties for machine-name.js
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#16 | 2614066-16.interdiff.txt | 665 bytes | webflo |
#16 | 2614066-16.patch | 3.81 KB | webflo |
Comments
Comment #2
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #3
dawehnerOh wow.
Given that
is used I'm wondering whether we should validate whether something is an actual instance? It would not be able to call non static code at the moment, right?
Comment #4
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedYeah, actually my patch is wrong. We don't need the class name at all inside of drupalSettings. We should whitelist the properties we need in
machine-name.js
.Comment #5
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #7
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #9
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #10
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #11
jibranI think we can backport this to D7 as well. Can we add some kind of tests for this? Patch itself doesn't touch JS but I think it'd great if JS maintainer can have a look at it so tagging.
Comment #12
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #13
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #15
dawehnerLet's drop those 2 lines then? I think its right to use the value object instead of a mock here.
Comment #16
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #17
dawehnerThank you.
Comment #18
dawehner.
Comment #19
catchFix looks right.
While this changes what's in drupalSettings, it should be impossible for contrib or custom JavaScript to rely on that, so I think it's fine for 8.0.x.
Tagging performance due to the cache entry bloat.
Comment #21
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #23
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #24
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedBack to RTBC.
Comment #25
catchCommitted/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!