Skip to content

Commit 6b9ab1a

Browse files
bug #61368 [HttpKernel] Refine Vary header check to skip special handling of 'Accept-Language' when it's the only entry and '_vary_by_language' is true in CacheAttributeListener (santysisi)
This PR was merged into the 6.4 branch. Discussion ---------- [HttpKernel] Refine Vary header check to skip special handling of 'Accept-Language' when it's the only entry and '_vary_by_language' is `true` in `CacheAttributeListener` | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #61229 | License | MIT ### Summary Refine Vary header check to skip special handling of 'Accept-Language' when it's the only entry and '_vary_by_language' is true in CacheAttributeListener ### Reason Previously, the `CacheAttributeListener` was executed after the `ResponseListener`. When the `set_locale_from_accept_language` option is enabled in `framework.yaml`, the `ResponseListener` sets a Vary header on the response. Because `CacheAttributeListener` checks if a Vary header already exists before modifying it, its logic was being bypassed if it ran after the `ResponseListener`. ### References * [LocaleListener](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php#L69-L76) * [ResponseListener](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/HttpKernel/EventListener/ResponseListener.php#L55-L57) * [CacheAttributeListener](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/HttpKernel/EventListener/CacheAttributeListener.php#L126) Commits ------- fa2bcbe6dec [HttpKernel] Refine Vary header check to skip special handling of 'Accept-Language' when it's the only entry and '_vary_by_language' is `true` in `CacheAttributeListener`
2 parents 5be8659 + e15b2e6 commit 6b9ab1a

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

EventListener/CacheAttributeListener.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ public function onKernelResponse(ResponseEvent $event)
123123

124124
unset($this->lastModified[$request]);
125125
unset($this->etags[$request]);
126-
$hasVary = $response->headers->has('Vary');
126+
// Check if the response has a Vary header that should be considered, ignoring cases where
127+
// it's only 'Accept-Language' and the request has the '_vary_by_language' attribute
128+
$hasVary = ['Accept-Language'] === $response->getVary() ? !$request->attributes->get('_vary_by_language') : $response->hasVary();
127129

128130
foreach (array_reverse($attributes) as $cache) {
129131
if (null !== $cache->smaxage && !$response->headers->hasCacheControlDirective('s-maxage')) {

Tests/EventListener/CacheAttributeListenerTest.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,70 @@ public function testAttribute()
318318
$this->assertSame(CacheAttributeController::CLASS_SMAXAGE, $response->getMaxAge());
319319
}
320320

321+
/**
322+
* @dataProvider provideVaryHeaderScenarios
323+
*/
324+
public function testHasRelevantVaryHeaderBehavior(array $responseVary, array $cacheVary, bool $varyByLanguage, array $expectedVary)
325+
{
326+
$request = $this->createRequest(new Cache(vary: $cacheVary));
327+
$request->attributes->set('_vary_by_language', $varyByLanguage);
328+
329+
$response = new Response();
330+
$response->setVary($responseVary);
331+
332+
$listener = new CacheAttributeListener();
333+
$event = new ResponseEvent($this->getKernel(), $request, HttpKernelInterface::MAIN_REQUEST, $response);
334+
$listener->onKernelResponse($event);
335+
336+
$this->assertSame($expectedVary, $response->getVary());
337+
}
338+
339+
public static function provideVaryHeaderScenarios(): \Traversable
340+
{
341+
yield 'no vary headers at all' => [
342+
'responseVary' => [],
343+
'cacheVary' => ['X-Foo'],
344+
'varyByLanguage' => false,
345+
'expectedVary' => ['X-Foo'],
346+
];
347+
yield 'response vary accept-language only, vary_by_language true (append new)' => [
348+
'responseVary' => ['Accept-Language'],
349+
'cacheVary' => ['X-Bar'],
350+
'varyByLanguage' => true,
351+
'expectedVary' => ['Accept-Language', 'X-Bar'], // X-Bar is added
352+
];
353+
yield 'response vary accept-language only, vary_by_language false (no append)' => [
354+
'responseVary' => ['Accept-Language'],
355+
'cacheVary' => ['X-Bar'],
356+
'varyByLanguage' => false,
357+
'expectedVary' => ['Accept-Language'], // no append
358+
];
359+
yield 'response vary multiple including accept-language, vary_by_language true (no append)' => [
360+
'responseVary' => ['Accept-Language', 'User-Agent'],
361+
'cacheVary' => ['X-Baz'],
362+
'varyByLanguage' => true,
363+
'expectedVary' => ['Accept-Language', 'User-Agent'], // no append
364+
];
365+
yield 'cache vary is empty' => [
366+
'responseVary' => ['X-Existing'],
367+
'cacheVary' => [],
368+
'varyByLanguage' => true,
369+
'expectedVary' => ['X-Existing'], // nothing to add
370+
];
371+
yield 'vary * (no append) — vary_by_language=true' => [
372+
'responseVary' => ['*'],
373+
'cacheVary' => ['X-Foo'],
374+
'varyByLanguage' => true,
375+
'expectedVary' => ['*'],
376+
];
377+
yield 'vary * (no append) — vary_by_language=false' => [
378+
'responseVary' => ['*'],
379+
'cacheVary' => ['X-Foo'],
380+
'varyByLanguage' => false,
381+
'expectedVary' => ['*'],
382+
];
383+
}
384+
321385
private function createRequest(Cache $cache): Request
322386
{
323387
return new Request([], [], ['_cache' => [$cache]]);

0 commit comments

Comments
 (0)