Issue #2504139 by borisson_, Wim Leers, plach, prashantgoel, Fabianx, dawehner, David_Rothstein, effulgentsia: Blocks containing a form include the form action in the cache, so they always submit to the first URL the form was viewed at

core/lib/Drupal/Core/Form/FormBuilder.php

@@ -637,6 +637,20 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
     }
   }
 
+  /**
+   * #lazy_builder callback; renders a form action URL.
+   *
+   * @return array
+   *   A renderable array representing the form action.
+   */
+  public function renderPlaceholderFormAction() {
+    return [
+      '#type' => 'markup',
+      '#markup' => $this->buildFormAction(),
+      '#cache' => ['contexts' => ['url.path', 'url.query_args']],
+    ];
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -647,7 +661,17 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
 
     // Only update the action if it is not already set.
     if (!isset($form['#action'])) {
-      $form['#action'] = $this->buildFormAction();
+      // Instead of setting an actual action URL, we set the placeholder, which
+      // will be replaced at the very last moment. This ensures forms with
+      // dynamically generated action URLs don't have poor cacheability.
+      // Use the proper API to generate the placeholder, when we have one. See
+      // https://www.drupal.org/node/2562341.
+      $placeholder = 'form_action_' . hash('crc32b', __METHOD__);
+
+      $form['#attached']['placeholders'][$placeholder] = [
+        '#lazy_builder' => ['form_builder:renderPlaceholderFormAction', []],
+      ];
+      $form['#action'] = $placeholder;
     }
 
     // Fix the form method, if it is 'get' in $form_state, but not in $form.

core/lib/Drupal/Core/Render/Renderer.php

@@ -179,7 +179,7 @@ class Renderer implements RendererInterface {
 
     // Replace the placeholder with its rendered markup, and merge its
     // bubbleable metadata with the main elements'.
-    $elements['#markup'] = str_replace($placeholder, $markup, $elements['#markup']);
+    $elements['#markup'] = SafeString::create(str_replace($placeholder, $markup, $elements['#markup']));
     $elements = $this->mergeBubbleableMetadata($elements, $placeholder_elements);
 
     // Remove the placeholder that we've just rendered.

core/modules/block/src/Tests/BlockFormInBlockTest.php

@@ -0,0 +1,76 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block\Tests\BlockFormInBlockTest.
+ */
+
+namespace Drupal\block\Tests;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests form in block caching.
+ *
+ * @group block
+ */
+class BlockFormInBlockTest extends WebTestBase {
+
+  /**
+   * Modules to install.
+   *
+   * @var array
+   */
+  public static $modules = ['block', 'block_test', 'test_page_test'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    // Enable our test block.
+   $this->drupalPlaceBlock('test_form_in_block');
+  }
+
+  /**
+   * Test to see if form in block's redirect isn't cached.
+   */
+  function testCachePerPage() {
+    $form_values = ['email' => 'test@example.com'];
+
+    // Go to "test-page" and test if the block is enabled.
+    $this->drupalGet('test-page');
+    $this->assertResponse(200);
+    $this->assertText('Your .com email address.', 'form found');
+
+    // Make sure that we're currently still on /test-page after submitting the
+    // form.
+    $this->drupalPostForm(NULL, $form_values, t('Submit'));
+    $this->assertUrl('test-page');
+    $this->assertText(t('Your email address is @email', ['@email' => 'test@example.com']));
+
+    // Go to a different page and see if the block is enabled there as well.
+    $this->drupalGet('test-render-title');
+    $this->assertResponse(200);
+    $this->assertText('Your .com email address.', 'form found');
+
+    // Make sure that submitting the form didn't redirect us to the first page
+    // we submitted the form from after submitting the form from
+    // /test-render-title.
+    $this->drupalPostForm(NULL, $form_values, t('Submit'));
+    $this->assertUrl('test-render-title');
+    $this->assertText(t('Your email address is @email', ['@email' => 'test@example.com']));
+  }
+
+  /**
+   * Test the actual placeholders
+   */
+  public function testPlaceholders() {
+    $this->drupalGet('test-multiple-forms');
+
+    $placeholder = 'form_action_' . hash('crc32b', 'Drupal\Core\Form\FormBuilder::prepareForm');
+    $this->assertText('Form action: ' . $placeholder, 'placeholder found.');
+  }
+
+}

core/modules/block/src/Tests/Views/DisplayBlockTest.php

@@ -290,7 +290,7 @@ class DisplayBlockTest extends ViewTestBase {
     // Ensure that the view cachability metadata is propagated even, for an
     // empty block.
     $this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
-    $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+    $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
 
     // Add a header displayed on empty result.
     $display = &$view->getDisplay('block_1');
@@ -308,7 +308,7 @@ class DisplayBlockTest extends ViewTestBase {
     $this->drupalGet('');
     $this->assertEqual(1, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
     $this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
-    $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+    $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
 
     // Hide the header on empty results.
     $display = &$view->getDisplay('block_1');
@@ -326,7 +326,7 @@ class DisplayBlockTest extends ViewTestBase {
     $this->drupalGet('');
     $this->assertEqual(0, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
     $this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
-    $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+    $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
 
     // Add an empty text.
     $display = &$view->getDisplay('block_1');
@@ -343,7 +343,7 @@ class DisplayBlockTest extends ViewTestBase {
     $this->drupalGet('');
     $this->assertEqual(1, count($this->xpath('//div[contains(@class, "block-views-blocktest-view-block-block-1")]')));
     $this->assertCacheTags(array_merge($block->getCacheTags(), ['block_view', 'config:block_list', 'config:system.site', 'config:views.view.test_view_block' ,'rendered']));
-    $this->assertCacheContexts(['url.query_args:_wrapper_format', 'user.roles:authenticated']);
+    $this->assertCacheContexts(['url.path', 'url.query_args', 'user.roles:authenticated']);
   }
 
   /**

core/modules/block/tests/modules/block_test/block_test.routing.yml

@@ -0,0 +1,7 @@
+block_test.test_multipleforms:
+  path: '/test-multiple-forms'
+  defaults:
+    _controller: '\Drupal\block_test\Controller\TestMultipleFormController::testMultipleForms'
+    _title: 'Multiple forms'
+  requirements:
+    _access: 'TRUE'

core/modules/block/tests/modules/block_test/src/Controller/TestMultipleFormController.php

@@ -0,0 +1,43 @@
+<?php
+/**
+ * @file
+ * Contains \Drupal\block_test\Controller\TestMultipleFormController.
+ */
+
+namespace Drupal\block_test\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Form\FormState;
+
+/**
+ * Controller for block_test module
+ */
+class TestMultipleFormController  extends ControllerBase {
+
+  public function testMultipleForms() {
+    $form_state = new FormState();
+    $build = [
+      'form1' => $this->formBuilder()->buildForm('\Drupal\block_test\Form\TestForm', $form_state),
+      'form2' => $this->formBuilder()->buildForm('\Drupal\block_test\Form\FavoriteAnimalTestForm', $form_state),
+    ];
+
+    // Output all attached placeholders trough drupal_set_message(), so we can
+    // see if there's only one in the tests.
+    $post_render_callable = function ($elements) {
+      $matches = [];
+      preg_match_all('<form\s(.*?)action="(.*?)"(.*)>', $elements, $matches);
+
+      $action_values = $matches[2];
+
+      foreach ($action_values as $action_value) {
+        drupal_set_message('Form action: ' . $action_value);
+      }
+      return $elements;
+    };
+
+    $build['#post_render'] = [$post_render_callable];
+
+    return $build;
+  }
+
+}

core/modules/block/tests/modules/block_test/src/Form/FavoriteAnimalTestForm.php

@@ -0,0 +1,46 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Form\FavoriteAnimalTestForm.
+ */
+
+namespace Drupal\block_test\Form;
+
+use Drupal\Core\Form\FormBase;
+use Drupal\Core\Form\FormStateInterface;
+
+class FavoriteAnimalTestForm extends FormBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFormId() {
+    return 'block_test_form_favorite_animal_test';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildForm(array $form, FormStateInterface $form_state) {
+    $form['favorite_animal'] = [
+      '#type' => 'textfield',
+      '#title' => $this->t('Your favorite animal.')
+    ];
+
+    $form['submit_animal'] = [
+      '#type' => 'submit',
+      '#value' => $this->t('Submit your chosen animal'),
+    ];
+
+    return $form;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitForm(array &$form, FormStateInterface $form_state) {
+    drupal_set_message($this->t('Your favorite animal is: @favorite_animal', ['@favorite_animal' => $form['favorite_animal']['#value']]));
+  }
+
+}

core/modules/block/tests/modules/block_test/src/Form/TestForm.php

@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Form\TestForm.
+ */
+
+namespace Drupal\block_test\Form;
+
+use Drupal\Core\Form\FormBase;
+use Drupal\Core\Form\FormStateInterface;
+
+class TestForm extends FormBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFormId() {
+    return 'block_test_form_test';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildForm(array $form, FormStateInterface $form_state) {
+    $form['email'] = [
+      '#type' => 'email',
+      '#title' => $this->t('Your .com email address.')
+    ];
+
+    $form['show'] = [
+      '#type' => 'submit',
+      '#value' => $this->t('Submit'),
+    ];
+
+    return $form;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, FormStateInterface $form_state) {
+    if (strpos($form_state->getValue('email'), '.com') === FALSE) {
+      $form_state->setErrorByName('email', $this->t('This is not a .com email address.'));
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitForm(array &$form, FormStateInterface $form_state) {
+    drupal_set_message($this->t('Your email address is @email', ['@email' => $form['email']['#value']]));
+  }
+
+}

core/modules/block/tests/modules/block_test/src/Plugin/Block/TestFormBlock.php

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\block_test\Plugin\Block\TestFormBlock.
+ */
+
+namespace Drupal\block_test\Plugin\Block;
+
+use Drupal\Core\Block\BlockBase;
+
+/**
+ * Provides a block to test caching.
+ *
+ * @Block(
+ *   id = "test_form_in_block",
+ *   admin_label = @Translation("Test form block caching")
+ * )
+ */
+class TestFormBlock extends BlockBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function build() {
+    return \Drupal::formBuilder()->getForm('Drupal\block_test\Form\TestForm');
+  }
+
+}

core/modules/block_content/src/Tests/BlockContentTranslationUITest.php

@@ -36,7 +36,8 @@ class BlockContentTranslationUITest extends ContentTranslationUITestBase {
   protected $defaultCacheContexts = [
     'languages:language_interface',
     'theme',
-    'url.query_args:_wrapper_format',
+    'url.path',
+    'url.query_args',
     'user.permissions',
     'user.roles:authenticated',
   ];

core/modules/content_translation/src/Tests/ContentTestTranslationUITest.php

@@ -32,7 +32,8 @@ class ContentTestTranslationUITest extends ContentTranslationUITestBase {
   protected $defaultCacheContexts = [
     'languages:language_interface',
     'theme',
-    'url.query_args:_wrapper_format',
+    'url.path',
+    'url.query_args',
     'user.permissions',
     'user.roles:authenticated',
   ];

core/modules/menu_link_content/src/Tests/MenuLinkContentTranslationUITest.php

@@ -20,7 +20,7 @@ class MenuLinkContentTranslationUITest extends ContentTranslationUITestBase {
   /**
    * {inheritdoc}
    */
-  protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'url.query_args:_wrapper_format', 'user.permissions', 'user.roles:authenticated'];
+  protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'url.path', 'url.query_args', 'user.permissions', 'user.roles:authenticated'];
 
   /**
    * Modules to enable.

core/modules/node/src/Tests/NodeBlockFunctionalTest.php

@@ -148,7 +148,7 @@ class NodeBlockFunctionalTest extends NodeTestBase {
     $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route']);
     $this->drupalGet('node/add/article');
     $this->assertText($label, 'Block was displayed on the node/add/article page.');
-    $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route']);
+    $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.path', 'url.query_args', 'user', 'route']);
     $this->drupalGet('node/' . $node1->id());
     $this->assertText($label, 'Block was displayed on the node/N when node is of type article.');
     $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user', 'route', 'timezone']);

core/modules/shortcut/src/Tests/ShortcutTranslationUITest.php

@@ -21,7 +21,7 @@ class ShortcutTranslationUITest extends ContentTranslationUITestBase {
   /**
    * {inheritdoc}
    */
-  protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user', 'url.query_args:_wrapper_format', 'url.site'];
+  protected $defaultCacheContexts = ['languages:language_interface', 'theme', 'user', 'url.path', 'url.query_args', 'url.site'];
 
   /**
    * Modules to enable.

core/modules/system/src/Tests/Common/RenderWebTest.php

@@ -30,17 +30,17 @@ class RenderWebTest extends WebTestBase {
    * Asserts the cache context for the wrapper format is always present.
    */
   function testWrapperFormatCacheContext() {
-    $this->drupalGet('');
+    $this->drupalGet('common-test/type-link-active-class');
     $this->assertIdentical(0, strpos($this->getRawContent(), "<!DOCTYPE html>\n<html"));
     $this->assertIdentical('text/html; charset=UTF-8', $this->drupalGetHeader('Content-Type'));
-    $this->assertTitle('Log in | Drupal');
+    $this->assertTitle('Test active link class | Drupal');
     $this->assertCacheContext('url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT);
 
-    $this->drupalGet('', ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT =>  'json']]);
+    $this->drupalGet('common-test/type-link-active-class', ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT =>  'json']]);
     $this->assertIdentical('application/json', $this->drupalGetHeader('Content-Type'));
     $json = Json::decode($this->getRawContent());
     $this->assertEqual(['content', 'title'], array_keys($json));
-    $this->assertIdentical('Log in', $json['title']);
+    $this->assertIdentical('Test active link class', $json['title']);
     $this->assertCacheContext('url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT);
   }
 

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

@@ -1,6 +1,7 @@
 common_test.l_active_class:
   path: '/common-test/type-link-active-class'
   defaults:
+    _title: 'Test active link class'
     _controller: '\Drupal\common_test\Controller\CommonTestController::typeLinkActiveClass'
   requirements:
     _access: 'TRUE'