bug #60569 [HttpKernel] Do not superseed private cache-control when no-store is set (alexander-schranz)

fabpot · nicolas-grekas · commit 158dff810615 · 2025-05-30T11:56:00.000+02:00
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
diff --git a/src/Symfony/Component/HttpKernel/EventListener/CacheAttributeListener.php b/src/Symfony/Component/HttpKernel/EventListener/CacheAttributeListener.php
@@ -165,7 +165,6 @@ public function onKernelResponse(ResponseEvent $event): void
             }
 
             if (true === $cache->noStore) {
-                $response->setPrivate();
                 $response->headers->addCacheControlDirective('no-store');
             }
 
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/CacheAttributeListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/CacheAttributeListenerTest.php
@@ -102,18 +102,18 @@ public function testResponseIsPublicIfConfigurationIsPublicTrueNoStoreFalse()
         $this->assertFalse($this->response->headers->hasCacheControlDirective('no-store'));
     }
 
-    public function testResponseIsPrivateIfConfigurationIsPublicTrueNoStoreTrue()
+    public function testResponseKeepPublicIfConfigurationIsPublicTrueNoStoreTrue()
     {
         $request = $this->createRequest(new Cache(public: true, noStore: true));
 
         $this->listener->onKernelResponse($this->createEventMock($request, $this->response));
 
-        $this->assertFalse($this->response->headers->hasCacheControlDirective('public'));
-        $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
+        $this->assertTrue($this->response->headers->hasCacheControlDirective('public'));
+        $this->assertFalse($this->response->headers->hasCacheControlDirective('private'));
         $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));
     }
 
-    public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue()
+    public function testResponseKeepPrivateNoStoreIfConfigurationIsNoStoreTrue()
     {
         $request = $this->createRequest(new Cache(noStore: true));
 
@@ -124,14 +124,14 @@ public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue()
         $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));
     }
 
-    public function testResponseIsPrivateIfSharedMaxAgeSetAndNoStoreIsTrue()
+    public function testResponseIsPublicIfSharedMaxAgeSetAndNoStoreIsTrue()
     {
         $request = $this->createRequest(new Cache(smaxage: 1, noStore: true));
 
         $this->listener->onKernelResponse($this->createEventMock($request, $this->response));
 
-        $this->assertFalse($this->response->headers->hasCacheControlDirective('public'));
-        $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
+        $this->assertTrue($this->response->headers->hasCacheControlDirective('public'));
+        $this->assertFalse($this->response->headers->hasCacheControlDirective('private'));
         $this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));
     }
 

Original file line number	Diff line number	Diff line change
`@@ -165,7 +165,6 @@ public function onKernelResponse(ResponseEvent $event): void`
`165`	`165`	`}`
`166`	`166`
`167`	`167`	`if (true === $cache->noStore) {`
`168`		`- $response->setPrivate();`
`169`	`168`	`$response->headers->addCacheControlDirective('no-store');`
`170`	`169`	`}`
`171`	`170`
Original file line number	Diff line number	Diff line change
`@@ -102,18 +102,18 @@ public function testResponseIsPublicIfConfigurationIsPublicTrueNoStoreFalse()`
`102`	`102`	`$this->assertFalse($this->response->headers->hasCacheControlDirective('no-store'));`
`103`	`103`	`}`
`104`	`104`
`105`		`- public function testResponseIsPrivateIfConfigurationIsPublicTrueNoStoreTrue()`
	`105`	`+ public function testResponseKeepPublicIfConfigurationIsPublicTrueNoStoreTrue()`
`106`	`106`	`{`
`107`	`107`	`$request = $this->createRequest(new Cache(public: true, noStore: true));`
`108`	`108`
`109`	`109`	`$this->listener->onKernelResponse($this->createEventMock($request, $this->response));`
`110`	`110`
`111`		`- $this->assertFalse($this->response->headers->hasCacheControlDirective('public'));`
`112`		`- $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));`
	`111`	`+ $this->assertTrue($this->response->headers->hasCacheControlDirective('public'));`
	`112`	`+ $this->assertFalse($this->response->headers->hasCacheControlDirective('private'));`
`113`	`113`	`$this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));`
`114`	`114`	`}`
`115`	`115`
`116`		`- public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue()`
	`116`	`+ public function testResponseKeepPrivateNoStoreIfConfigurationIsNoStoreTrue()`
`117`	`117`	`{`
`118`	`118`	`$request = $this->createRequest(new Cache(noStore: true));`
`119`	`119`
`@@ -124,14 +124,14 @@ public function testResponseIsPrivateNoStoreIfConfigurationIsNoStoreTrue()`
`124`	`124`	`$this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));`
`125`	`125`	`}`
`126`	`126`
`127`		`- public function testResponseIsPrivateIfSharedMaxAgeSetAndNoStoreIsTrue()`
	`127`	`+ public function testResponseIsPublicIfSharedMaxAgeSetAndNoStoreIsTrue()`
`128`	`128`	`{`
`129`	`129`	`$request = $this->createRequest(new Cache(smaxage: 1, noStore: true));`
`130`	`130`
`131`	`131`	`$this->listener->onKernelResponse($this->createEventMock($request, $this->response));`
`132`	`132`
`133`		`- $this->assertFalse($this->response->headers->hasCacheControlDirective('public'));`
`134`		`- $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));`
	`133`	`+ $this->assertTrue($this->response->headers->hasCacheControlDirective('public'));`
	`134`	`+ $this->assertFalse($this->response->headers->hasCacheControlDirective('private'));`
`135`	`135`	`$this->assertTrue($this->response->headers->hasCacheControlDirective('no-store'));`
`136`	`136`	`}`
`137`	`137`