8000 [HttpKernel] Add a `noStore` argument to the `#` attribute by smnandre · Pull Request #59301 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpKernel] Add a noStore 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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 2, 2025
Merged

[HttpKernel] Add a noStore argument to the # attribute #59301

merged 1 commit into from
Jan 2, 2025

Conversation

smnandre
Copy link
Member
Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #...
License MIT

This PR introduces a noStore argument to the #[Cache] attribute, allowing controllers to easily set the no-store directive.

use Symfony\Component\HttpKernel\Attribute\Cache;

#[Cache(noStore: true)]
final class MyController
{
    public function __invoke(): Response
    {
        // This response will NOT be stored in ANY cache

When set to true, it also supersedes the public / private value.

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-cache don’t stop this behavior. The only reliable way to disable the bfcache across all major browsers is by using the no-store directive.

use Symfony\Component\HttpKernel\Attribute\Cache;

final class MyController
{
    #[Cache(public: false)]
    public function private(): Response
    {
        // ❌ This page can (and probably will) be cached in the browser bfc
    }
    
    #[Cache(noStore: true)]
    public function notStored(): Response
    {
        // ✅ This page will NOT be cached -- not even in the browser bfc
    }
}

The HTTP cache documentation states that all options available for the Response::setCache() method can also be used with the #[Cache] attribute. However, the no-store option 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:

@carsonbot carsonbot added this to the 7.3 milestone Dec 26, 2024
@carsonbot carsonbot changed the title [HttpKernel] Add a noStore argument to the #[Cache] attribute [Cache][HttpKernel] Add a noStore argument to the # attribute Dec 26, 2024
@chalasr chalasr changed the title [Cache][HttpKernel] Add a noStore argument to the # attribute [Cache][HttpKernel] Add a noStore argument to the #[Cache] attribute Dec 26, 2024
@carsonbot carsonbot changed the title [Cache][HttpKernel] Add a noStore argument to the #[Cache] attribute [Cache][HttpKernel] Add a noStore argument to the # attribute Dec 29, 2024
@smnandre smnandre requested a review from GromNaN December 29, 2024 18:42
@smnandre
Copy link
Member Author

When set to true, it also supersedes the public / private value.

I expected a hotter debate on this to be honest.... 😅 (not that i want a debate)

@fabpot fabpot force-pushed the cache-no-store branch from bfddf54 to 8000 ecc8c33 Compare January 2, 2025 11:57
@fabpot
Copy link
Member
fabpot commented Jan 2, 2025

Thank you @smnandre.

@fabpot fabpot merged commit 78648f0 into symfony:7.3 Jan 2, 2025
1 check passed
@fabpot fabpot mentioned this pull request Jan 2, 2025
Closed
@OskarStark OskarStark changed the title [Cache][HttpKernel] Add a noStore argument to the # attribute [Cache][HttpKernel] Add a noStore argument to the #[Cache] attribute Jan 2, 2025
@carsonbot carsonbot changed the title [Cache][HttpKernel] Add a noStore argument to the #[Cache] attribute [HttpKernel] Add a noStore argument to the # attribute Mar 21, 2025
@94noni 94noni mentioned this pull request Mar 27, 2025
Open
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
@alexander-schranz alexander-schranz mentioned this pull request May 28, 2025
Merged
fabpot added a commit that referenced this pull request May 30, 2025
@fabpot
feature #60569 [HttpKernel] Do not superseed private cache-control wh…
8c89e4c 
…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       | MIT

I don't think its a good idea to superseed the private cache control via the new noStore option
* #59301

If 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 request May 30, 2025
@fabpot @nicolas-grekas
bug #60569 [HttpKernel] Do not superseed private cache-control when n…
158dff8 
…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       | MIT

I don't think its a good idea to superseed the private cache control via the new noStore option
* #59301

If 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@GromNaN GromNaN GromNaN approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Feature HttpKernel Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
6 participants
@smnandre @fabpot @GromNaN @chalasr @nicolas-grekas @carsonbot
0