Problem/Motivation
This was originally reported to the security team who decided it could be public
If you have a piece of content which differs among anonymous users for some reason then in Drupal 8 and 9 you could just add the "session" cache context and it would be enough. Anonymous users had random session id, the cache context resolved to a random string and so the render cache ID derived from that differed too and so the render cache never actually hit.
But in Drupal 10, the session id is the empty string, the return value of the cache context is the encoded empty string which is a constant, the render cache ID is constant and so the render cache hits. This leads to information disclosure where the data of the first anonymous user to hit the site after a render cache clear will be served to everyone.
And yes, it can be argued it would've better to be explicit about what makes the content different between anonymous users but there was no need. Now there is. And the depreciation notice or error message does not make this self evident by far -- there's no mention of the session cache context in the first place.
Steps to reproduce
Proposed resolution
Disable caching when the session cache context is present but the session is not started. This matches the old behavior because in D8/D9 every page load got a new session id and so there was never a cache hit.
Remaining tasks
Write a test.
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3440398
Show commands
Start 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:
Comments
Comment #3
larowlanComment #4
ghost of drupal pastComment #6
andypostLooks like duplicate of #2915594: SessionCacheContext class should implement CacheContextInterface
Comment #7
andypostComment #8
ghost of drupal pastThe two should be merged into one, I am not partial which stays alive. That one has a test (yay!) but this has a bugfix of some importance:
so either the test should be moved from there to here or the fix should be moved over there. Either case the test needs to be expanded to cover this fix.
Comment #11
ghost of drupal pastWrong issue, please disregard. This newfangled process completely confuses me. Patches were way easier.
Comment #14
prudloff commentedI suppose we should merge #2915594: SessionCacheContext class should implement CacheContextInterface first then this issue can focus on adding the 0 max age fix when there is no session.