Skip to content

Commit c6e2837

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#61518 [HttpKernel] Handle an array vary header in the http cache store for write (philpichet)
This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [HttpKernel] Handle an array vary header in the http cache store for write | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT We can have multiple vary on a response : ``` < HTTP/1.1 200 OK < Content-Type: application/json < Vary: Accept-Encoding < Vary: Foo < Content-Length: 1234 ``` But if we make several calls with different value for the header `Foo`, we don't check them and remove the previous responses. This fix has the purpose to check all values to keep previous call Releated to symfony#14635 Commits ------- c818606 [HttpKernel] Handle an array vary header in the http cache store for write
2 parents 4a75888 + c818606 commit c6e2837

File tree

2 files changed

+124
-7
lines changed

2 files changed

+124
-7
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/Store.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,9 @@ public function write(Request $request, Response $response): string
211211

212212
// read existing cache entries, remove non-varying, and add this one to the list
213213
$entries = [];
214-
$vary = $response->headers->get('vary');
214+
$vary = implode(', ', $response->headers->all('vary'));
215215
foreach ($this->getMetadata($key) as $entry) {
216-
if (!isset($entry[1]['vary'][0])) {
217-
$entry[1]['vary'] = [''];
218-
}
219-
220-
if ($entry[1]['vary'][0] != $vary || !$this->requestsMatch($vary ?? '', $entry[0], $storedEnv)) {
216+
if (!$this->requestsMatch($vary ?? '', $entry[0], $storedEnv)) {
221217
$entries[] = $entry;
222218
}
223219
}
@@ -285,7 +281,7 @@ public function invalidate(Request $request)
285281
*/
286282
private function requestsMatch(?string $vary, array $env1, array $env2): bool
287283
{
288-
if (empty($vary)) {
284+
if ('' === ($vary ?? '')) {
289285
return true;
290286
}
291287

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/StoreTest.php‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,127 @@ public function testLoadsBodyEval()
349349
$this->assertSame($content, $response->getContent());
350350
}
351351

352+
/**
353+
* Basic case when the second header has a different value.
354+
* Both responses should be cached
355+
*/
356+
public function testWriteWithMultipleVaryAndCachedAllResponse()
357+
{
358+
$req1 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'bar']);
359+
$content = str_repeat('a', 24).'b'.str_repeat('a', 24);
360+
$res1 = new Response($content, 200, ['vary' => ['Foo', 'Bar'], 'X-Body-Eval' => 'SSI']);
361+
$this->store->write($req1, $res1);
362+
363+
$responseLook = $this->store->lookup($req1);
364+
$this->assertSame($content, $responseLook->getContent());
365+
366+
$req2 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'foobar']);
367+
$content2 = str_repeat('b', 24).'a'.str_repeat('b', 24);
368+
$res2 = new Response($content2, 200, ['vary' => ['Foo', 'Bar'], 'X-Body-Eval' => 'SSI']);
369+
$this->store->write($req2, $res2);
370+
371+
$responseLook = $this->store->lookup($req2);
372+
$this->assertSame($content2, $responseLook->getContent());
373+
374+
$responseLook = $this->store->lookup($req1);
375+
$this->assertSame($content, $responseLook->getContent());
376+
}
377+
378+
/**
379+
* Basic case when the second header has the same value on both requests.
380+
* The last response should be cached
381+
*/
382+
public function testWriteWithMultipleVaryAndCachedLastResponse()
383+
{
384+
$req1 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'bar']);
385+
$content = str_repeat('a', 24).'b'.str_repeat('a', 24);
386+
$res1 = new Response($content, 200, ['vary' => ['Foo', 'Bar'], 'X-Body-Eval' => 'SSI']);
387+
$this->store->write($req1, $res1);
388+
389+
$responseLook = $this->store->lookup($req1);
390+
$this->assertSame($content, $responseLook->getContent());
391+
392+
$req2 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'bar']);
393+
$content2 = str_repeat('b', 24).'a'.str_repeat('b', 24);
394+
$res2 = new Response($content2, 200, ['vary' => ['Foo', 'Bar'], 'X-Body-Eval' => 'SSI']);
395+
$this->store->write($req2, $res2);
396+
397+
$responseLook = $this->store->lookup($req2);
398+
$this->assertSame($content2, $responseLook->getContent());
399+
400+
$responseLook = $this->store->lookup($req1);
401+
$this->assertSame($content2, $responseLook->getContent());
402+
}
403+
404+
/**
405+
* Case when a vary value has been removed.
406+
* Both responses should be cached
407+
*/
408+
public function testWriteWithChangingVary()
409+
{
410+
$req1 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'bar']);
411+
$content = str_repeat('a', 24).'b'.str_repeat('a', 24);
412+
$res1 = new Response($content, 200, ['vary' => ['Foo', 'bar', 'foobar'], 'X-Body-Eval' => 'SSI']);
413+
$this->store->write($req1, $res1);
414+
415+
$req2 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_FOOBAR' => 'bar']);
416+
$content2 = str_repeat('b', 24).'a'.str_repeat('b', 24);
417+
$res2 = new Response($content2, 200, ['vary' => ['Foo', 'foobar'], 'X-Body-Eval' => 'SSI']);
418+
$this->store->write($req2, $res2);
419+
420+
$responseLook = $this->store->lookup($req2);
421+
$this->assertSame($content2, $responseLook->getContent());
422+
423+
$responseLook = $this->store->lookup($req1);
424+
$this->assertSame($content, $responseLook->getContent());
425+
}
426+
427+
/**
428+
* Case when a vary value has been removed and headers of the new vary list are the same.
429+
* The last response should be cached
430+
*/
431+
public function testWriteWithRemoveVaryAndAllHeadersOnTheList()
432+
{
433+
$req1 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_FOOBAR' => 'bar',]);
434+
$content = str_repeat('a', 24).'b'.str_repeat('a', 24);
435+
$res1 = new Response($content, 200, ['vary' => ['Foo', 'bar', 'foobar'], 'X-Body-Eval' => 'SSI']);
436+
$this->store->write($req1, $res1);
437+
438+
$req2 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_FOOBAR' => 'bar']);
439+
$content2 = str_repeat('b', 24).'a'.str_repeat('b', 24);
440+
$res2 = new Response($content2, 200, ['vary' => ['Foo', 'foobar'], 'X-Body-Eval' => 'SSI']);
441+
$this->store->write($req2, $res2);
442+
443+
$responseLook = $this->store->lookup($req2);
444+
$this->assertSame($content2, $responseLook->getContent());
445+
446+
$responseLook = $this->store->lookup($req1);
447+
$this->assertSame($content2, $responseLook->getContent());
448+
}
449+
450+
/**
451+
* Case when a vary value has been added and headers of the new vary list are the same.
452+
* The last response should be cached
453+
*/
454+
public function testWriteWithAddingVaryAndAllHeadersOnTheList()
455+
{
456+
$req1 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_FOOBAR' => 'bar']);
457+
$content = str_repeat('a', 24).'b'.str_repeat('a', 24);
458+
$res1 = new Response($content, 200, ['vary' => ['Foo', 'foobar'], 'X-Body-Eval' => 'SSI']);
459+
$this->store->write($req1, $res1);
460+
461+
$req2 = Request::create('/foo', 'get', [], [], [], ['HTTP_FOO' => 'foo', 'HTTP_BAR' => 'foobar', 'HTTP_FOOBAR' => 'bar']);
462+
$content2 = str_repeat('b', 24).'a'.str_repeat('b', 24);
463+
$res2 = new Response($content2, 200, ['vary' => ['Foo', 'bar', 'foobar'], 'X-Body-Eval' => 'SSI']);
464+
$this->store->write($req2, $res2);
465+
466+
$responseLook = $this->store->lookup($req2);
467+
$this->assertSame($content2, $responseLook->getContent());
468+
469+
$responseLook = $this->store->lookup($req1);
470+
$this->assertSame($content, $responseLook->getContent());
471+
}
472+
352473
protected function storeSimpleEntry($path = null, $headers = [])
353474
{
354475
$path ??= '/test';

0 commit comments

Comments
 (0)