Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[HttpKernel] Add anoStore argument to the# attribute#59301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
noStore argument to the#[Cache] attributenoStore argument to the# attributenoStore argument to the# attributenoStore argument to the#[Cache] attributesrc/Symfony/Component/HttpKernel/Tests/EventListener/CacheAttributeListenerTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
chalasr approved these changesDec 29, 2024
noStore argument to the#[Cache] attributenoStore argument to the# attribute
I expected a hotter debate on this to be honest.... 😅 (not that iwant a debate) |
GromNaN approved these changesDec 29, 2024
fabpot approved these changesJan 2, 2025
Thank you@smnandre. |
78648f0 intosymfony:7.3 1 check passed
Uh oh!
There was an error while loading.Please reload this page.
noStore argument to the# attributenoStore argument to the#[Cache] attributenoStore argument to the#[Cache] attributenoStore argument to the# attribute Merged
fabpot added a commit that referenced this pull requestMay 30, 2025
…en no-store is set (alexander-schranz)This PR was submitted for the 7.3 branch but it was squashed and merged into the 7.4 branch instead.Discussion----------[HttpKernel] Do not superseed private cache-control when no-store is set| Q | A| ------------- | ---| Branch? | 7.3| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->| License | MITI don't think its a good idea to superseed the private cache control via the new noStore option*#59301If somebody want to set it to `private` they should explicit do it. via `#[Cache(private: true, noStore: true)]`. I would avoid this non transparent changes in general.I had usecases in the past where the response is still public for the symfony cache and varnish public and the no store was only for the third party caches and in browser caches. This specially come into play with usage of `ESI` where the general page is cached, but no-store set to not allow back forwards caches, because of the ESI content./cc `@smnandre`Commits-------7e6e33e [HttpKernel] Do not superseed private cache-control when no-store is set
nicolas-grekas pushed a commit that referenced this pull requestMay 30, 2025
…o-store is set (alexander-schranz)Discussion----------[HttpKernel] Do not superseed private cache-control when no-store is set| Q | A| ------------- | ---| Branch? | 7.3| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues | -| License | MITI don't think its a good idea to superseed the private cache control via the new noStore option*#59301If somebody want to set it to `private` they should explicit do it. via `#[Cache(private: true, noStore: true)]`. I would avoid this non transparent changes in general.I had usecases in the past where the response is still public for the symfony cache and varnish public and the no store was only for the third party caches and in browser caches. This specially come into play with usage of `ESI` where the general page is cached, but no-store set to not allow back forwards caches, because of the ESI content./cc `@smnandre`Commits-------7e6e33e [HttpKernel] Do not superseed private cache-control when no-store is set
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a
noStoreargument to the#[Cache]attribute, allowing controllers to easily set theno-store directive.When set to
true, it also supersedes thepublic/privatevalue.I recently encountered issues with the back-forward cache (bfcache), a browser feature that stores entire pages in memory to make navigating back and forth faster. It can cause problems when pages rely on JavaScript initialization, dynamic content, or state-changing resources. For example, an edit form might reappear after submission just by hitting “Back” (even after a redirection), with no HTTP request being triggered—leading to unexpected behavior and frustrating the user.
Standard cache headers like
Cache-Control: no-cachedon’t stop this behavior. Theonly reliable way to disable the bfcache across all major browsers is by using theno-storedirective.TheHTTP cache documentation states that all options available for the
Response::setCache()method can also be used with the#[Cache]attribute. However, theno-storeoption is currently missing.Note
This is a very "raw" implementation.. not sure about it or potential consequences I might not have considered... but I wanted to start the discussion :)
Resources: