Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Forms are an important part of Drupal, but most have them have CSRF tokens, which make them uncacheable, but they're not marked as such.
Proposed resolution
Mark forms' CSRF token inputs as uncacheable.
Remaining tasks
Review.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2559011-13.patch | 7.12 KB | wim leers |
Comments
Comment #2
wim leersComment #3
moshe weitzman commentedIts correct.
Comment #4
wim leersAnd test coverage. The test-only patch is also the interdiff.
Comment #5
wim leersIn other words: the patch verifies that
max-age = 0is only set if a form token is indeed present, and *if* a form token is present, that it's alwaysmax-age=0.Comment #6
effulgentsia commentedSince the form token is added conditionally on $user->isAuthenticated(), should we also add the 'user.roles:authenticated' context to $form?
Comment #8
wim leersFirst, a reroll after #2554233: Port Cross-site Request Forgery - Form API fixes from SA-CORE-2015-003 to Drupal 8, which conflicted with this.
Comment #9
wim leers#6: indeed. That was unnecessary in #2526472: Ensure forms are marked max-age=0, but have a way to opt-in to being cacheable, and I failed to analyze the logic used for generating form tokens sufficiently, or I'd have caught that fact.
Comment #11
wim leersComment #13
wim leersGreen.
Comment #14
effulgentsia commentedLooks great. Thanks.
Comment #15
fabianx commentedRTBC + 1
Comment #16
catchCommitted/pushed to 8.0.x, thanks!
Comment #18
wim leersHurray!
This was the last blocker for #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!). It was split off from #2526472: Ensure forms are marked max-age=0, but have a way to opt-in to being cacheable, and uses a more granular, more precise approach to ensure SmartCache doesn't cache forms that should not be cached.
So, I tested #2429617-339: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!) + #13 (the patch here that just got committed), to see if SmartCache would still be green. But, unfortunately, the answer is : #2560959-4: Testing issue for #2429617 is red.
Because of the more granular (and better!) approach taken by this issue compared to #2526472, we need one more step: #2561775: Forms without $form['#action'] set get their action automatically generated based on current path + query args: cacheability metadata is missing. Then forms will have sufficient cacheability metadata. Hopefully see you there!