bug #35304 [HttpKernel] Fix that no-cache MUST revalidate with the origin (mpdude)

fabpot · fabpot · commit 764c91bd9f99 · 2020-01-11T08:35:43.000+01:00
This PR was merged into the 3.4 branch. Discussion ---------- [HttpKernel] Fix that no-cache MUST revalidate with the origin | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | From [RFC 7234 Section 5.2.2](https://tools.ietf.org/html/rfc7234#section-5.2.2) > The "no-cache" response directive indicates that the response MUST NOT be used to satisfy a subsequent request without successful validation on the origin server. This allows an origin server to prevent a cache from using it to satisfy a request without contacting it, even by caches that have been configured to send stale responses. This is unconditional – the response must be revalidated right away. (`must-revalidate`, to the contrary, requires revalidation only once the response has become stale.) Commits ------- c8bdcb3 Fix that no-cache requires positive validation with the origin, even for fresh responses
diff --git a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
@@ -323,6 +323,10 @@ protected function lookup(Request $request, $catch = false)
             return $this->validate($request, $entry, $catch);
         }
 
+        if ($entry->headers->hasCacheControlDirective('no-cache')) {
+            return $this->validate($request, $entry, $catch);
+        }
+
         $this->record($request, 'fresh');
 
         $entry->headers->set('Age', $entry->getAge());
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
@@ -443,6 +443,22 @@ public function testCachesResponsesWithExplicitNoCacheDirective()
         $this->assertTrue($this->response->headers->has('Age'));
     }
 
+    public function testRevalidatesResponsesWithNoCacheDirectiveEvenIfFresh()
+    {
+        $this->setNextResponse(200, ['Cache-Control' => 'public, no-cache, max-age=10', 'ETag' => 'some-etag'], 'OK');
+        $this->request('GET', '/'); // warm the cache
+
+        sleep(5);
+
+        $this->setNextResponse(304, ['Cache-Control' => 'public, no-cache, max-age=10', 'ETag' => 'some-etag']);
+        $this->request('GET', '/');
+
+        $this->assertHttpKernelIsCalled(); // no-cache -> MUST have revalidated at origin
+        $this->assertTraceContains('valid');
+        $this->assertEquals('OK', $this->response->getContent());
+        $this->assertEquals(0, $this->response->getAge());
+    }
+
     public function testCachesResponsesWithAnExpirationHeader()
     {
         $time = \DateTime::createFromFormat('U', time() + 5);

Original file line number	Diff line number	Diff line change
`@@ -323,6 +323,10 @@ protected function lookup(Request $request, $catch = false)`
`323`	`323`	`return $this->validate($request, $entry, $catch);`
`324`	`324`	`}`
`325`	`325`
	`326`	`+ if ($entry->headers->hasCacheControlDirective('no-cache')) {`
	`327`	`+ return $this->validate($request, $entry, $catch);`
	`328`	`+ }`
	`329`	`+`
`326`	`330`	`$this->record($request, 'fresh');`
`327`	`331`
`328`	`332`	`$entry->headers->set('Age', $entry->getAge());`