[5.x] Prevent query parameters bloating the static cache (#10701)

Co-authored-by: duncanmcclean <[email protected]> Co-authored-by: Jason Varga <[email protected]>
statamic · Sep 9, 2024 · 7204291 · 7204291
1 parent 42f43ca
commit 7204291
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 44 deletions.
diff --git a/config/static_caching.php b/config/static_caching.php
@@ -102,6 +102,14 @@
 
     'ignore_query_strings' => false,
 
+    'allowed_query_strings' => [
+        //
+    ],
+
+    'disallowed_query_strings' => [
+        //
+    ],
+
     /*
     |--------------------------------------------------------------------------
     | Nocache

diff --git a/src/StaticCaching/Cachers/AbstractCacher.php b/src/StaticCaching/Cachers/AbstractCacher.php
@@ -130,22 +130,6 @@ public function cacheDomain($domain = null)
         $this->cache->forever($this->normalizeKey('domains'), $domains->all());
     }
 
-    /**
-     * Get the URL from a request.
-     *
-     * @return string
-     */
-    public function getUrl(Request $request)
-    {
-        $url = $request->getUri();
-
-        if ($this->config('ignore_query_strings')) {
-            $url = explode('?', $url)[0];
-        }
-
-        return $url;
-    }
-
     /**
      * Get all the URLs that have been cached.
      *
@@ -295,4 +279,40 @@ protected function getPathAndDomain($url)
             $parsed['scheme'].'://'.$parsed['host'],
         ];
     }
+
+    public function getUrl(Request $request)
+    {
+        $url = $request->getUri();
+
+        if ($this->isExcluded($url)) {
+            return $url;
+        }
+
+        if ($this->config('ignore_query_strings', false)) {
+            $url = explode('?', $url)[0];
+        }
+
+        $parts = parse_url($url);
+
+        if (isset($parts['query'])) {
+            parse_str($parts['query'], $query);
+
+            if ($allowedQueryStrings = $this->config('allowed_query_strings')) {
+                $query = array_intersect_key($query, array_flip($allowedQueryStrings));
+            }
+
+            if ($disallowedQueryStrings = $this->config('disallowed_query_strings')) {
+                $disallowedQueryStrings = array_flip($disallowedQueryStrings);
+                $query = array_diff_key($query, $disallowedQueryStrings);
+            }
+
+            $url = $parts['scheme'].'://'.$parts['host'].$parts['path'];
+
+            if ($query) {
+                $url .= '?'.http_build_query($query);
+            }
+        }
+
+        return $url;
+    }
 }
diff --git a/src/StaticCaching/Cachers/FileCacher.php b/src/StaticCaching/Cachers/FileCacher.php
@@ -12,6 +12,7 @@
 use Statamic\StaticCaching\Replacers\CsrfTokenReplacer;
 use Statamic\Support\Arr;
 use Statamic\Support\Str;
+use Symfony\Component\HttpFoundation\HeaderUtils;
 
 class FileCacher extends AbstractCacher
 {
@@ -61,7 +62,7 @@ public function cachePage(Request $request, $content)
 
         $content = $this->normalizeContent($content);
 
-        $path = $this->getFilePath($request->getUri());
+        $path = $this->getFilePath($url);
 
         if (! $this->writer->write($path, $content, $this->config('lock_hold_length'))) {
             return;
@@ -265,4 +266,31 @@ public function getNocachePlaceholder()
     {
         return $this->nocachePlaceholder ?? '';
     }
+
+    public function getUrl(Request $request)
+    {
+        $url = $request->getUri();
+
+        if ($this->isExcluded($url)) {
+            return $url;
+        }
+
+        $url = explode('?', $url)[0];
+
+        if ($this->config('ignore_query_strings', false)) {
+            return $url;
+        }
+
+        // Symfony will normalize the query string which includes alphabetizing it. However, we
+        // want to maintain the real order so that when nginx looks for the file, it can find
+        // it. The following is the same normalizing code from Symfony without the ordering.
+
+        if (! $qs = $request->server->get('QUERY_STRING')) {
+            return $url;
+        }
+
+        $qs = HeaderUtils::parseQuery($qs);
+
+        return $url.'?'.http_build_query($qs, '', '&', \PHP_QUERY_RFC3986);
+    }
 }
diff --git a/src/StaticCaching/Cachers/NullCacher.php b/src/StaticCaching/Cachers/NullCacher.php
@@ -7,6 +7,11 @@
 
 class NullCacher implements Cacher
 {
+    public function config($key, $default = null)
+    {
+        return $default;
+    }
+
     public function cachePage(Request $request, $content)
     {
         //
@@ -44,6 +49,11 @@ public function getUrls($domain = null)
 
     public function getBaseUrl()
     {
-        //
+        return '/';
+    }
+
+    public function getUrl(Request $request)
+    {
+        return $request->getUri();
     }
 }
diff --git a/src/StaticCaching/ServiceProvider.php b/src/StaticCaching/ServiceProvider.php
@@ -36,7 +36,7 @@ public function register()
         });
 
         $this->app->singleton(Session::class, function ($app) {
-            $uri = $app['request']->getUri();
+            $uri = $app[Cacher::class]->getUrl($app['request']);
 
             if (config('statamic.static_caching.ignore_query_strings', false)) {
                 $uri = explode('?', $uri)[0];
@@ -87,6 +87,10 @@ public function boot()
             return '<?php echo app("Statamic\StaticCaching\NoCache\BladeDirective")->handle('.$exp.', \Illuminate\Support\Arr::except(get_defined_vars(), [\'__data\', \'__path\'])); ?>';
         });
 
+        Request::macro('normalizedFullUrl', function () {
+            return app(Cacher::class)->getUrl($this);
+        });
+
         Request::macro('fakeStaticCacheStatus', function (int $status) {
             $url = '/__shared-errors/'.$status;
             $this->pathInfo = $url;

diff --git a/src/StaticCaching/StaticCacheManager.php b/src/StaticCaching/StaticCacheManager.php
@@ -59,6 +59,8 @@ protected function getConfig($name)
         return array_merge($config, [
             'exclude' => $this->app['config']['statamic.static_caching.exclude'] ?? [],
             'ignore_query_strings' => $this->app['config']['statamic.static_caching.ignore_query_strings'] ?? false,
+            'allowed_query_strings' => $this->app['config']['statamic.static_caching.allowed_query_strings'] ?? [],
+            'disallowed_query_strings' => $this->app['config']['statamic.static_caching.disallowed_query_strings'] ?? [],
             'locale' => Site::current()->handle(),
         ]);
     }

diff --git a/tests/StaticCaching/ApplicationCacherTest.php b/tests/StaticCaching/ApplicationCacherTest.php
@@ -177,4 +177,57 @@ public function it_flushes()
         $this->assertEquals([], $cacher->getUrls('http://example.com')->all());
         $this->assertEquals([], $cacher->getUrls('http://another.com')->all());
     }
+
+    #[Test]
+    #[DataProvider('currentUrlProvider')]
+    public function it_gets_the_current_url(
+        array $query,
+        array $config,
+        string $expectedUrl
+    ) {
+        $request = Request::create('http://example.com/test', 'GET', $query);
+
+        $cacher = new ApplicationCacher(app(Repository::class), $config);
+
+        $this->assertEquals($expectedUrl, $cacher->getUrl($request));
+    }
+
+    public static function currentUrlProvider()
+    {
+        return [
+            'no query' => [
+                [],
+                [],
+                'http://example.com/test',
+            ],
+            'with query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                [],
+                'http://example.com/test?alfa=a&bravo=b&charlie=c',
+            ],
+            'with query, ignoring query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['ignore_query_strings' => true],
+                'http://example.com/test',
+            ],
+            'with query, allowed query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['allowed_query_strings' => ['alfa', 'bravo']],
+                'http://example.com/test?alfa=a&bravo=b',
+            ],
+            'with query, disallowed query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['disallowed_query_strings' => ['charlie']],
+                'http://example.com/test?alfa=a&bravo=b',
+            ],
+            'with query, allowed and disallowed' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                [
+                    'allowed_query_strings' => ['alfa', 'bravo'],
+                    'disallowed_query_strings' => ['bravo'],
+                ],
+                'http://example.com/test?alfa=a',
+            ],
+        ];
+    }
 }
diff --git a/tests/StaticCaching/CacherTest.php b/tests/StaticCaching/CacherTest.php
@@ -3,7 +3,6 @@
 namespace Tests\StaticCaching;
 
 use Illuminate\Cache\Repository;
-use Illuminate\Http\Request;
 use Illuminate\Support\Collection;
 use Mockery;
 use PHPUnit\Framework\Attributes\Test;
@@ -33,30 +32,6 @@ public function gets_default_expiration()
         $this->assertEquals(10, $cacher->getDefaultExpiration());
     }
 
-    #[Test]
-    public function gets_a_url()
-    {
-        $cacher = $this->cacher();
-
-        $request = Request::create('http://example.com/test', 'GET', [
-            'foo' => 'bar',
-        ]);
-
-        $this->assertEquals('http://example.com/test?foo=bar', $cacher->getUrl($request));
-    }
-
-    #[Test]
-    public function gets_a_url_with_query_strings_disabled()
-    {
-        $cacher = $this->cacher(['ignore_query_strings' => true]);
-
-        $request = Request::create('http://example.com/test', 'GET', [
-            'foo' => 'bar',
-        ]);
-
-        $this->assertEquals('http://example.com/test', $cacher->getUrl($request));
-    }
-
     #[Test]
     public function gets_the_base_url_using_the_deprecated_config_value()
     {

diff --git a/tests/StaticCaching/FileCacherTest.php b/tests/StaticCaching/FileCacherTest.php
@@ -3,6 +3,7 @@
 namespace Tests\StaticCaching;
 
 use Illuminate\Contracts\Cache\Repository;
+use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Event;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
@@ -325,6 +326,60 @@ public static function invalidateEventProvider()
         ];
     }
 
+    #[Test]
+    #[DataProvider('currentUrlProvider')]
+    public function it_gets_the_current_url(
+        array $query,
+        array $config,
+        string $expectedUrl
+    ) {
+        $request = Request::create('http://example.com/test', 'GET', $query);
+
+        $cacher = $this->fileCacher($config);
+
+        $this->assertEquals($expectedUrl, $cacher->getUrl($request));
+    }
+
+    public static function currentUrlProvider()
+    {
+        return [
+            'no query' => [
+                [],
+                [],
+                'http://example.com/test',
+            ],
+            'with query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                [],
+                'http://example.com/test?bravo=b&charlie=c&alfa=a',
+            ],
+            'with query, ignoring query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['ignore_query_strings' => true],
+                'http://example.com/test',
+            ],
+            'with query, allowed query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['allowed_query_strings' => ['alfa', 'bravo']],
+                'http://example.com/test?bravo=b&charlie=c&alfa=a', // allowed_query_strings has no effect
+            ],
+            'with query, disallowed query' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                ['disallowed_query_strings' => ['charlie']],
+                'http://example.com/test?bravo=b&charlie=c&alfa=a', // disallowed_query_strings has no effect
+
+            ],
+            'with query, allowed and disallowed' => [
+                ['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
+                [
+                    'allowed_query_strings' => ['alfa', 'bravo'],
+                    'disallowed_query_strings' => ['bravo'],
+                ],
+                'http://example.com/test?bravo=b&charlie=c&alfa=a', // allowed_query_strings and disallowed_query_strings have no effect
+            ],
+        ];
+    }
+
     private function cacheKey($domain)
     {
         return 'static-cache:'.md5($domain).'.urls';