Skip to content

Commit 7204291

Browse files
duncanmccleanjasonvarga
duncanmcclean
and
jasonvarga
authored
[5.x] Prevent query parameters bloating the static cache (#10701)
Co-authored-by: duncanmcclean <[email protected]> Co-authored-by: Jason Varga <[email protected]>
1 parent 42f43ca commit 7204291

File tree

9 files changed

+199
-44
lines changed

9 files changed

+199
-44
lines changed

‎config/static_caching.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@
102102

103103
'ignore_query_strings' => false,
104104

105+
'allowed_query_strings' => [
106+
//
107+
],
108+
109+
'disallowed_query_strings' => [
110+
//
111+
],
112+
105113
/*
106114
|--------------------------------------------------------------------------
107115
| Nocache

‎src/StaticCaching/Cachers/AbstractCacher.php‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,6 @@ public function cacheDomain($domain = null)
130130
$this->cache->forever($this->normalizeKey('domains'), $domains->all());
131131
}
132132

133-
/**
134-
* Get the URL from a request.
135-
*
136-
* @return string
137-
*/
138-
public function getUrl(Request $request)
139-
{
140-
$url = $request->getUri();
141-
142-
if ($this->config('ignore_query_strings')) {
143-
$url = explode('?', $url)[0];
144-
}
145-
146-
return $url;
147-
}
148-
149133
/**
150134
* Get all the URLs that have been cached.
151135
*
@@ -295,4 +279,40 @@ protected function getPathAndDomain($url)
295279
$parsed['scheme'].'://'.$parsed['host'],
296280
];
297281
}
282+
283+
public function getUrl(Request $request)
284+
{
285+
$url = $request->getUri();
286+
287+
if ($this->isExcluded($url)) {
288+
return $url;
289+
}
290+
291+
if ($this->config('ignore_query_strings', false)) {
292+
$url = explode('?', $url)[0];
293+
}
294+
295+
$parts = parse_url($url);
296+
297+
if (isset($parts['query'])) {
298+
parse_str($parts['query'], $query);
299+
300+
if ($allowedQueryStrings = $this->config('allowed_query_strings')) {
301+
$query = array_intersect_key($query, array_flip($allowedQueryStrings));
302+
}
303+
304+
if ($disallowedQueryStrings = $this->config('disallowed_query_strings')) {
305+
$disallowedQueryStrings = array_flip($disallowedQueryStrings);
306+
$query = array_diff_key($query, $disallowedQueryStrings);
307+
}
308+
309+
$url = $parts['scheme'].'://'.$parts['host'].$parts['path'];
310+
311+
if ($query) {
312+
$url .= '?'.http_build_query($query);
313+
}
314+
}
315+
316+
return $url;
317+
}
298318
}

‎src/StaticCaching/Cachers/FileCacher.php‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Statamic\StaticCaching\Replacers\CsrfTokenReplacer;
1313
use Statamic\Support\Arr;
1414
use Statamic\Support\Str;
15+
use Symfony\Component\HttpFoundation\HeaderUtils;
1516

1617
class FileCacher extends AbstractCacher
1718
{
@@ -61,7 +62,7 @@ public function cachePage(Request $request, $content)
6162

6263
$content = $this->normalizeContent($content);
6364

64-
$path = $this->getFilePath($request->getUri());
65+
$path = $this->getFilePath($url);
6566

6667
if (! $this->writer->write($path, $content, $this->config('lock_hold_length'))) {
6768
return;
@@ -265,4 +266,31 @@ public function getNocachePlaceholder()
265266
{
266267
return $this->nocachePlaceholder ?? '';
267268
}
269+
270+
public function getUrl(Request $request)
271+
{
272+
$url = $request->getUri();
273+
274+
if ($this->isExcluded($url)) {
275+
return $url;
276+
}
277+
278+
$url = explode('?', $url)[0];
279+
280+
if ($this->config('ignore_query_strings', false)) {
281+
return $url;
282+
}
283+
284+
// Symfony will normalize the query string which includes alphabetizing it. However, we
285+
// want to maintain the real order so that when nginx looks for the file, it can find
286+
// it. The following is the same normalizing code from Symfony without the ordering.
287+
288+
if (! $qs = $request->server->get('QUERY_STRING')) {
289+
return $url;
290+
}
291+
292+
$qs = HeaderUtils::parseQuery($qs);
293+
294+
return $url.'?'.http_build_query($qs, '', '&', \PHP_QUERY_RFC3986);
295+
}
268296
}

‎src/StaticCaching/Cachers/NullCacher.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77

88
class NullCacher implements Cacher
99
{
10+
public function config($key, $default = null)
11+
{
12+
return $default;
13+
}
14+
1015
public function cachePage(Request $request, $content)
1116
{
1217
//
@@ -44,6 +49,11 @@ public function getUrls($domain = null)
4449

4550
public function getBaseUrl()
4651
{
47-
//
52+
return '/';
53+
}
54+
55+
public function getUrl(Request $request)
56+
{
57+
return $request->getUri();
4858
}
4959
}

‎src/StaticCaching/ServiceProvider.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function register()
3636
});
3737

3838
$this->app->singleton(Session::class, function ($app) {
39-
$uri = $app['request']->getUri();
39+
$uri = $app[Cacher::class]->getUrl($app['request']);
4040

4141
if (config('statamic.static_caching.ignore_query_strings', false)) {
4242
$uri = explode('?', $uri)[0];
@@ -87,6 +87,10 @@ public function boot()
8787
return '<?php echo app("Statamic\StaticCaching\NoCache\BladeDirective")->handle('.$exp.', \Illuminate\Support\Arr::except(get_defined_vars(), [\'__data\', \'__path\'])); ?>';
8888
});
8989

90+
Request::macro('normalizedFullUrl', function () {
91+
return app(Cacher::class)->getUrl($this);
92+
});
93+
9094
Request::macro('fakeStaticCacheStatus', function (int $status) {
9195
$url = '/__shared-errors/'.$status;
9296
$this->pathInfo = $url;

‎src/StaticCaching/StaticCacheManager.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ protected function getConfig($name)
5959
return array_merge($config, [
6060
'exclude' => $this->app['config']['statamic.static_caching.exclude'] ?? [],
6161
'ignore_query_strings' => $this->app['config']['statamic.static_caching.ignore_query_strings'] ?? false,
62+
'allowed_query_strings' => $this->app['config']['statamic.static_caching.allowed_query_strings'] ?? [],
63+
'disallowed_query_strings' => $this->app['config']['statamic.static_caching.disallowed_query_strings'] ?? [],
6264
'locale' => Site::current()->handle(),
6365
]);
6466
}

‎tests/StaticCaching/ApplicationCacherTest.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,4 +177,57 @@ public function it_flushes()
177177
$this->assertEquals([], $cacher->getUrls('http://example.com')->all());
178178
$this->assertEquals([], $cacher->getUrls('http://another.com')->all());
179179
}
180+
181+
#[Test]
182+
#[DataProvider('currentUrlProvider')]
183+
public function it_gets_the_current_url(
184+
array $query,
185+
array $config,
186+
string $expectedUrl
187+
) {
188+
$request = Request::create('http://example.com/test', 'GET', $query);
189+
190+
$cacher = new ApplicationCacher(app(Repository::class), $config);
191+
192+
$this->assertEquals($expectedUrl, $cacher->getUrl($request));
193+
}
194+
195+
public static function currentUrlProvider()
196+
{
197+
return [
198+
'no query' => [
199+
[],
200+
[],
201+
'http://example.com/test',
202+
],
203+
'with query' => [
204+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
205+
[],
206+
'http://example.com/test?alfa=a&bravo=b&charlie=c',
207+
],
208+
'with query, ignoring query' => [
209+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
210+
['ignore_query_strings' => true],
211+
'http://example.com/test',
212+
],
213+
'with query, allowed query' => [
214+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
215+
['allowed_query_strings' => ['alfa', 'bravo']],
216+
'http://example.com/test?alfa=a&bravo=b',
217+
],
218+
'with query, disallowed query' => [
219+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
220+
['disallowed_query_strings' => ['charlie']],
221+
'http://example.com/test?alfa=a&bravo=b',
222+
],
223+
'with query, allowed and disallowed' => [
224+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
225+
[
226+
'allowed_query_strings' => ['alfa', 'bravo'],
227+
'disallowed_query_strings' => ['bravo'],
228+
],
229+
'http://example.com/test?alfa=a',
230+
],
231+
];
232+
}
180233
}

‎tests/StaticCaching/CacherTest.php‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace Tests\StaticCaching;
44

55
use Illuminate\Cache\Repository;
6-
use Illuminate\Http\Request;
76
use Illuminate\Support\Collection;
87
use Mockery;
98
use PHPUnit\Framework\Attributes\Test;
@@ -33,30 +32,6 @@ public function gets_default_expiration()
3332
$this->assertEquals(10, $cacher->getDefaultExpiration());
3433
}
3534

36-
#[Test]
37-
public function gets_a_url()
38-
{
39-
$cacher = $this->cacher();
40-
41-
$request = Request::create('http://example.com/test', 'GET', [
42-
'foo' => 'bar',
43-
]);
44-
45-
$this->assertEquals('http://example.com/test?foo=bar', $cacher->getUrl($request));
46-
}
47-
48-
#[Test]
49-
public function gets_a_url_with_query_strings_disabled()
50-
{
51-
$cacher = $this->cacher(['ignore_query_strings' => true]);
52-
53-
$request = Request::create('http://example.com/test', 'GET', [
54-
'foo' => 'bar',
55-
]);
56-
57-
$this->assertEquals('http://example.com/test', $cacher->getUrl($request));
58-
}
59-
6035
#[Test]
6136
public function gets_the_base_url_using_the_deprecated_config_value()
6237
{

‎tests/StaticCaching/FileCacherTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Tests\StaticCaching;
44

55
use Illuminate\Contracts\Cache\Repository;
6+
use Illuminate\Http\Request;
67
use Illuminate\Support\Facades\Event;
78
use PHPUnit\Framework\Attributes\DataProvider;
89
use PHPUnit\Framework\Attributes\Test;
@@ -325,6 +326,60 @@ public static function invalidateEventProvider()
325326
];
326327
}
327328

329+
#[Test]
330+
#[DataProvider('currentUrlProvider')]
331+
public function it_gets_the_current_url(
332+
array $query,
333+
array $config,
334+
string $expectedUrl
335+
) {
336+
$request = Request::create('http://example.com/test', 'GET', $query);
337+
338+
$cacher = $this->fileCacher($config);
339+
340+
$this->assertEquals($expectedUrl, $cacher->getUrl($request));
341+
}
342+
343+
public static function currentUrlProvider()
344+
{
345+
return [
346+
'no query' => [
347+
[],
348+
[],
349+
'http://example.com/test',
350+
],
351+
'with query' => [
352+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
353+
[],
354+
'http://example.com/test?bravo=b&charlie=c&alfa=a',
355+
],
356+
'with query, ignoring query' => [
357+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
358+
['ignore_query_strings' => true],
359+
'http://example.com/test',
360+
],
361+
'with query, allowed query' => [
362+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
363+
['allowed_query_strings' => ['alfa', 'bravo']],
364+
'http://example.com/test?bravo=b&charlie=c&alfa=a', // allowed_query_strings has no effect
365+
],
366+
'with query, disallowed query' => [
367+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
368+
['disallowed_query_strings' => ['charlie']],
369+
'http://example.com/test?bravo=b&charlie=c&alfa=a', // disallowed_query_strings has no effect
370+
371+
],
372+
'with query, allowed and disallowed' => [
373+
['bravo' => 'b', 'charlie' => 'c', 'alfa' => 'a'],
374+
[
375+
'allowed_query_strings' => ['alfa', 'bravo'],
376+
'disallowed_query_strings' => ['bravo'],
377+
],
378+
'http://example.com/test?bravo=b&charlie=c&alfa=a', // allowed_query_strings and disallowed_query_strings have no effect
379+
],
380+
];
381+
}
382+
328383
private function cacheKey($domain)
329384
{
330385
return 'static-cache:'.md5($domain).'.urls';

0 commit comments

Comments
 (0)