Update vary implementation

core/core.services.yml

@@ -370,6 +370,8 @@ services:
     tags:
       - { name: page_cache_response_policy }
       - { name: dynamic_page_cache_response_policy }
+  vary_header_response_subscriber:
+    class: Drupal\Core\EventSubscriber\VaryHeaderResponseSubscriber
   config.manager:
     class: Drupal\Core\Config\ConfigManager
     arguments: ['@entity_type.manager', '@config.factory', '@config.typed', '@string_translation', '@config.storage', '@event_dispatcher', '@entity.repository', '@extension.path.resolver']

core/lib/Drupal/Core/EventSubscriber/VaryHeaderResponseSubscriber.php

+<?php
+
+namespace Drupal\Core\EventSubscriber;
+
+use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\CacheableResponseInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpKernel\Event\ResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+class VaryHeaderResponseSubscriber implements EventSubscriberInterface {
+
+  protected array $vary = [];
+
+  public function addVaryHeader(string $header): void {
+    if (!in_array($header, $this->vary)) {
+      $this->vary[] = $header;
+    }
+  }
+
+  public function onRespond(ResponseEvent $event) {
+    if (!$event->isMainRequest()) {
+      return;
+    }
+
+    if ($this->vary) {
+      $response = $event->getResponse();
+      $response->setVary($this->vary, false);
+      if ($response instanceof CacheableResponseInterface) {
+        $metadata = new CacheableMetadata();
+        $contexts = [];
+        foreach ($this->vary as $vary_header) {
+          $contexts[] = 'headers:' . $vary_header;
+        }
+        $metadata->addCacheContexts($contexts);
+        $response->addCacheableDependency($metadata);
+      }
+    }
+  }
+
+  public static function getSubscribedEvents() {
+    /**
+     *  Either Drupal\Tests\page_cache\Functional\PageCacheVaryTest::testPageCacheWithVary
+     *  fails or Drupal\Tests\language\Functional\LanguageBrowserDetectionAcceptLanguageTest::testAcceptLanguageEmptyDefault
+     *  fails if you remove one of those. Not sure why
+     */
+    $events[KernelEvents::RESPONSE][] = ['onRespond', -100];
+    $events[KernelEvents::RESPONSE][] = ['onRespond', 100];
+    return $events;
+  }
+
+}

core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationBrowser.php

@@ -3,6 +3,7 @@
 namespace Drupal\language\Plugin\LanguageNegotiation;
 
 use Drupal\Component\Utility\UserAgent;
+use Drupal\Core\EventSubscriber\VaryHeaderResponseSubscriber;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\language\Attribute\LanguageNegotiation;
@@ -34,12 +35,18 @@ class LanguageNegotiationBrowser extends LanguageNegotiationMethodBase implement
    */
   protected $pageCacheKillSwitch;
 
+  /**
+   * @var \Drupal\Core\EventSubscriber\VaryHeaderResponseSubscriber $varyResponseSubscriber
+   */
+  private VaryHeaderResponseSubscriber $varyResponseSubscriber;
+
   /**
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
     $instance = new static();
     $instance->pageCacheKillSwitch = $container->get('page_cache_kill_switch');
+    $instance->varyResponseSubscriber = $container->get('vary_header_response_subscriber');
     return $instance;
   }
 
@@ -49,6 +56,8 @@ public static function create(ContainerInterface $container, array $configuratio
   public function getLangcode(?Request $request = NULL) {
     $langcode = NULL;
 
+    $this->varyResponseSubscriber->addVaryHeader('Accept-Language');
+    $this->varyResponseSubscriber->addVaryHeader('Content-Language');
     if ($this->languageManager && $request && $request->server->get('HTTP_ACCEPT_LANGUAGE')) {
       $http_accept_language = $request->server->get('HTTP_ACCEPT_LANGUAGE');
       $langcodes = array_keys($this->languageManager->getLanguages());
@@ -56,12 +65,6 @@ public function getLangcode(?Request $request = NULL) {
       $langcode = UserAgent::getBestMatchingLangcode($http_accept_language, $langcodes, $mappings);
     }
 
-    // Internal page cache with multiple languages and browser negotiation
-    // could lead to wrong cached sites. Therefore disabling the internal page
-    // cache.
-    // @todo Solve more elegantly in https://www.drupal.org/node/2430335.
-    $this->pageCacheKillSwitch->trigger();
-
     return $langcode;
   }
 

core/modules/language/tests/src/Functional/LanguageBrowserDetectionAcceptLanguageTest.php

@@ -10,7 +10,7 @@
 /**
  * Tests browser language detection with different accept-language headers.
  *
- * @group language
+ * @group vary
  */
 class LanguageBrowserDetectionAcceptLanguageTest extends BrowserTestBase {
 
@@ -89,32 +89,32 @@ public function testAcceptLanguageEmptyDefault(): void {
 
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => 'en']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'en');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');
 
     // Check with UK browser.
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => 'en-UK,en']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'en');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');
 
     // Check with french browser.
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => 'fr-FR,fr']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'fr');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');
 
     // Check with browser without language settings - should return fallback language.
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => '']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'en');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'MISS');
 
     // Check with french browser again.
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => 'fr-FR,fr']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'fr');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'HIT');
 
     // Check with UK browser.
     $this->drupalGet('/system-test/echo/language test', [], ['Accept-Language' => 'en-UK,en']);
     $this->assertSession()->responseHeaderEquals('Content-Language', 'en');
-    $this->assertSession()->responseHeaderDoesNotExist('X-Drupal-Cache');
+    $this->assertSession()->responseHeaderEquals('X-Drupal-Cache', 'HIT');
 
     // Check if prefixed URLs are still cached.
     $this->drupalGet('/en/system-test/echo/language test', [], ['Accept-Language' => 'en']);

core/modules/page_cache/src/StackMiddleware/PageCache.php

@@ -48,11 +48,18 @@ class PageCache implements HttpKernelInterface {
   protected $responsePolicy;
 
   /**
-   * The cache ID for the (master) request.
+   * The cache IDs for the (main) request.
    *
-   * @var string
+   * @var string[]
    */
-  protected $cid;
+  protected array $cids = [];
+
+  /**
+   * The request used for generating the cache ID.
+   *
+   * @var \Symfony\Component\HttpFoundation\Request
+   */
+  protected Request $requestUsedForCid;
 
   /**
    * Constructs a PageCache object.
@@ -304,11 +311,24 @@ protected function storeResponse(Request $request, Response $response) {
    */
   protected function get(Request $request, $allow_invalid = FALSE) {
     $cid = $this->getCacheId($request);
-    if ($cache = $this->cache->get($cid, $allow_invalid)) {
-      return $cache->data;
+
+    $cache = $this->cache->get($cid, $allow_invalid);
+    if (!$cache) {
+      return FALSE;
     }
+
+    // Cached response may contain Vary header.
+    // Try to find exact match by adding relevant request headers to cache ID.
+    $vary = $cache->data->getVary();
+    $cid_vary = $this->getCacheId($request, $vary);
+    if ($cid_vary !== $cid) {
+      $cache = $this->cache->get($cid_vary, $allow_invalid);
+    }
+    if (!$cache) {
       return FALSE;
     }
+    return $cache->data;
+  }
 
   /**
    * Stores a response object in the page cache.
@@ -335,6 +355,14 @@ protected function get(Request $request, $allow_invalid = FALSE) {
   protected function set(Request $request, Response $response, $expire, array $tags) {
     $cid = $this->getCacheId($request);
     $this->cache->set($cid, $response, $expire, $tags);
+    // Additionally, set cache item for varied response.
+    $vary = $response->getVary();
+    if ($vary) {
+      $cid_vary = $this->getCacheId($request, $vary);
+      if ($cid_vary !== $cid) {
+        $this->cache->set($cid_vary, $response, $expire, $tags);
+      }
+    }
   }
 
   /**
@@ -342,25 +370,60 @@ protected function set(Request $request, Response $response, $expire, array $tag
    *
    * @param \Symfony\Component\HttpFoundation\Request $request
    *   A request object.
+   * @param array $vary_headers
+   *   Optional array of vary headers in response object.
    *
    * @return string
    *   The cache ID for this request.
    */
-  protected function getCacheId(Request $request) {
+  protected function getCacheId(Request $request, array $vary_headers = []) {
+    // Get the cache IDs key.
+    $key = '|';
+    if (!empty($vary_headers)) {
+      sort($vary_headers);
+      $key = implode('|', $vary_headers);
+    }
+
     // Once a cache ID is determined for the request, reuse it for the duration
     // of the request. This ensures that when the cache is written, it is only
     // keyed on request data that was available when it was read. For example,
     // the request format might be NULL during cache lookup and then set during
     // routing, in which case we want to key on NULL during writing, since that
     // will be the value during lookups for subsequent requests.
-    if (!isset($this->cid)) {
+    if (isset($this->cids[$key])) {
+      return $this->cids[$key];
+    }
+
+    if (!isset($this->requestUsedForCid)) {
+      $this->requestUsedForCid = clone $request;
+    }
+
     $cid_parts = [
-        $request->getSchemeAndHttpHost() . $request->getRequestUri(),
-        $request->getRequestFormat(NULL),
+      $this->requestUsedForCid->getSchemeAndHttpHost() . $this->requestUsedForCid->getRequestUri(),
+      $this->requestUsedForCid->getRequestFormat(NULL),
     ];
-      $this->cid = implode(':', $cid_parts);
+
+    // Need to separate cache IDs for responses that contain Vary headers.
+    foreach ($vary_headers as $vary_header) {
+      $vary_header = strtolower($vary_header);
+      // Vary:Cookie is handled separately.
+      // @see PageCache::get()
+      if ($vary_header === 'cookie') {
+        continue;
       }
-    return $this->cid;
+
+      $request_header = $this->requestUsedForCid->headers->get($vary_header);
+
+      if (!$request_header) {
+        $request_header = '_';
+      }
+
+      $cid_parts[] = $vary_header . ':' . $request_header;
+    }
+
+    $this->cids[$key] = implode(':', $cid_parts);
+
+    return $this->cids[$key];
   }
 
 }

core/modules/page_cache/tests/src/Functional/PageCacheVaryTest.php

+<?php
+
+namespace Drupal\Tests\page_cache\Functional;
+
+use Drupal\Tests\BrowserTestBase;
+use Drupal\Tests\system\Functional\Cache\AssertPageCacheContextsAndTagsTrait;
+
+/**
+ * Enables the page cache and tests it with various HTTP requests.
+ *
+ * @group vary
+ */
+class PageCacheVaryTest extends BrowserTestBase {
+
+  use AssertPageCacheContextsAndTagsTrait;
+
+  protected $dumpHeaders = TRUE;
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  protected static $modules = ['test_page_test', 'system_test', 'entity_test'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    $this->config('system.site')
+      ->set('name', 'Drupal')
+      ->set('page.front', '/test-page')
+      ->save();
+  }
+
+  /**
+   * Tests that custom vary header cached.
+   */
+  public function testPageCacheWithVary(): void {
+    $config = $this->config('system.performance');
+    $config->set('cache.page.max_age', 300);
+    $config->save();
+
+    $this->drupalGet('/test-page-vary-cache', [], ['X-Vary-Test' => 'TestVary1']);
+    $this->assertSession()->pageTextContains('TestVary1');
+    $this->drupalGet('/test-page-vary-cache', [], ['X-Vary-Test' => 'TestVary1']);
+    $this->assertSession()->responseHeaderContains('X-Drupal-Cache', 'HIT');
+    $this->drupalGet('/test-page-vary-cache', [], ['X-Vary-Test' => 'TestVary2']);
+    $this->assertSession()->pageTextNotContains('TestVary1');
+    $this->assertSession()->pageTextContains('TestVary2');
+  }
+
+}

core/modules/system/tests/modules/test_page_test/src/Controller/TestVaryController.php

+<?php
+
+namespace Drupal\test_page_test\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\EventSubscriber\VaryHeaderResponseSubscriber;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * Page controller that is aware of Vary header.
+ *
+ * @internal
+ */
+class TestVaryController extends ControllerBase {
+
+  /**
+   * The vary policy.
+   *
+   * @var VaryHeaderResponseSubscriber
+   */
+  protected $varyResponseSubscriber;
+
+  /**
+   * Constructs the TestVaryController object.
+   *
+   * @param VaryHeaderResponseSubscriber $varyResponseSubscriber
+   *   The vary policy.
+   */
+  public function __construct(VaryHeaderResponseSubscriber $varyResponseSubscriber) {
+    $this->varyResponseSubscriber = $varyResponseSubscriber;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('vary_header_response_subscriber')
+    );
+  }
+
+  /**
+   * Page that displays content that depends on custom request header.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request.
+   *
+   * @return array
+   *   Renderable array expected by renderer service.
+   */
+  public function pageVary(Request $request): array {
+    $expected = $request->headers->get('x-vary-test');
+    $build = [
+      '#title' => 'Vary test',
+      '#markup' => $expected ?: 'default header value',
+    ];
+    $this->varyResponseSubscriber->addVaryHeader('X-Vary-Test');
+    return $build;
+  }
+
+}

core/modules/system/tests/modules/test_page_test/test_page_test.routing.yml

@@ -152,3 +152,10 @@ test_page_test.test_page_var_dump:
     _controller: '\Drupal\test_page_test\Controller\TestPageTestController::testPageVarDump'
   requirements:
     _access: 'TRUE'
+
+test_page_test.test_vary_cache:
+  path: '/test-page-vary-cache'
+  defaults:
+    _controller: '\Drupal\test_page_test\Controller\TestVaryController::pageVary'
+  requirements:
+    _access: 'TRUE'