From 8e9f3a5479ac73f22d13e7730903dfedd2c1a2fd Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Thu, 23 Jul 2026 10:39:13 +0100 Subject: [PATCH 1/2] don't track error responses in the static cache's url set with the "half measure" `ApplicationCacher`, 404s get cached alongside successful pages. previously, every error response was also tracked in the forever-cached `.urls` set used for wildcard invalidation and background recache warming. on busy public sites, this meant every bot/scanner junk url ever hit (e.g. `.env`/`.php` probes, dead urls) got permanently tracked. when a wildcard invalidation rule matched, `refreshWildcardUrl()` would dispatch a `StaticWarmJob` for every junk url too, flooding `failed_jobs` and delaying real content updates. `ApplicationCacher::cachePage()` now skips adding a url to the tracked set for any non-2xx response, except the shared-error url used by `share_errors`. the 404 response itself is still cached and served correctly on repeat hits, since that lookup is keyed by url hash and doesn't depend on the `.urls` set. --- .../Cachers/ApplicationCacher.php | 22 +++++- tests/StaticCaching/ApplicationCacherTest.php | 75 +++++++++++++++++++ .../HalfMeasureStaticCachingTest.php | 53 +++++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) diff --git a/src/StaticCaching/Cachers/ApplicationCacher.php b/src/StaticCaching/Cachers/ApplicationCacher.php index 92bae1e6747..802a7fcac3c 100644 --- a/src/StaticCaching/Cachers/ApplicationCacher.php +++ b/src/StaticCaching/Cachers/ApplicationCacher.php @@ -3,6 +3,7 @@ namespace Statamic\StaticCaching\Cachers; use Illuminate\Http\Request; +use Illuminate\Http\Response; use Illuminate\Routing\Events\ResponsePrepared; use Illuminate\Support\Facades\Event; use Statamic\Events\UrlInvalidated; @@ -38,8 +39,10 @@ public function cachePage(Request $request, $content) // and other URL characters wouldn't work as a cache key. $key = $this->makeHash($url); - // Keep track of the URL and key the response content is about to be stored within. - $this->cacheUrl($key, ...$this->getPathAndDomain($url)); + if ($this->shouldTrackUrl($url, $content)) { + // Keep track of the URL and key the response content is about to be stored within. + $this->cacheUrl($key, ...$this->getPathAndDomain($url)); + } $key = $this->normalizeKey('responses:'.$key); $value = $this->normalizeContent($content); @@ -61,6 +64,21 @@ public function cachePage(Request $request, $content) }); } + /** + * Determine whether a URL should be tracked in the cacher's URL set. + * + * @param string $url + * @param mixed $content + */ + private function shouldTrackUrl($url, $content): bool + { + if (! $content instanceof Response || $content->isSuccessful()) { + return true; + } + + return str_contains($url, '/__shared-errors/'); + } + /** * Check if a page has been cached. * diff --git a/tests/StaticCaching/ApplicationCacherTest.php b/tests/StaticCaching/ApplicationCacherTest.php index c8b0bfe9f4b..4c0c3617bd8 100644 --- a/tests/StaticCaching/ApplicationCacherTest.php +++ b/tests/StaticCaching/ApplicationCacherTest.php @@ -4,9 +4,12 @@ use Illuminate\Contracts\Cache\Repository; use Illuminate\Http\Request; +use Illuminate\Routing\Events\ResponsePrepared; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Queue; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use Statamic\Console\Commands\StaticWarmJob; use Statamic\Events\UrlInvalidated; use Statamic\StaticCaching\Cacher; use Statamic\StaticCaching\Cachers\ApplicationCacher; @@ -67,6 +70,78 @@ public function checking_if_page_is_cached_then_retrieving_it_will_only_hit_the_ $this->assertEquals('application/html', $cachedPage->headers['Content-Type']); } + #[Test] + public function caching_a_successful_page_tracks_the_url() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::create('http://example.com/about', 'GET'); + + $cacher->cachePage($request, response('about page', 200)); + event(new ResponsePrepared($request, response('about page', 200))); + + $this->assertEquals(['/about'], $cacher->getUrls()->values()->all()); + } + + #[Test] + public function caching_a_404_page_does_not_track_the_url() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::create('http://example.com/this-does-not-exist', 'GET'); + $response = response('not found', 404); + + $cacher->cachePage($request, $response); + event(new ResponsePrepared($request, $response)); + + // The URL isn't tracked in the `.urls` set... + $this->assertEquals([], $cacher->getUrls()->all()); + + // ...but the 404 response is still cached and served directly by URL hash. + $this->assertTrue($cacher->hasCachedPage($request)); + $this->assertEquals('not found', $cacher->getCachedPage($request)->content); + } + + #[Test] + public function caching_a_shared_error_page_is_always_tracked() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::createFrom(Request::create('http://example.com/'))->fakeStaticCacheStatus(404); + $response = response('shared not found', 404); + + $cacher->cachePage($request, $response); + event(new ResponsePrepared($request, $response)); + + $this->assertEquals(['/__shared-errors/'.\Statamic\Facades\Site::current()->handle().'/404'], $cacher->getUrls()->values()->all()); + } + + #[Test] + public function wildcard_refresh_does_not_warm_untracked_404_urls() + { + Queue::fake(); + + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + + $goodRequest = Request::create('http://example.com/rail/one', 'GET'); + $cacher->cachePage($goodRequest, response('one', 200)); + event(new ResponsePrepared($goodRequest, response('one', 200))); + + $junkRequest = Request::create('http://example.com/rail/scanner-junk', 'GET'); + $cacher->cachePage($junkRequest, response('not found', 404)); + event(new ResponsePrepared($junkRequest, response('not found', 404))); + + $cacher->refreshUrls(['/rail/*']); + + Queue::assertPushed(StaticWarmJob::class, function ($job) { + return str_contains((string) $job->request->getUri(), '/rail/one'); + }); + Queue::assertNotPushed(StaticWarmJob::class, function ($job) { + return str_contains((string) $job->request->getUri(), 'scanner-junk'); + }); + } + #[Test] public function invalidating_a_url_removes_the_html_and_the_url() { diff --git a/tests/StaticCaching/HalfMeasureStaticCachingTest.php b/tests/StaticCaching/HalfMeasureStaticCachingTest.php index 8c4b59325f9..3b2fa9c80d1 100644 --- a/tests/StaticCaching/HalfMeasureStaticCachingTest.php +++ b/tests/StaticCaching/HalfMeasureStaticCachingTest.php @@ -3,8 +3,11 @@ namespace Tests\StaticCaching; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Queue; use Orchestra\Testbench\Attributes\DefineEnvironment; use PHPUnit\Framework\Attributes\Test; +use Statamic\Console\Commands\StaticWarmJob; +use Statamic\StaticCaching\Cacher; use Statamic\StaticCaching\Replacer; use Symfony\Component\HttpFoundation\Response; use Tests\FakesContent; @@ -205,6 +208,56 @@ public function nocache_session_is_written_under_the_real_url_for_shared_errors( ); } + #[Test] + public function it_does_not_track_404_urls() + { + \Illuminate\Support\Facades\Cache::flush(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('errors.404', '404 not found'); + + $this->get('/this-does-not-exist')->assertNotFound(); + + $cacher = app(Cacher::class); + $this->assertEquals([], $cacher->getUrls()->all()); + + // The 404 response is still served from the cache on a repeat hit, + // even though it was never added to the tracked `.urls` set. + $response = $this->get('/this-does-not-exist')->assertNotFound(); + $this->assertTrue($response->wasStaticallyCached()); + } + + #[Test] + public function wildcard_invalidation_does_not_warm_untracked_404_urls() + { + \Illuminate\Support\Facades\Cache::flush(); + + Queue::fake(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('default', '{{ title }}'); + $this->viewShouldReturnRaw('errors.404', '404 not found'); + + $this->createPage('about', ['with' => ['title' => 'The About Page']]); + + // A real page, matching the wildcard `/about*` rule below. + $this->get('/about')->assertOk(); + + // A junk URL that also matches the wildcard prefix, but doesn't resolve + // to real content (e.g. a bot/scanner probe under the same path). + $this->get('/about-this-does-not-exist')->assertNotFound(); + + app(Cacher::class)->refreshUrls(['/about*']); + + Queue::assertPushed(StaticWarmJob::class, function ($job) { + return str_contains((string) $job->request->getUri(), '/about') + && ! str_contains((string) $job->request->getUri(), 'this-does-not-exist'); + }); + Queue::assertNotPushed(StaticWarmJob::class, function ($job) { + return str_contains((string) $job->request->getUri(), 'this-does-not-exist'); + }); + } + #[Test] public function it_can_keep_parts_dynamic_using_nocache_tags_in_loops() { From 5c782d528b7f57268ba12c7848a01e2b9231ca45 Mon Sep 17 00:00:00 2001 From: Duncan McClean Date: Wed, 19 Aug 2026 23:34:22 +0200 Subject: [PATCH 2/2] keep tracking error responses and invalidate them on refresh instead of warming untracked error responses were unreachable by `invalidateUrl()` and `flush()`, which only walk the tracked url set, so a cached 404 could be served indefinitely even after content was published at that url. error responses are now tracked again, and the refresh path invalidates them instead of dispatching warm jobs that would fail with the same error. the `ResponsePrepared` listener is also guarded so it only handles the response for the request being cached, since stale listeners in long-running processes would re-store entries with later requests' statuses and headers. Co-Authored-By: Claude Fable 5 --- src/StaticCaching/Cachers/AbstractCacher.php | 21 +++- .../Cachers/ApplicationCacher.php | 53 +++++---- tests/StaticCaching/ApplicationCacherTest.php | 102 ++++++++++++------ .../HalfMeasureStaticCachingTest.php | 33 ++++-- 4 files changed, 149 insertions(+), 60 deletions(-) diff --git a/src/StaticCaching/Cachers/AbstractCacher.php b/src/StaticCaching/Cachers/AbstractCacher.php index 2071ae3a66d..7203a165592 100644 --- a/src/StaticCaching/Cachers/AbstractCacher.php +++ b/src/StaticCaching/Cachers/AbstractCacher.php @@ -262,7 +262,15 @@ public function refreshUrl($url, $domain = null) { $this->getUrls($domain)->filter(function ($value) use ($url) { return $value === $url || Str::startsWith($value, $url.'?'); - })->each(function ($url) use ($domain) { + })->each(function ($url, $key) use ($domain) { + // Warming an error response would just fail with the same error, + // so invalidate it and let the next request cache a fresh copy. + if ($this->hasCachedErrorResponse($key)) { + $this->invalidateUrl($url, $domain); + + return; + } + $url = ($domain ?: $this->getBaseUrl()).$url; $url = RecacheToken::addToUrl($url); @@ -275,6 +283,17 @@ public function refreshUrl($url, $domain = null) }); } + /** + * Check if the cached response for a URL key is an error response. + * + * @param string $key + * @return bool + */ + protected function hasCachedErrorResponse($key) + { + return false; + } + /** * Refresh a wildcard URL. * diff --git a/src/StaticCaching/Cachers/ApplicationCacher.php b/src/StaticCaching/Cachers/ApplicationCacher.php index 802a7fcac3c..9ee450b51d8 100644 --- a/src/StaticCaching/Cachers/ApplicationCacher.php +++ b/src/StaticCaching/Cachers/ApplicationCacher.php @@ -3,7 +3,6 @@ namespace Statamic\StaticCaching\Cachers; use Illuminate\Http\Request; -use Illuminate\Http\Response; use Illuminate\Routing\Events\ResponsePrepared; use Illuminate\Support\Facades\Event; use Statamic\Events\UrlInvalidated; @@ -39,15 +38,25 @@ public function cachePage(Request $request, $content) // and other URL characters wouldn't work as a cache key. $key = $this->makeHash($url); - if ($this->shouldTrackUrl($url, $content)) { - // Keep track of the URL and key the response content is about to be stored within. - $this->cacheUrl($key, ...$this->getPathAndDomain($url)); - } + // Keep track of the URL and key the response content is about to be stored within. + $this->cacheUrl($key, ...$this->getPathAndDomain($url)); $key = $this->normalizeKey('responses:'.$key); $value = $this->normalizeContent($content); - Event::listen(ResponsePrepared::class, function (ResponsePrepared $event) use ($key, $value) { + // The listener stays registered for the lifetime of the process, so it should + // only handle the response for the request that's currently being cached. + // Otherwise, in long-running processes (e.g. Octane, tests) it would + // re-store this entry using later requests' statuses and headers. + $handled = false; + + Event::listen(ResponsePrepared::class, function (ResponsePrepared $event) use ($key, $value, &$handled) { + if ($handled) { + return; + } + + $handled = true; + $headers = collect($event->response->headers->all()) ->reject(fn ($value, $key) => in_array($key, ['date', 'x-powered-by', 'cache-control', 'expires', 'set-cookie'])) ->all(); @@ -64,21 +73,6 @@ public function cachePage(Request $request, $content) }); } - /** - * Determine whether a URL should be tracked in the cacher's URL set. - * - * @param string $url - * @param mixed $content - */ - private function shouldTrackUrl($url, $content): bool - { - if (! $content instanceof Response || $content->isSuccessful()) { - return true; - } - - return str_contains($url, '/__shared-errors/'); - } - /** * Check if a page has been cached. * @@ -110,6 +104,23 @@ private function getFromCache(Request $request) return $this->cache->get($this->normalizeKey('responses:'.$key)); } + /** + * Check if the cached response for a URL key is an error response. + * + * @param string $key + * @return bool + */ + protected function hasCachedErrorResponse($key) + { + $cached = $this->cache->get($this->normalizeKey('responses:'.$key)); + + if (! is_array($cached)) { + return false; + } + + return ($cached['status'] ?? 200) >= 400; + } + /** * Flush out the entire static cache. * diff --git a/tests/StaticCaching/ApplicationCacherTest.php b/tests/StaticCaching/ApplicationCacherTest.php index 4c0c3617bd8..62e19739af6 100644 --- a/tests/StaticCaching/ApplicationCacherTest.php +++ b/tests/StaticCaching/ApplicationCacherTest.php @@ -71,53 +71,32 @@ public function checking_if_page_is_cached_then_retrieving_it_will_only_hit_the_ } #[Test] - public function caching_a_successful_page_tracks_the_url() + #[DataProvider('cachedResponseProvider')] + public function caching_a_page_tracks_the_url($status, $content) { $cache = app(Repository::class); $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); $request = Request::create('http://example.com/about', 'GET'); - - $cacher->cachePage($request, response('about page', 200)); - event(new ResponsePrepared($request, response('about page', 200))); - - $this->assertEquals(['/about'], $cacher->getUrls()->values()->all()); - } - - #[Test] - public function caching_a_404_page_does_not_track_the_url() - { - $cache = app(Repository::class); - $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); - $request = Request::create('http://example.com/this-does-not-exist', 'GET'); - $response = response('not found', 404); + $response = response($content, $status); $cacher->cachePage($request, $response); event(new ResponsePrepared($request, $response)); - // The URL isn't tracked in the `.urls` set... - $this->assertEquals([], $cacher->getUrls()->all()); - - // ...but the 404 response is still cached and served directly by URL hash. + $this->assertEquals(['/about'], $cacher->getUrls()->values()->all()); $this->assertTrue($cacher->hasCachedPage($request)); - $this->assertEquals('not found', $cacher->getCachedPage($request)->content); + $this->assertEquals($content, $cacher->getCachedPage($request)->content); } - #[Test] - public function caching_a_shared_error_page_is_always_tracked() + public static function cachedResponseProvider() { - $cache = app(Repository::class); - $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); - $request = Request::createFrom(Request::create('http://example.com/'))->fakeStaticCacheStatus(404); - $response = response('shared not found', 404); - - $cacher->cachePage($request, $response); - event(new ResponsePrepared($request, $response)); - - $this->assertEquals(['/__shared-errors/'.\Statamic\Facades\Site::current()->handle().'/404'], $cacher->getUrls()->values()->all()); + return [ + 'successful response' => [200, 'about page'], + 'error response' => [404, 'not found'], + ]; } #[Test] - public function wildcard_refresh_does_not_warm_untracked_404_urls() + public function refreshing_a_wildcard_warms_successful_urls_and_invalidates_error_urls() { Queue::fake(); @@ -140,6 +119,31 @@ public function wildcard_refresh_does_not_warm_untracked_404_urls() Queue::assertNotPushed(StaticWarmJob::class, function ($job) { return str_contains((string) $job->request->getUri(), 'scanner-junk'); }); + + // The error response is invalidated rather than warmed, so the + // tracked set converges to real pages. + $this->assertEquals(['/rail/one'], $cacher->getUrls()->values()->all()); + $this->assertFalse($cacher->hasCachedPage($junkRequest)); + } + + #[Test] + public function refreshing_an_error_url_invalidates_it_instead_of_warming_it() + { + Queue::fake(); + + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::create('http://example.com/foo', 'GET'); + $response = response('not found', 404); + + $cacher->cachePage($request, $response); + event(new ResponsePrepared($request, $response)); + + $cacher->refreshUrls(['/foo']); + + Queue::assertNothingPushed(); + $this->assertEquals([], $cacher->getUrls()->all()); + $this->assertFalse($cacher->hasCachedPage($request)); } #[Test] @@ -167,6 +171,23 @@ public function invalidating_a_url_removes_the_html_and_the_url() $this->assertNotNull($cache->get('static-cache:responses:two')); } + #[Test] + public function invalidating_a_url_removes_a_cached_error_response() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::create('http://example.com/foo', 'GET'); + $response = response('not found', 404); + + $cacher->cachePage($request, $response); + event(new ResponsePrepared($request, $response)); + + $cacher->invalidateUrl('/foo'); + + $this->assertEquals([], $cacher->getUrls()->all()); + $this->assertFalse($cacher->hasCachedPage($request)); + } + #[Test] public function invalidating_a_url_will_invalidate_all_query_string_versions_too() { @@ -307,6 +328,23 @@ public function it_flushes() $this->assertEquals([], $cacher->getUrls('http://another.com')->all()); } + #[Test] + public function flushing_removes_cached_error_responses() + { + $cache = app(Repository::class); + $cacher = new ApplicationCacher($cache, ['base_url' => 'http://example.com']); + $request = Request::create('http://example.com/foo', 'GET'); + $response = response('not found', 404); + + $cacher->cachePage($request, $response); + event(new ResponsePrepared($request, $response)); + + $cacher->flush(); + + $this->assertEquals([], $cacher->getUrls()->all()); + $this->assertFalse($cacher->hasCachedPage($request)); + } + #[Test] #[DataProvider('currentUrlProvider')] public function it_gets_the_current_url( diff --git a/tests/StaticCaching/HalfMeasureStaticCachingTest.php b/tests/StaticCaching/HalfMeasureStaticCachingTest.php index 3b2fa9c80d1..d9612ffda51 100644 --- a/tests/StaticCaching/HalfMeasureStaticCachingTest.php +++ b/tests/StaticCaching/HalfMeasureStaticCachingTest.php @@ -209,7 +209,7 @@ public function nocache_session_is_written_under_the_real_url_for_shared_errors( } #[Test] - public function it_does_not_track_404_urls() + public function it_caches_and_tracks_404s() { \Illuminate\Support\Facades\Cache::flush(); @@ -218,17 +218,34 @@ public function it_does_not_track_404_urls() $this->get('/this-does-not-exist')->assertNotFound(); - $cacher = app(Cacher::class); - $this->assertEquals([], $cacher->getUrls()->all()); + $this->assertEquals(['/this-does-not-exist'], app(Cacher::class)->getUrls()->values()->all()); - // The 404 response is still served from the cache on a repeat hit, - // even though it was never added to the tracked `.urls` set. $response = $this->get('/this-does-not-exist')->assertNotFound(); $this->assertTrue($response->wasStaticallyCached()); } #[Test] - public function wildcard_invalidation_does_not_warm_untracked_404_urls() + public function invalidating_a_cached_404_lets_new_content_be_served() + { + \Illuminate\Support\Facades\Cache::flush(); + + $this->withStandardFakeViews(); + $this->viewShouldReturnRaw('default', '{{ title }}'); + $this->viewShouldReturnRaw('errors.404', '404 not found'); + + // The URL 404s before the page exists, and the 404 gets cached. + $this->get('/about')->assertNotFound(); + + // Publishing a page at that URL invalidates the cached 404... + $this->createPage('about', ['with' => ['title' => 'The About Page']]); + app(Cacher::class)->invalidateUrls(['/about']); + + // ...so the new page is served instead of the stale 404. + $this->get('/about')->assertOk()->assertSee('The About Page'); + } + + #[Test] + public function wildcard_refresh_invalidates_cached_404s_instead_of_warming_them() { \Illuminate\Support\Facades\Cache::flush(); @@ -256,6 +273,10 @@ public function wildcard_invalidation_does_not_warm_untracked_404_urls() Queue::assertNotPushed(StaticWarmJob::class, function ($job) { return str_contains((string) $job->request->getUri(), 'this-does-not-exist'); }); + + // The junk URL is invalidated rather than warmed, so the tracked + // set converges to real pages. + $this->assertEquals(['/about'], app(Cacher::class)->getUrls()->values()->all()); } #[Test]