Issue #2802403 by _Archy_, Berdir, rjay, catch, Tachion, jacktonkin,...

Issue #2802403 by _Archy_, Berdir, rjay, catch, Tachion, jacktonkin, oriol_e9g, willwh, tameeshb, dawehner, dhansen, bmcclure, DeFr, david.gil, gagarine, plach: Combination of language negotiation and path aliasing can cause a corrupted route cache, 404s

core/core.services.yml

@@ -803,7 +803,7 @@ services:
     arguments: ['@current_route_match']
   router.route_provider:
     class: Drupal\Core\Routing\RouteProvider
-    arguments: ['@database', '@state', '@path.current', '@cache.data', '@path_processor_manager', '@cache_tags.invalidator']
+    arguments: ['@database', '@state', '@path.current', '@cache.data', '@path_processor_manager', '@cache_tags.invalidator', 'router', '@language_manager']
     tags:
       - { name: event_subscriber }
       - { name: backend_overridable }

core/lib/Drupal/Core/Routing/RouteProvider.php

@@ -6,6 +6,8 @@
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Cache\CacheBackendInterface;
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
+use Drupal\Core\Language\LanguageInterface;
+use Drupal\Core\Language\LanguageManagerInterface;
 use Drupal\Core\Path\CurrentPathStack;
 use Drupal\Core\PathProcessor\InboundPathProcessorInterface;
 use Drupal\Core\State\StateInterface;
@@ -85,6 +87,13 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
    */
   protected $pathProcessor;
 
+  /**
+   * The language manager.
+   *
+   * @var \Drupal\Core\Language\LanguageManagerInterface
+   */
+  protected $languageManager;
+
   /**
    * Cache ID prefix used to load routes.
    */
@@ -107,8 +116,10 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
    *   The cache tag invalidator.
    * @param string $table
    *   (Optional) The table in the database to use for matching. Defaults to 'router'
+   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
+   *   (Optional) The language manager.
    */
-  public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, CacheBackendInterface $cache_backend, InboundPathProcessorInterface $path_processor, CacheTagsInvalidatorInterface $cache_tag_invalidator, $table = 'router') {
+  public function __construct(Connection $connection, StateInterface $state, CurrentPathStack $current_path, CacheBackendInterface $cache_backend, InboundPathProcessorInterface $path_processor, CacheTagsInvalidatorInterface $cache_tag_invalidator, $table = 'router', LanguageManagerInterface $language_manager = NULL) {
     $this->connection = $connection;
     $this->state = $state;
     $this->currentPath = $current_path;
@@ -116,6 +127,7 @@ public function __construct(Connection $connection, StateInterface $state, Curre
     $this->cacheTagInvalidator = $cache_tag_invalidator;
     $this->pathProcessor = $path_processor;
     $this->tableName = $table;
+    $this->languageManager = $language_manager ?: \Drupal::languageManager();
   }
 
   /**
@@ -147,7 +159,7 @@ public function __construct(Connection $connection, StateInterface $state, Curre
   public function getRouteCollectionForRequest(Request $request) {
     // Cache both the system path as well as route parameters and matching
     // routes.
-    $cid = 'route:' . $request->getPathInfo() . ':' . $request->getQueryString();
+    $cid = $this->getRouteCollectionCacheId($request);
     if ($cached = $this->cache->get($cid)) {
       $this->currentPath->setPath($cached->data['path'], $request);
       $request->query->replace($cached->data['query']);
@@ -431,4 +443,35 @@ public function getRoutesCount() {
     return $this->connection->query("SELECT COUNT(*) FROM {" . $this->connection->escapeTable($this->tableName) . "}")->fetchField();
   }
 
+  /**
+   * Returns the cache ID for the route collection cache.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request object.
+   *
+   * @return string
+   *   The cache ID.
+   */
+  protected function getRouteCollectionCacheId(Request $request) {
+    // Include the current language code in the cache identifier as
+    // the language information can be elsewhere than in the path, for example
+    // based on the domain.
+    $language_part = $this->getCurrentLanguageCacheIdPart();
+    return 'route:' . $language_part . ':' . $request->getPathInfo() . ':' . $request->getQueryString();
+  }
+
+  /**
+   * Returns the language identifier for the route collection cache.
+   *
+   * @return string
+   *   The language identifier.
+   */
+  protected function getCurrentLanguageCacheIdPart() {
+    // This must be in sync with the language logic in
+    // \Drupal\Core\PathProcessor\PathProcessorAlias::processInbound() and
+    // \Drupal\Core\Path\AliasManager::getPathByAlias().
+    // @todo Update this if necessary in https://www.drupal.org/node/1125428.
+    return $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_URL)->getId();
+  }
+
 }

core/tests/Drupal/FunctionalTests/Routing/RouteCachingLanguageTest.php

+<?php
+
+namespace Drupal\FunctionalTests\Routing;
+
+use Drupal\field\Entity\FieldConfig;
+use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\language\Entity\ConfigurableLanguage;
+use Drupal\link\LinkItemInterface;
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests that route lookup is cached by the current language.
+ *
+ * @group routing
+ */
+class RouteCachingLanguageTest extends BrowserTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = ['path', 'node', 'content_translation', 'link', 'block'];
+
+  /**
+   * An user with permissions to administer content types.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $webUser;
+
+  protected function setUp() {
+    parent::setUp();
+
+    $this->createContentType(['type' => 'page']);
+
+    $this->drupalPlaceBlock('local_tasks_block');
+    $this->drupalPlaceBlock('page_title_block');
+
+    $permissions = [
+      'access administration pages',
+      'administer content translation',
+      'administer content types',
+      'administer languages',
+      'administer url aliases',
+      'create content translations',
+      'create page content',
+      'create url aliases',
+      'edit any page content',
+      'translate any entity',
+    ];
+    // Create and log in user.
+    $this->webUser = $this->drupalCreateUser($permissions);
+    $this->drupalLogin($this->webUser);
+
+    // Enable French language.
+    ConfigurableLanguage::createFromLangcode('fr')->save();
+
+    // Enable translation for page node.
+    $edit = [
+      'entity_types[node]' => 1,
+      'settings[node][page][translatable]' => 1,
+      'settings[node][page][fields][path]' => 1,
+      'settings[node][page][fields][body]' => 1,
+      'settings[node][page][settings][language][language_alterable]' => 1,
+    ];
+    $this->drupalPostForm('admin/config/regional/content-language', $edit, t('Save configuration'));
+
+    // Create a field with settings to validate.
+    $field_storage = FieldStorageConfig::create([
+      'field_name' => 'field_link',
+      'entity_type' => 'node',
+      'type' => 'link',
+    ]);
+    $field_storage->save();
+    $field = FieldConfig::create([
+      'field_storage' => $field_storage,
+      'bundle' => 'page',
+      'settings' => [
+        'title' => DRUPAL_OPTIONAL,
+        'link_type' => LinkItemInterface::LINK_GENERIC,
+      ],
+    ]);
+    $field->save();
+
+    entity_get_form_display('node', 'page', 'default')
+      ->setComponent('field_link', [
+        'type' => 'link_default',
+      ])
+      ->save();
+    entity_get_display('node', 'page', 'full')
+      ->setComponent('field_link', [
+        'type' => 'link',
+      ])
+      ->save();
+
+    // Enable URL language detection and selection and set a prefix for both
+    // languages.
+    $edit = ['language_interface[enabled][language-url]' => 1];
+    $this->drupalPostForm('admin/config/regional/language/detection', $edit, 'Save settings');
+    $edit = ['prefix[en]' => 'en'];
+    $this->drupalPostForm('admin/config/regional/language/detection/url', $edit, 'Save configuration');
+
+    // Reset the cache after changing the negotiation settings as that changes
+    // how links are built.
+    $this->resetAll();
+
+    $definitions = \Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'page');
+    $this->assertTrue($definitions['path']->isTranslatable(), 'Node path is translatable.');
+    $this->assertTrue($definitions['body']->isTranslatable(), 'Node body is translatable.');
+  }
+
+  /**
+   * Creates content with a link field pointing to an alias of another language.
+   *
+   * @dataProvider providerLanguage
+   */
+  public function testLinkTranslationWithAlias($source_langcode) {
+    $source_url_options = [
+      'language' => ConfigurableLanguage::load($source_langcode),
+    ];
+
+    // Create a target node in the source language that is the link target.
+    $edit = [
+      'langcode[0][value]' => $source_langcode,
+      'title[0][value]' => 'Target page',
+      'path[0][alias]' => '/target-page',
+    ];
+    $this->drupalPostForm('node/add/page', $edit, t('Save'), $source_url_options);
+
+    // Confirm that the alias works.
+    $assert_session = $this->assertSession();
+    $assert_session->addressEquals($source_langcode . '/target-page');
+    $assert_session->statusCodeEquals(200);
+    $assert_session->pageTextContains('Target page');
+
+    // Create a second node that links to the first through the link field.
+    $edit = [
+      'langcode[0][value]' => $source_langcode,
+      'title[0][value]' => 'Link page',
+      'field_link[0][uri]' => '/target-page',
+      'field_link[0][title]' => 'Target page',
+      'path[0][alias]' => '/link-page',
+    ];
+    $this->drupalPostForm('node/add/page', $edit, t('Save'), $source_url_options);
+
+    // Make sure the link node is displayed with a working link.
+    $assert_session->pageTextContains('Link page');
+    $this->clickLink('Target page');
+    $assert_session->addressEquals($source_langcode . '/target-page');
+    $assert_session->statusCodeEquals(200);
+    $assert_session->pageTextContains('Target page');
+
+    // Clear all caches, then add a translation for the link node.
+    $this->resetAll();
+
+    $this->drupalGet('link-page', $source_url_options);
+    $this->clickLink('Translate');
+    $this->clickLink(t('Add'));
+
+    // Do not change the link field.
+    $edit = [
+      'title[0][value]' => 'Translated link page',
+      'path[0][alias]' => '/translated-link-page',
+    ];
+    $this->drupalPostForm(NULL, $edit, 'Save (this translation)');
+
+    $assert_session->pageTextContains('Translated link page');
+
+    // @todo Clicking on the link does not include the language prefix.
+    $this->drupalGet('target-page', $source_url_options);
+    $assert_session->statusCodeEquals(200);
+    $assert_session->pageTextContains('Target page');
+  }
+
+  /**
+   * Data provider for testFromUri().
+   */
+  public function providerLanguage() {
+    return [
+      ['en'],
+      ['fr'],
+    ];
+  }
+
+}

core/tests/Drupal/FunctionalTests/Routing/RouteCachingNonPathLanguageNegotiationTest.php

+<?php
+
+namespace Drupal\FunctionalTests\Routing;
+
+use Drupal\language\Entity\ConfigurableLanguage;
+use Drupal\Core\Language\LanguageInterface;
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests the route cache when the language is not in the path.
+ *
+ * @group language
+ */
+class RouteCachingNonPathLanguageNegotiationTest extends BrowserTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = ['language', 'block'];
+
+  /**
+   * The admin user.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser;
+
+  protected function setUp() {
+    parent::setUp();
+
+    // Create and log in user.
+    $this->adminUser = $this->drupalCreateUser(['administer blocks', 'administer languages', 'access administration pages']);
+    $this->drupalLogin($this->adminUser);
+
+    // Add language.
+    ConfigurableLanguage::createFromLangcode('fr')->save();
+
+    // Enable session language detection and selection.
+    $edit = [
+      'language_interface[enabled][language-url]' => FALSE,
+      'language_interface[enabled][language-session]' => TRUE,
+    ];
+    $this->drupalPostForm('admin/config/regional/language/detection', $edit, t('Save settings'));
+
+    // A more common scenario is domain-based negotiation but that can not be
+    // tested. Session negotiation by default is not considered by the URL
+    // language type that is used to resolve the alias. Explicitly enable
+    // that to be able to test this scenario.
+    // @todo Improve in https://www.drupal.org/project/drupal/issues/1125428.
+    $this->config('language.types')
+      ->set('negotiation.language_url.enabled', ['language-session' => 0])
+      ->save();
+
+    // Enable the language switching block.
+    $this->drupalPlaceBlock('language_block:' . LanguageInterface::TYPE_INTERFACE, [
+      'id' => 'test_language_block',
+    ]);
+
+  }
+
+  /**
+   * Tests aliases when the negotiated language is not in the path.
+   */
+  public function testAliases() {
+    // Switch to French and try to access the now inaccessible block.
+    $this->drupalGet('');
+
+    // Create an alias for user/UID just for en, make sure that this is a 404
+    // on the french page exist in english, no matter which language is
+    // checked first. Create the alias after visiting frontpage to make sure
+    // there is no existing cache entry for this that affects the tests.
+    \Drupal::service('path.alias_storage')->save('/user/' . $this->adminUser->id(), '/user-page', 'en');
+
+    $this->clickLink('French');
+    $this->drupalGet('user-page');
+    $this->assertSession()->statusCodeEquals(404);
+
+    // Switch to english, make sure it works now.
+    $this->clickLink('English');
+    $this->drupalGet('user-page');
+    $this->assertSession()->statusCodeEquals(200);
+
+    // Clear cache and repeat the check, this time with english first.
+    $this->resetAll();
+    $this->drupalGet('user-page');
+    $this->assertSession()->statusCodeEquals(200);
+
+    $this->clickLink('French');
+    $this->drupalGet('user-page');
+    $this->assertSession()->statusCodeEquals(404);
+  }
+
+}

core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php

@@ -18,6 +18,7 @@
 use Drupal\Core\Routing\RouteProvider;
 use Drupal\Core\State\State;
 use Drupal\KernelTests\KernelTestBase;
+use Drupal\language\Entity\ConfigurableLanguage;
 use Drupal\Tests\Core\Routing\RoutingFixtures;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -36,7 +37,7 @@ class RouteProviderTest extends KernelTestBase {
   /**
    * Modules to enable.
    */
-  public static $modules = ['url_alter_test', 'system'];
+  public static $modules = ['url_alter_test', 'system', 'language'];
 
   /**
    * A collection of shared fixture data for tests.
@@ -544,7 +545,8 @@ public function testOutlinePathNoMatch() {
    */
   public function testRouteCaching() {
     $connection = Database::getConnection();
-    $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes');
+    $language_manager = \Drupal::languageManager();
+    $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes', $language_manager);
 
     $this->fixtures->createTables($connection);
 
@@ -558,7 +560,7 @@ public function testRouteCaching() {
     $request = Request::create($path, 'GET');
     $provider->getRouteCollectionForRequest($request);
 
-    $cache = $this->cache->get('route:/path/add/one:');
+    $cache = $this->cache->get('route:en:/path/add/one:');
     $this->assertEqual('/path/add/one', $cache->data['path']);
     $this->assertEqual([], $cache->data['query']);
     $this->assertEqual(3, count($cache->data['routes']));
@@ -568,7 +570,7 @@ public function testRouteCaching() {
     $request = Request::create($path, 'GET');
     $provider->getRouteCollectionForRequest($request);
 
-    $cache = $this->cache->get('route:/path/add/one:foo=bar');
+    $cache = $this->cache->get('route:en:/path/add/one:foo=bar');
     $this->assertEqual('/path/add/one', $cache->data['path']);
     $this->assertEqual(['foo' => 'bar'], $cache->data['query']);
     $this->assertEqual(3, count($cache->data['routes']));
@@ -578,7 +580,7 @@ public function testRouteCaching() {
     $request = Request::create($path, 'GET');
     $provider->getRouteCollectionForRequest($request);
 
-    $cache = $this->cache->get('route:/path/1/one:');
+    $cache = $this->cache->get('route:en:/path/1/one:');
     $this->assertEqual('/path/1/one', $cache->data['path']);
     $this->assertEqual([], $cache->data['query']);
     $this->assertEqual(2, count($cache->data['routes']));
@@ -595,10 +597,25 @@ public function testRouteCaching() {
     $request = Request::create($path, 'GET');
     $provider->getRouteCollectionForRequest($request);
 
-    $cache = $this->cache->get('route:/path/add-one:');
+    $cache = $this->cache->get('route:en:/path/add-one:');
     $this->assertEqual('/path/add/one', $cache->data['path']);
     $this->assertEqual([], $cache->data['query']);
     $this->assertEqual(3, count($cache->data['routes']));
+
+    // Test with a different current language by switching out the default
+    // language.
+    $swiss = ConfigurableLanguage::createFromLangcode('gsw-berne');
+    $language_manager->reset();
+    \Drupal::service('language.default')->set($swiss);
+
+    $path = '/path/add-one';
+    $request = Request::create($path, 'GET');
+    $provider->getRouteCollectionForRequest($request);
+
+    $cache = $this->cache->get('route:gsw-berne:/path/add-one:');
+    $this->assertEquals('/path/add/one', $cache->data['path']);
+    $this->assertEquals([], $cache->data['query']);
+    $this->assertEquals(3, count($cache->data['routes']));
   }
 
   /**