hook_page_attachments() puts the current user's subscription into drupalSettings.subscriptionManager.currentUser.subscription on every page: plan_id, status, connector_plugin_id, the remote subscription_id and expiry. It declares no cacheability for that. Dynamic Page Cache stores a page by route and the cache contexts the page declares — the required ones are language, theme and permissions — so, on a route whose other content does not vary by user, the first authenticated user's subscription blob is stored with the page and served to every later user with the same permissions. On top of the leak, a subscription made after the page was cached never appears in the blob until the entry expires.
Reproduced in a browser test: two users with the same permissions request the same route (a 404 page, in Stark); the second receives the first one's remote subscription ID. On a real site most routes happen to carry a user context from something else — the account menu's Join/Upgrade link, the toolbar — which is why it has gone unnoticed, but nothing guarantees that, and a theme without the account menu, or a route rendered without blocks, exposes it.
Affects every release that has the attachment, which is all of them.
Remove the attachment and its hook. A front end that wants the current user's subscription calls api/my-subscription, which returns the same fields and more and is uncacheable by design. Declaring the attachment per user would fix the leak but cost every authenticated page the user cache context and a subscription lookup, for data nothing in the module reads.
The MR carries the browser test that demonstrated the leak in its history.
None.
drupalSettings.subscriptionManager.currentUser.subscription is removed. Use GET /subscription-manager/api/my-subscription.
None.
Show commandsStart 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 #2
colanImplemented in the MR, in three commits, with a change of plan from the summary.
The first two commits do what the summary says: declare the
usercache context and thesubscription_listtag on the attachment, with a browser test — Dynamic Page Cache on, two users with the same permissions on the same 404 page in Stark — that fails on 1.0.x with the second user receiving the first one's remote subscription ID, and passes with the fix.The third commit removes the attachment instead. Nothing in the module reads
drupalSettings.subscriptionManager, and the known connectors and sites do not either;api/my-subscriptionalready serves the same fields and more, uncacheably, to anyone with Manage own subscriptions. Kept and made cacheable, the blob would have cost every authenticated page theusercache context — one Dynamic Page Cache entry per user per route, all of them invalidated whenever any subscription changes — plus a subscription lookup per page render (#3616936: SubscribeMenuLink declares the user.roles cache context but varies per user, leaking Join/Upgrade labels across users trimmed that lookup; this removes it). That is a high price for a duplicate. Removing it closes the whole class of this bug rather than one instance. It is a breaking change for site JavaScript that read the setting, which is why it belongs in beta7 with the other breaking changes rather than later; the change record says what to fetch instead.Gone with it: the
hook_page_attachments()implementation and its unused current-user dependency, thePageAttachmentsQueryBudgetTestthat existed to keep the lookup cheap, and the browser test from the second commit, which has nothing left to test. The README's decoupled section points at the API.Suite: 199 Kernel tests, 6 Functional tests; phpcs clean. Change record drafted.
Comment #8
colan