Repository navigation
Built-in FPC: save cacheable headers before setting no-cache headers (fixes #41643) - #41707
Open
bluefairydevelopment wants to merge 1 commit into
Conversation
…ernel Kernel::process() called setNoCacheHeaders() on the response before getPreparedData() snapshotted its headers for the cache, so every page was saved with Cache-Control: no-store and every cache hit replayed it. Set the no-cache headers on the live response only after the page has been saved. Fixes magento#41643
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (*)
With the built-in full page cache, every cached page is saved with
Cache-Control: no-store, so every cache HIT is sent to the browser (and any CDN in front of Magento) as non-cacheable, even though the page was cacheable and the FPC lookup succeeded.Cause: in
Magento\Framework\App\PageCache\Kernel::process(),$response->setNoCacheHeaders()runs before the cacheable-response branch, andgetPreparedData()then snapshots$response->getHeaders()forfullPageCache->save(). Both act on the same response object, so the snapshot already contains the no-storeCache-Control.Kernel::load()/buildResponse()replay those saved headers on every HIT.This change moves
setNoCacheHeaders()to the end of the cacheable branch, after the page has been saved. The live response still gets its no-cache headers exactly as before; only the saved snapshot changes. Every other step (Set-Cookie and X-Magento-Tags clearing, cookie disabling, the save conditions) keeps its order.Related Pull Requests
None.
Fixed Issues (if relevant)
Manual testing scenarios (*)
system/full_page_cache/caching_application= 1) and production mode, flush the cache.curl -sI https://<store>/ | grep -iE '^(cache-control|x-magento-cache-debug)'X-Magento-Cache-Debug: HITwithCache-Control: max-age=0, must-revalidate, no-cache, no-store.X-Magento-Cache-Debug: HITwithCache-Control: max-age=86400, public, s-maxage=86400.We reproduced the problem on a clean 2.4.9 install with no third-party modules, on both Luma and a Hyvä theme. The same reordering, shipped as a DI preference, has been running since 2026-10-22/23 on two production stores and one staging store, where cache hits now serve public headers.
Unit test:
KernelTest::testProcessSavesHeadersBeforeSettingNoCacheHeadersrecords the headers handed tofullPageCache->save(). Without this change it fails, because the savedCache-Controlismax-age=0, must-revalidate, no-cache, no-store. With it, the test passes. AllFramework/App/Test/Unit/PageCacheandMagento/PageCache/Test/Unittests pass (147 tests), and phpcs (Magento2 standard) is clean on both changed files.Questions or comments
setNoCacheHeaders()previously ran even when the response wasn't saved (e.g. a POST or a 500); it still does, because it stays inside the same outerCache-Control: public … s-maxagecondition.Contribution checklist (*)