Built-in FPC: save cacheable headers before setting no-cache headers (fixes #41643)

lib/internal/Magento/Framework/App/PageCache/Kernel.php

@@ -158,7 +158,6 @@ public function process(\Magento\Framework\App\Response\Http $response)
             && preg_match('/public.*s-maxage=(\d+)/', $cacheControlHeader->getFieldValue(), $matches)
         ) {
             $maxAge = (int) $matches[1];
-            $response->setNoCacheHeaders();
             if (($response->getHttpResponseCode() == 200 || $response->getHttpResponseCode() == 404)
                 && !$response instanceof NotCacheableInterface
                 && ($this->request->isGet() || $this->request->isHead())
@@ -179,6 +178,8 @@ public function process(\Magento\Framework\App\Response\Http $response)
                     $maxAge
                 );
             }
+            // Only after the cacheable headers have been saved, so a cache hit replays them.
+            $response->setNoCacheHeaders();
         }
     }
 

lib/internal/Magento/Framework/App/Test/Unit/PageCache/KernelTest.php

@@ -285,6 +285,64 @@ function ($value) {
         $this->kernel->process($this->responseMock);
     }
 
+    /**
+     * The saved page must keep its cacheable headers, so a cache hit does not replay no-store.
+     *
+     * @return void
+     */
+    public function testProcessSavesHeadersBeforeSettingNoCacheHeaders(): void
+    {
+        $noCacheHeadersSet = false;
+        // A fresh mock: the shared one already stubs getHeaders() in setUp().
+        $response = $this->createMock(\Magento\Framework\App\Response\Http::class);
+        $response->method('getHeader')
+            ->willReturnCallback(function ($name) {
+                return $name === 'Cache-Control'
+                    ? CacheControl::fromString('Cache-Control: public, max-age=100, s-maxage=100')
+                    : null;
+            });
+        $response->method('getHttpResponseCode')->willReturn(200);
+        $this->requestMock->method('isGet')->willReturn(true);
+        $response->method('setNoCacheHeaders')
+            ->willReturnCallback(function () use (&$noCacheHeadersSet) {
+                $noCacheHeadersSet = true;
+            });
+        $response->method('getHeaders')
+            ->willReturnCallback(function () use (&$noCacheHeadersSet) {
+                return Headers::fromString(
+                    $noCacheHeadersSet
+                        ? 'Cache-Control: max-age=0, must-revalidate, no-cache, no-store'
+                        : 'Cache-Control: public, max-age=100, s-maxage=100'
+                );
+            });
+        $this->serializer->method('serialize')
+            ->willReturnCallback(function ($value) {
+                return json_encode($value);
+            });
+
+        $savedData = null;
+        $this->fullPageCacheMock->expects($this->once())
+            ->method('save')
+            ->willReturnCallback(function ($data) use (&$savedData) {
+                $savedData = json_decode($data, true);
+                return true;
+            });
+
+        $this->kernel->process($response);
+
+        $this->assertStringContainsString(
+            'public',
+            $savedData['headers']['Cache-Control'],
+            'The cached page should keep its public Cache-Control header.'
+        );
+        $this->assertStringNotContainsString(
+            'no-store',
+            $savedData['headers']['Cache-Control'],
+            'The cached page should not be saved with no-store.'
+        );
+        $this->assertTrue($noCacheHeadersSet, 'The live response should still get no-cache headers.');
+    }
+
     /**
      * @return array
      */