bug #43998 [HttpKernel] [HttpCache] Don't throw on 304 Not Modified (aleho)

fabpot · fabpot · commit fa063a1e2851 · 2022-07-20T22:06:34.000+02:00
This PR was merged into the 4.4 branch. Discussion ---------- [HttpKernel] [HttpCache] Don't throw on 304 Not Modified | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fixes #43997 | License | MIT | Doc PR | ~ If the response cache keeps a `Last-Modified` header clients will request with `If-Modified-Since`. The surrogate will not handle a `304 Not Modified` correctly, resulting in a 500 and a failed request. This fixes that request / response cycle, as observed in testing PR #42355. Commits ------- d27f02a [HttpKernel] [HttpCache] Don't throw on 304 Not Modified
diff --git a/src/Symfony/Component/HttpKernel/HttpCache/AbstractSurrogate.php b/src/Symfony/Component/HttpKernel/HttpCache/AbstractSurrogate.php
@@ -95,7 +95,7 @@ public function handle(HttpCache $cache, $uri, $alt, $ignoreErrors)
         try {
             $response = $cache->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);
 
-            if (!$response->isSuccessful()) {
+            if (!$response->isSuccessful() && Response::HTTP_NOT_MODIFIED !== $response->getStatusCode()) {
                 throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %d).', $subRequest->getUri(), $response->getStatusCode()));
             }
 
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/EsiTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/EsiTest.php
@@ -221,6 +221,15 @@ public function testHandleWhenResponseIsNot200AndAltIsPresent()
         $this->assertEquals('bar', $esi->handle($cache, '/', '/alt', false));
     }
 
+    public function testHandleWhenResponseIsNotModified()
+    {
+        $esi = new Esi();
+        $response = new Response('');
+        $response->setStatusCode(304);
+        $cache = $this->getCache(Request::create('/'), $response);
+        $this->assertEquals('', $esi->handle($cache, '/', '/alt', true));
+    }
+
     protected function getCache($request, $response)
     {
         $cache = $this->getMockBuilder(HttpCache::class)->setMethods(['getRequest', 'handle'])->disableOriginalConstructor()->getMock();

Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ public function handle(HttpCache $cache, $uri, $alt, $ignoreErrors)`
`95`	`95`	`try {`
`96`	`96`	`$response = $cache->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);`
`97`	`97`
`98`		`- if (!$response->isSuccessful()) {`
	`98`	`+ if (!$response->isSuccessful() && Response::HTTP_NOT_MODIFIED !== $response->getStatusCode()) {`
`99`	`99`	`throw new \RuntimeException(sprintf('Error when rendering "%s" (Status code is %d).', $subRequest->getUri(), $response->getStatusCode()));`
`100`	`100`	`}`
`101`	`101`