Issue #2502785 by dawehner, effulgentsia, tim.plunkett, amateescu, Fabianx, Wim Leers, catch, dsnopek, EclipseGc, yched, Berdir, larowlan, mondrake, olli: Remove support for $form_state->setCached() for GET requests

core/core.api.php

@@ -2054,12 +2054,6 @@ function hook_validation_constraint_alter(array &$definitions) {
  * an array. Here are the details of its elements, all of which are optional:
  * - callback: The callback to invoke to handle the server side of the
  *   Ajax event. More information on callbacks is below in @ref sub_callback.
- * - path: The URL path to use for the request. If omitted, defaults to
- *   'system/ajax', which invokes the default Drupal Ajax processing (this will
- *   call the callback supplied in the 'callback' element). If you supply a
- *   path, you must set up a routing entry to handle the request yourself and
- *   return output described in @ref sub_callback below. See the
- *   @link menu Routing topic @endlink for more information on routing.
  * - wrapper: The HTML 'id' attribute of the area where the content returned by
  *   the callback should be placed. Note that callbacks have a choice of
  *   returning content or JavaScript commands; 'wrapper' is used for content
@@ -2085,6 +2079,13 @@ function hook_validation_constraint_alter(array &$definitions) {
  *   - message: Translated message to display.
  *   - url: For a bar progress indicator, URL path for determining progress.
  *   - interval: For a bar progress indicator, how often to update it.
+ * - url: A \Drupal\Core\Url to which to submit the Ajax request. If omitted,
+ *   defaults to either the same URL as the form or link destination is for
+ *   someone with JavaScript disabled, or a slightly modified version (e.g.,
+ *   with a query parameter added, removed, or changed) of that URL if
+ *   necessary to support Drupal's content negotiation. It is recommended to
+ *   omit this key and use Drupal's content negotiation rather than using
+ *   substantially different URLs between Ajax and non-Ajax.
  *
  * @subsection sub_callback Setting up a callback to process Ajax
  * Once you have set up your form to trigger an Ajax response (see @ref sub_form

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

@@ -185,9 +185,22 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
     // Ensure the form ID is prepared.
     $form_id = $this->getFormId($form_id, $form_state);
 
+    $request = $this->requestStack->getCurrentRequest();
+
+    // Inform $form_state about the request method that's building it, so that
+    // it can prevent persisting state changes during HTTP methods for which
+    // that is disallowed by HTTP: GET and HEAD.
+    $form_state->setRequestMethod($request->getMethod());
+
+    // Initialize the form's user input. The user input should include only the
+    // input meant to be treated as part of what is submitted to the form, so
+    // we base it on the form's method rather than the request's method. For
+    // example, when someone does a GET request for
+    // /node/add/article?destination=foo, which is a form that expects its
+    // submission method to be POST, the user input during the GET request
+    // should be initialized to empty rather than to ['destination' => 'foo'].
     $input = $form_state->getUserInput();
     if (!isset($input)) {
-      $request = $this->requestStack->getCurrentRequest();
       $input = $form_state->isMethodType('get') ? $request->query->all() : $request->request->all();
       $form_state->setUserInput($input);
     }
@@ -313,8 +326,22 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
    */
   public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form = NULL) {
     $form = $this->retrieveForm($form_id, $form_state);
-    // All rebuilt forms will be cached.
+
+    // Only GET and POST are valid form methods. If the form receives its input
+    // via POST, then $form_state must be persisted when it is rebuilt between
+    // submissions. If the form receives its input via GET, then persisting
+    // state is forbidden by $form_state->setCached(), and the form must use
+    // the URL itself to transfer its state across steps. Although $form_state
+    // throws an exception based on the request method rather than the form's
+    // method, we base the decision to cache on the form method, because:
+    // - It's the form method that defines what the form needs to do to manage
+    //   its state.
+    // - rebuildForm() should only be called after successful input processing,
+    //   which means the request method matches the form method, and if not,
+    //   there's some other error, so it's ok if an exception is thrown.
+    if ($form_state->isMethodType('POST')) {
       $form_state->setCached();
+    }
 
     // If only parts of the form will be returned to the browser (e.g., Ajax or
     // RIA clients), or if the form already had a new build ID regenerated when

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

@@ -108,9 +108,7 @@ interface FormBuilderInterface {
    * form workflow, to be returned for rendering.
    *
    * Ajax form submissions are almost always multi-step workflows, so that is
-   * one common use-case during which form rebuilding occurs. See
+   * one common use-case during which form rebuilding occurs.
-   * Drupal\system\FormAjaxController::content() for more information about
-   * creating Ajax-enabled forms.
    *
    * @param string $form_id
    *   The unique string identifying the desired form. If a function with that
@@ -130,7 +128,6 @@ interface FormBuilderInterface {
    *   The newly built form.
    *
    * @see self::processForm()
-   * @see \Drupal\system\FormAjaxController::content()
    */
   public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form = NULL);
 

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

@@ -141,16 +141,29 @@ class FormState implements FormStateInterface {
   /**
    * The HTTP form method to use for finding the input for this form.
    *
-   * May be 'post' or 'get'. Defaults to 'post'. Note that 'get' method forms do
+   * May be 'POST' or 'GET'. Defaults to 'POST'. Note that 'GET' method forms do
    * not use form ids so are always considered to be submitted, which can have
-   * unexpected effects. The 'get' method should only be used on forms that do
+   * unexpected effects. The 'GET' method should only be used on forms that do
-   * not change data, as that is exclusively the domain of 'post.'
+   * not change data, as that is exclusively the domain of 'POST.'
    *
    * This property is uncacheable.
    *
    * @var string
    */
-  protected $method = 'post';
+  protected $method = 'POST';
+
+  /**
+   * The HTTP method used by the request building or processing this form.
+   *
+   * May be any valid HTTP method. Defaults to 'GET', because even though
+   * $method is 'POST' for most forms, the form's initial build is usually
+   * performed as part of a GET request.
+   *
+   * This property is uncacheable.
+   *
+   * @var string
+   */
+  protected $requestMethod = 'GET';
 
   /**
    * If set to TRUE the original, unprocessed form structure will be cached,
@@ -475,6 +488,12 @@ class FormState implements FormStateInterface {
    * {@inheritdoc}
    */
   public function setCached($cache = TRUE) {
+    // Persisting $form_state is a side-effect disallowed during a "safe" HTTP
+    // method.
+    if ($cache && $this->isRequestMethodSafe()) {
+      throw new \LogicException(sprintf('Form state caching on %s requests is not allowed.', $this->requestMethod));
+    }
+
     $this->cache = (bool) $cache;
     return $this;
   }
@@ -569,6 +588,29 @@ class FormState implements FormStateInterface {
     return $this->method === strtoupper($method_type);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function setRequestMethod($method) {
+    $this->requestMethod = strtoupper($method);
+    return $this;
+  }
+
+  /**
+   * Checks whether the request method is a "safe" HTTP method.
+   *
+   * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1 defines
+   * GET and HEAD as "safe" methods, meaning they SHOULD NOT have side-effects,
+   * such as persisting $form_state changes.
+   *
+   * @return bool
+   *
+   * @see \Symfony\Component\HttpFoundation\Request::isMethodSafe()
+   */
+  protected function isRequestMethodSafe() {
+    return in_array($this->requestMethod, array('GET', 'HEAD'));
+  }
+
   /**
    * {@inheritdoc}
    */

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

@@ -640,6 +640,10 @@ interface FormStateInterface {
    *   TRUE if the form should be cached, FALSE otherwise.
    *
    * @return $this
+   *
+   * @throws \LogicException
+   *   If the current request is using an HTTP method that must not change
+   *   state (e.g., GET).
    */
   public function setCached($cache = TRUE);
 
@@ -731,17 +735,36 @@ interface FormStateInterface {
   public function getLimitValidationErrors();
 
   /**
-   * Sets the HTTP form method.
+   * Sets the HTTP method to use for the form's submission.
+   *
+   * This is what the form's "method" attribute should be, not necessarily what
+   * the current request's HTTP method is. For example, a form can have a
+   * method attribute of POST, but the request that initially builds it uses
+   * GET.
    *
    * @param string $method
-   *   The HTTP form method.
+   *   Either "GET" or "POST". Other HTTP methods are not valid form submission
+   *   methods.
    *
    * @see \Drupal\Core\Form\FormState::$method
+   * @see \Drupal\Core\Form\FormStateInterface::setRequestMethod()
    *
    * @return $this
    */
   public function setMethod($method);
 
+  /**
+   * Sets the HTTP method used by the request that is building the form.
+   *
+   * @param string $method
+   *   Can be any valid HTTP method, such as GET, POST, HEAD, etc.
+   *
+   * @return $this
+   *
+   * @see \Drupal\Core\Form\FormStateInterface::setMethod()
+   */
+  public function setRequestMethod($method);
+
   /**
    * Returns the HTTP form method.
    *

core/lib/Drupal/Core/Render/Element/RenderElement.php

@@ -128,14 +128,7 @@ abstract class RenderElement extends PluginBase implements ElementInterface {
    * @see self::preRenderAjaxForm()
    */
   public static function processAjaxForm(&$element, FormStateInterface $form_state, &$complete_form) {
-    $element = static::preRenderAjaxForm($element);
+    return static::preRenderAjaxForm($element);
-
-    // If the element was processed as an #ajax element, and a custom URL was
-    // provided, set the form to be cached.
-    if (!empty($element['#ajax_processed']) && !empty($element['#ajax']['url'])) {
-      $form_state->setCached();
-    }
-    return $element;
   }
 
   /**
@@ -238,10 +231,6 @@ abstract class RenderElement extends PluginBase implements ElementInterface {
       // content negotiation takes care of formatting the response appropriately.
       // However, 'url' and 'options' may be set when wanting server processing
       // to be substantially different for a JavaScript triggered submission.
-      // One such substantial difference is form elements that use
-      // #ajax['callback'] for determining which part of the form needs
-      // re-rendering. For that, we have a special 'system.ajax' route which
-      // must be manually set.
       $settings += [
         'url' => NULL,
         'options' => ['query' => []],

core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php

@@ -15,12 +15,12 @@ use Symfony\Component\HttpFoundation\RequestStack;
 /**
  * Defines a theme negotiator that deals with the active theme on ajax requests.
  *
- * Many different pages can invoke an Ajax request to system/ajax or another
+ * Many different pages can invoke an Ajax request to a generic Ajax path. It is
- * generic Ajax path. It is almost always desired for an Ajax response to be
+ * almost always desired for an Ajax response to be rendered using the same
- * rendered using the same theme as the base page, because most themes are built
+ * theme as the base page, because most themes are built with the assumption
- * with the assumption that they control the entire page, so if the CSS for two
+ * that they control the entire page, so if the CSS for two themes are both
- * themes are both loaded for a given page, they may conflict with each other.
+ * loaded for a given page, they may conflict with each other. For example,
- * For example, Bartik is Drupal's default theme, and Seven is Drupal's default
+ * Bartik is Drupal's default theme, and Seven is Drupal's default
  * administration theme. Depending on whether the "Use the administration theme
  * when editing or creating content" checkbox is checked, the node edit form may
  * be displayed in either theme, but the Ajax response to the Field module's

core/modules/dblog/src/Tests/DbLogFormInjectionTest.php

@@ -99,6 +99,9 @@ class DbLogFormInjectionTest extends KernelTestBase implements FormInterface {
    */
   public function testLoggerSerialization() {
     $form_state = new FormState();
+
+    // Forms are only serialized during POST requests.
+    $form_state->setRequestMethod('POST');
     $form_state->setCached();
     $form_builder = $this->container->get('form_builder');
     $form_id = $form_builder->getFormId($this, $form_state);

core/modules/file/src/Controller/FileWidgetAjaxController.php

@@ -7,13 +7,12 @@
 
 namespace Drupal\file\Controller;
 
-use Drupal\system\Controller\FormAjaxController;
 use Symfony\Component\HttpFoundation\JsonResponse;
 
 /**
  * Defines a controller to respond to file widget AJAX requests.
  */
-class FileWidgetAjaxController extends FormAjaxController {
+class FileWidgetAjaxController {
 
   /**
    * Returns the progress status for a file upload process.

core/modules/simpletest/src/WebTestBase.php

@@ -1591,17 +1591,14 @@ abstract class WebTestBase extends TestBase {
    *
    *   This function can also be called to emulate an Ajax submission. In this
    *   case, this value needs to be an array with the following keys:
-   *   - path: A path to submit the form values to for Ajax-specific processing,
+   *   - path: A path to submit the form values to for Ajax-specific processing.
-   *     which is likely different than the $path parameter used for retrieving
+   *   - triggering_element: If the value for the 'path' key is a generic Ajax
-   *     the initial form. Defaults to 'system/ajax'.
+   *     processing path, this needs to be set to the name of the element. If
-   *   - triggering_element: If the value for the 'path' key is 'system/ajax' or
+   *     the name doesn't identify the element uniquely, then this should
-   *     another generic Ajax processing path, this needs to be set to the name
+   *     instead be an array with a single key/value pair, corresponding to the
-   *     of the element. If the name doesn't identify the element uniquely, then
+   *     element name and value. The \Drupal\Core\Form\FormAjaxResponseBuilder
-   *     this should instead be an array with a single key/value pair,
+   *     uses this to find the #ajax information for the element, including
-   *     corresponding to the element name and value. The callback for the
+   *     which specific callback to use for processing the request.
-   *     generic Ajax processing path uses this to find the #ajax information
-   *     for the element, including which specific callback to use for
-   *     processing the request.
    *
    *   This can also be set to NULL in order to emulate an Internet Explorer
    *   submission of a form with a single text field, and pressing ENTER in that
@@ -1649,7 +1646,10 @@ abstract class WebTestBase extends TestBase {
         $submit_matches = $this->handleForm($post, $edit, $upload, $ajax ? NULL : $submit, $form);
         $action = isset($form['action']) ? $this->getAbsoluteUrl((string) $form['action']) : $this->getUrl();
         if ($ajax) {
-          $action = $this->getAbsoluteUrl(!empty($submit['path']) ? $submit['path'] : 'system/ajax');
+          if (empty($submit['path'])) {
+            throw new \Exception('No #ajax path specified.');
+          }
+          $action = $this->getAbsoluteUrl($submit['path']);
           // Ajax callbacks verify the triggering element if necessary, so while
           // we may eventually want extra code that verifies it in the
           // handleForm() function, it's not currently a requirement.
@@ -1735,8 +1735,7 @@ abstract class WebTestBase extends TestBase {
    *   and the value is the button label. i.e.) array('op' => t('Refresh')).
    * @param $ajax_path
    *   (optional) Override the path set by the Ajax settings of the triggering
-   *   element. In the absence of both the triggering element's Ajax path and
+   *   element.
-   *   $ajax_path 'system/ajax' will be used.
    * @param $options
    *   (optional) Options to be forwarded to the url generator.
    * @param $headers
@@ -1807,7 +1806,7 @@ abstract class WebTestBase extends TestBase {
     $extra_post = '&' . $this->serializePostValues($extra_post);
 
     // Unless a particular path is specified, use the one specified by the
-    // Ajax settings, or else 'system/ajax'.
+    // Ajax settings.
     if (!isset($ajax_path)) {
       if (isset($ajax_settings['url'])) {
         // In order to allow to set for example the wrapper envelope query
@@ -1824,10 +1823,12 @@ abstract class WebTestBase extends TestBase {
           $parsed_url['path']
         );
       }
-      else {
-        $ajax_path = 'system/ajax';
     }
+
+    if (empty($ajax_path)) {
+      throw new \Exception('No #ajax path specified.');
     }
+
     $ajax_path = $this->container->get('unrouted_url_assembler')->assemble('base://' . $ajax_path, $options);
 
     // Submit the POST request.

core/modules/system/src/Controller/FormAjaxController.php

@@ -1,186 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\system\Controller\FormAjaxController.
- */
-
-namespace Drupal\system\Controller;
-
-use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
-use Drupal\Core\Form\FormAjaxResponseBuilderInterface;
-use Drupal\Core\Form\FormState;
-use Drupal\Core\Form\FormBuilderInterface;
-use Drupal\Core\Render\MainContent\MainContentRendererInterface;
-use Drupal\Core\Render\RendererInterface;
-use Drupal\Core\Routing\RouteMatchInterface;
-use Drupal\system\FileAjaxForm;
-use Psr\Log\LoggerInterface;
-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
-
-/**
- * Defines a controller to respond to form Ajax requests.
- */
-class FormAjaxController implements ContainerInjectionInterface {
-
-  /**
-   * A logger instance.
-   *
-   * @var \Psr\Log\LoggerInterface
-   */
-  protected $logger;
-
-  /**
-   * The form builder.
-   *
-   * @var \Drupal\Core\Form\FormBuilderInterface|\Drupal\Core\Form\FormCacheInterface
-   */
-  protected $formBuilder;
-
-  /**
-   * The renderer.
-   *
-   * @var \Drupal\Core\Render\RendererInterface
-   */
-  protected $renderer;
-
-  /**
-   * The main content to AJAX Response renderer.
-   *
-   * @var \Drupal\Core\Render\MainContent\MainContentRendererInterface
-   */
-  protected $ajaxRenderer;
-
-  /**
-   * The current route match.
-   *
-   * @var \Drupal\Core\Routing\RouteMatchInterface
-   */
-  protected $routeMatch;
-
-  /**
-   * The form AJAX response builder.
-   *
-   * @var \Drupal\Core\Form\FormAjaxResponseBuilderInterface
-   */
-  protected $formAjaxResponseBuilder;
-
-  /**
-   * Constructs a FormAjaxController object.
-   *
-   * @param \Psr\Log\LoggerInterface $logger
-   *   A logger instance.
-   * @param \Drupal\Core\Form\FormBuilderInterface $form_builder
-   *   The form builder.
-   * @param \Drupal\Core\Render\RendererInterface $renderer
-   *   The renderer.
-   * @param \Drupal\Core\Render\MainContent\MainContentRendererInterface $ajax_renderer
-   *   The main content to AJAX Response renderer.
-   * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
-   *   The current route match.
-   * @param \Drupal\Core\Form\FormAjaxResponseBuilderInterface $form_ajax_response_builder
-   *   The form AJAX response builder.
-   */
-  public function __construct(LoggerInterface $logger, FormBuilderInterface $form_builder, RendererInterface $renderer, MainContentRendererInterface $ajax_renderer, RouteMatchInterface $route_match, FormAjaxResponseBuilderInterface $form_ajax_response_builder) {
-    $this->logger = $logger;
-    $this->formBuilder = $form_builder;
-    $this->renderer = $renderer;
-    $this->ajaxRenderer = $ajax_renderer;
-    $this->routeMatch = $route_match;
-    $this->formAjaxResponseBuilder = $form_ajax_response_builder;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public static function create(ContainerInterface $container) {
-    return new static(
-      $container->get('logger.factory')->get('ajax'),
-      $container->get('form_builder'),
-      $container->get('renderer'),
-      $container->get('main_content_renderer.ajax'),
-      $container->get('current_route_match'),
-      $container->get('form_ajax_response_builder')
-    );
-  }
-
-  /**
-   * Processes an Ajax form submission.
-   *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The current request object.
-   *
-   * @return mixed
-   *   Whatever is returned by the triggering element's #ajax['callback']
-   *   function. One of:
-   *   - A render array containing the new or updated content to return to the
-   *     browser. This is commonly an element within the rebuilt form.
-   *   - A \Drupal\Core\Ajax\AjaxResponse object containing commands for the
-   *     browser to process.
-   *
-   * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface
-   */
-  public function content(Request $request) {
-    $ajax_form = $this->getForm($request);
-    $form = $ajax_form->getForm();
-    $form_state = $ajax_form->getFormState();
-    $commands = $ajax_form->getCommands();
-
-    $this->formBuilder->processForm($form['#form_id'], $form, $form_state);
-
-    return $this->formAjaxResponseBuilder->buildResponse($request, $form, $form_state, $commands);
-  }
-
-  /**
-   * Gets a form submitted via #ajax during an Ajax callback.
-   *
-   * This will load a form from the form cache used during Ajax operations. It
-   * pulls the form info from the request body.
-   *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The current request object.
-   *
-   * @return \Drupal\system\FileAjaxForm
-   *   A wrapper object containing the $form, $form_state, $form_id,
-   *   $form_build_id and an initial list of Ajax $commands.
-   *
-   * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface
-   */
-  protected function getForm(Request $request) {
-    $form_state = new FormState();
-    $form_build_id = $request->request->get('form_build_id');
-
-    // Get the form from the cache.
-    $form = $this->formBuilder->getCache($form_build_id, $form_state);
-    if (!$form) {
-      // If $form cannot be loaded from the cache, the form_build_id must be
-      // invalid, which means that someone performed a POST request onto
-      // system/ajax without actually viewing the concerned form in the browser.
-      // This is likely a hacking attempt as it never happens under normal
-      // circumstances.
-      $this->logger->warning('Invalid form POST data.');
-      throw new BadRequestHttpException();
-    }
-
-    // Since some of the submit handlers are run, redirects need to be disabled.
-    $form_state->disableRedirect();
-
-    // When a form is rebuilt after Ajax processing, its #build_id and #action
-    // should not change.
-    // @see \Drupal\Core\Form\FormBuilderInterface::rebuildForm()
-    $form_state->addRebuildInfo('copy', [
-      '#build_id' => TRUE,
-      '#action' => TRUE,
-    ]);
-
-    // The form needs to be processed; prepare for that by setting a few
-    // internal variables.
-    $form_state->setUserInput($request->request->all());
-    $form_id = $form['#form_id'];
-
-    return new FileAjaxForm($form, $form_state, $form_id, $form['#build_id'], []);
-  }
-
-}

core/modules/system/src/Tests/Ajax/AjaxFormCacheTest.php

@@ -37,15 +37,6 @@ class AjaxFormCacheTest extends AjaxTestBase {
 
     // The number of cache entries should not have changed.
     $this->assertEqual(0, count($key_value_expirable->getAll()));
-
-    // Visit a form that is explicitly cached, 3 times.
-    $cached_form_url = Url::fromRoute('ajax_forms_test.cached_form');
-    $this->drupalGet($cached_form_url);
-    $this->drupalGet($cached_form_url);
-    $this->drupalGet($cached_form_url);
-
-    // The number of cache entries should be exactly 3.
-    $this->assertEqual(3, count($key_value_expirable->getAll()));
   }
 
   /**

core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php

@@ -35,7 +35,7 @@ class AjaxFormPageCacheTest extends AjaxTestBase {
   }
 
   /**
-   * Create a simple form, then POST to system/ajax to change to it.
+   * Create a simple form, then submit the form via AJAX to change to it.
    */
   public function testSimpleAJAXFormValue() {
    $this->drupalGet('ajax_forms_test_get_form');

core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php

@@ -53,7 +53,7 @@ class FormStoragePageCacheTest extends WebTestBase {
     // Trigger validation error by submitting an empty title.
     $edit = ['title' => ''];
     $this->drupalPostForm(NULL, $edit, 'Save');
-    $this->assertText($build_id_initial, 'Old build id on the page');
+    $this->assertText('No old build id', 'No old build id on the page');
     $build_id_first_validation = $this->getFormBuildId();
     $this->assertNotEqual($build_id_initial, $build_id_first_validation, 'Build id changes when form validation fails');
 
@@ -74,7 +74,7 @@ class FormStoragePageCacheTest extends WebTestBase {
     // Trigger validation error by submitting an empty title.
     $edit = ['title' => ''];
     $this->drupalPostForm(NULL, $edit, 'Save');
-    $this->assertText($build_id_initial, 'Old build id is initial build id');
+    $this->assertText('No old build id', 'No old build id on the page');
     $build_id_from_cache_first_validation = $this->getFormBuildId();
     $this->assertNotEqual($build_id_initial, $build_id_from_cache_first_validation, 'Build id changes when form validation fails');
     $this->assertNotEqual($build_id_first_validation, $build_id_from_cache_first_validation, 'Build id from first user is not reused');
@@ -96,10 +96,15 @@ class FormStoragePageCacheTest extends WebTestBase {
     $this->assertText('No old build id', 'No old build id on the page');
     $build_id_initial = $this->getFormBuildId();
 
-    // Trigger rebuild, should regenerate build id.
+    // Trigger rebuild, should regenerate build id. When a submit handler
+    // triggers a rebuild, the form is built twice in the same POST request,
+    // and during the second build, there is an old build ID, but because the
+    // form is not cached during the initial GET request, it is different from
+    // that initial build ID.
     $edit = ['title' => 'something'];
     $this->drupalPostForm(NULL, $edit, 'Rebuild');
-    $this->assertText($build_id_initial, 'Initial build id as old build id on the page');
+    $this->assertNoText('No old build id', 'There is no old build id on the page.');
+    $this->assertNoText($build_id_initial, 'The old build id is not the initial build id.');
     $build_id_first_rebuild = $this->getFormBuildId();
     $this->assertNotEqual($build_id_initial, $build_id_first_rebuild, 'Build id changes on first rebuild.');
 

core/modules/system/src/Tests/Form/StorageTest.php

@@ -73,16 +73,19 @@ class StorageTest extends WebTestBase {
 
     // Use form rebuilding triggered by a submit button.
     $this->drupalPostForm(NULL, $edit, 'Continue submit');
+    // The first one is for the building of the form.
     $this->assertText('Form constructions: 2');
+    // The second one is for the rebuilding of the form.
+    $this->assertText('Form constructions: 3');
 
     // Reset the form to the values of the storage, using a form rebuild
     // triggered by button of type button.
     $this->drupalPostForm(NULL, array('title' => 'changed'), 'Reset');
     $this->assertFieldByName('title', 'new', 'Values have been reset.');
-    $this->assertText('Form constructions: 3');
+    $this->assertText('Form constructions: 4');
 
     $this->drupalPostForm(NULL, $edit, 'Save');
-    $this->assertText('Form constructions: 3');
+    $this->assertText('Form constructions: 4');
     $this->assertText('Title: new', 'The form storage has stored the values.');
   }
 
@@ -129,55 +132,6 @@ class StorageTest extends WebTestBase {
     $this->assertText("The thing has been changed.", 'The altered form storage value was updated in cache and taken over.');
   }
 
-  /**
-   * Tests a form using form state without using 'storage' to pass data from the
-   * constructor to a submit handler. The data has to persist even when caching
-   * gets activated, what may happen when a modules alter the form and adds
-   * #ajax properties.
-   */
-  function testFormStatePersist() {
-    // Test the form one time with caching activated and one time without.
-    $run_options = array(
-      array(),
-      array('query' => array('cache' => 1)),
-    );
-    foreach ($run_options as $options) {
-      $this->drupalPostForm('form-test/state-persist', array(), t('Submit'), $options);
-      // The submit handler outputs the value in $form_state, assert it's there.
-      $this->assertText('State persisted.');
-
-      // Test it again, but first trigger a validation error, then test.
-      $this->drupalPostForm('form-test/state-persist', array('title' => ''), t('Submit'), $options);
-      $this->assertText(t('!name field is required.', array('!name' => 'title')));
-      // Submit the form again triggering no validation error.
-      $this->drupalPostForm(NULL, array('title' => 'foo'), t('Submit'), $options);
-      $this->assertText('State persisted.');
-
-      // Now post to the rebuilt form and verify it's still there afterwards.
-      $this->drupalPostForm(NULL, array('title' => 'bar'), t('Submit'), $options);
-      $this->assertText('State persisted.');
-    }
-  }
-
-  /**
-   * Verify that the form build-id remains the same when validation errors
-   * occur on a mutable form.
-   */
-  public function testMutableForm() {
-    // Request the form with 'cache' query parameter to enable form caching.
-    $this->drupalGet('form_test/form-storage', ['query' => ['cache' => 1]]);
-    $buildIdFields = $this->xpath('//input[@name="form_build_id"]');
-    $this->assertEqual(count($buildIdFields), 1, 'One form build id field on the page');
-    $buildId = (string) $buildIdFields[0]['value'];
-
-    // Trigger validation error by submitting an empty title.
-    $edit = ['title' => ''];
-    $this->drupalPostForm(NULL, $edit, 'Continue submit');
-
-    // Verify that the build-id did not change.
-    $this->assertFieldByName('form_build_id', $buildId, 'Build id remains the same when form validation fails');
-  }
-
   /**
    * Verifies that form build-id is regenerated when loading an immutable form
    * from the cache.

core/modules/system/src/Tests/Queue/QueueSerializationTest.php

@@ -97,6 +97,7 @@ class QueueSerializationTest extends KernelTestBase implements FormInterface {
    */
   public function testQueueSerialization() {
     $form_state = new FormState();
+    $form_state->setRequestMethod('POST');
     $form_state->setCached();
     $form_builder = $this->container->get('form_builder');
     $form_id = $form_builder->getFormId($this, $form_state);

core/modules/system/system.routing.yml

@@ -1,12 +1,3 @@
-system.ajax:
-  path: '/system/ajax'
-  defaults:
-    _controller: '\Drupal\system\Controller\FormAjaxController::content'
-  options:
-    _theme: ajax_base_page
-  requirements:
-    _access: 'TRUE'
-
 system.401:
   path: '/system/401'
   defaults:

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

@@ -30,10 +30,3 @@ ajax_forms_test.lazy_load_form:
   requirements:
     _access: 'TRUE'
 
-ajax_forms_test.cached_form:
-  path: '/ajax_forms_test_cached_form'
-  defaults:
-    _title: 'AJAX forms cached form test'
-    _form: '\Drupal\ajax_forms_test\Form\AjaxFormsTestCachedForm'
-  requirements:
-    _access: 'TRUE'

core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestCachedForm.php

@@ -1,50 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\ajax_forms_test\Form\AjaxFormsTestCachedForm.
- */
-
-namespace Drupal\ajax_forms_test\Form;
-
-use Drupal\Core\Form\FormBase;
-use Drupal\Core\Form\FormStateInterface;
-use Drupal\Core\Url;
-
-/**
- * Provides an AJAX form that will be cached.
- */
-class AjaxFormsTestCachedForm extends FormBase {
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getFormId() {
-    return 'ajax_form_cache_test_form';
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function buildForm(array $form, FormStateInterface $form_state) {
-    $form['test1'] = [
-      '#type' => 'select',
-      '#title' => $this->t('Test 1'),
-      '#options' => [
-        'option1' => $this->t('Option 1'),
-        'option2' => $this->t('Option 2'),
-      ],
-      '#ajax' => [
-        'url' => Url::fromRoute('system.ajax'),
-      ],
-    ];
-    return $form;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function submitForm(array &$form, FormStateInterface $form_state) {
-  }
-
-}

core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestSimpleForm.php

@@ -57,7 +57,7 @@ class AjaxFormsTestSimpleForm extends FormBase {
     );
 
     // This is for testing invalid callbacks that should return a 500 error in
-    // \Drupal\system\FormAjaxController::content().
+    // \Drupal\Core\Form\FormAjaxResponseBuilderInterface::buildResponse().
     $invalid_callbacks = array(
       'null' => NULL,
       'empty' => '',

core/modules/system/tests/modules/form_test/form_test.module

@@ -75,19 +75,6 @@ function _form_test_tableselect_get_data() {
   return array($header, $options);
 }
 
-/**
- * Implements hook_form_FORM_ID_alter().
- *
- * @see form_test_state_persist()
- */
-function form_test_form_form_test_state_persist_alter(&$form, FormStateInterface $form_state) {
-  // Simulate a form alter implementation inserting form elements that enable
-  // caching of the form, e.g. elements having #ajax.
-  if (\Drupal::request()->get('cache')) {
-    $form_state->setCached();
-  }
-}
-
 /**
  * Implements hook_form_FORM_ID_alter() for the registration form.
  */

core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php

@@ -82,17 +82,33 @@ class FormTestStorageForm extends FormBase {
       '#value' => 'Save',
     );
 
-    if (\Drupal::request()->get('cache')) {
+    // @todo Remove this in https://www.drupal.org/node/2524408, because form
+    //   cache immutability is no longer necessary, because we no longer cache
+    //   forms during safe HTTP methods. In the meantime, because
+    //   Drupal\system\Tests\Form still has test coverage for a poisoned form
+    //   cache following a GET request, trick $form_state into caching the form
+    //   to keep that test working until we either remove it or change it in
+    //   that issue.
+    if ($this->getRequest()->get('immutable')) {
+      $form_state->addBuildInfo('immutable', TRUE);
+      if ($this->getRequest()->get('cache') && $this->getRequest()->isMethodSafe()) {
+        $form_state->setRequestMethod('FAKE');
+        $form_state->setCached();
+      }
+    }
+
+    return $form;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, FormStateInterface $form_state) {
+    if ($this->getRequest()->get('cache')) {
       // Manually activate caching, so we can test that the storage keeps working
       // when it's enabled.
       $form_state->setCached();
     }
-
-    if ($this->getRequest()->get('immutable')) {
-      $form_state->addBuildInfo('immutable', TRUE);
-    }
-
-    return $form;
   }
 
   /**
@@ -105,7 +121,7 @@ class FormTestStorageForm extends FormBase {
     // This presumes that another submitted form value triggers a validation error
     // elsewhere in the form. Form API should still update the cached form storage
     // though.
-    if (\Drupal::request()->get('cache') && $form_state->getValue('value') == 'change_title') {
+    if ($this->getRequest()->get('cache') && $form_state->getValue('value') == 'change_title') {
       $form_state->set(['thing', 'changed'], TRUE);
     }
   }

core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php

@@ -48,7 +48,6 @@ class FormTestStoragePageCacheForm extends FormBase {
     );
 
     $form['#after_build'] = array(array($this, 'form_test_storage_page_cache_old_build_id'));
-    $form_state->setCached();
 
     return $form;
   }
@@ -70,6 +69,17 @@ class FormTestStoragePageCacheForm extends FormBase {
     $form_state->setRebuild();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function validateForm(array &$form, FormStateInterface $form_state) {
+    // Test using form cache when re-displaying a form due to validation
+    // errors.
+    if ($form_state->hasAnyErrors()) {
+      $form_state->setCached();
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php

@@ -49,10 +49,6 @@ class FormTestValidateForm extends FormBase {
       '#value' => 'Save',
     );
 
-    // To simplify this test, enable form caching and use form storage to
-    // remember our alteration.
-    $form_state->setCached();
-
     return $form;
   }
 
@@ -70,6 +66,10 @@ class FormTestValidateForm extends FormBase {
 
       // Trigger a form validation error to see our changes.
       $form_state->setErrorByName('');
+
+      // To simplify this test, enable form caching and use form storage to
+      // remember our alteration.
+      $form_state->setCached();
     }
   }
 

core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php

@@ -283,7 +283,7 @@ class FormBuilderTest extends FormTestBase {
   }
 
   /**
-   * Tests the rebuildForm() method.
+   * Tests the rebuildForm() method for a POST submission.
    */
   public function testRebuildForm() {
     $form_id = 'test_form_id';
@@ -303,6 +303,9 @@ class FormBuilderTest extends FormTestBase {
     $form = $this->formBuilder->buildForm($form_arg, $form_state);
     $original_build_id = $form['#build_id'];
 
+    $this->request->setMethod('POST');
+    $form_state->setRequestMethod('POST');
+
     // Rebuild the form, and assert that the build ID has not changed.
     $form_state->setRebuild();
     $input['form_id'] = $form_id;
@@ -310,11 +313,51 @@ class FormBuilderTest extends FormTestBase {
     $form_state->addRebuildInfo('copy', ['#build_id' => TRUE]);
     $this->formBuilder->processForm($form_id, $form, $form_state);
     $this->assertSame($original_build_id, $form['#build_id']);
+    $this->assertTrue($form_state->isCached());
 
     // Rebuild the form again, and assert that there is a new build ID.
     $form_state->setRebuildInfo([]);
     $form = $this->formBuilder->buildForm($form_arg, $form_state);
     $this->assertNotSame($original_build_id, $form['#build_id']);
+    $this->assertTrue($form_state->isCached());
+  }
+
+  /**
+   * Tests the rebuildForm() method for a GET submission.
+   */
+  public function testRebuildFormOnGetRequest() {
+    $form_id = 'test_form_id';
+    $expected_form = $form_id();
+
+    // The form will be built four times.
+    $form_arg = $this->getMock('Drupal\Core\Form\FormInterface');
+    $form_arg->expects($this->exactly(2))
+      ->method('getFormId')
+      ->will($this->returnValue($form_id));
+    $form_arg->expects($this->exactly(4))
+      ->method('buildForm')
+      ->will($this->returnValue($expected_form));
+
+    // Do an initial build of the form and track the build ID.
+    $form_state = new FormState();
+    $form_state->setMethod('GET');
+    $form = $this->formBuilder->buildForm($form_arg, $form_state);
+    $original_build_id = $form['#build_id'];
+
+    // Rebuild the form, and assert that the build ID has not changed.
+    $form_state->setRebuild();
+    $input['form_id'] = $form_id;
+    $form_state->setUserInput($input);
+    $form_state->addRebuildInfo('copy', ['#build_id' => TRUE]);
+    $this->formBuilder->processForm($form_id, $form, $form_state);
+    $this->assertSame($original_build_id, $form['#build_id']);
+    $this->assertFalse($form_state->isCached());
+
+    // Rebuild the form again, and assert that there is a new build ID.
+    $form_state->setRebuildInfo([]);
+    $form = $this->formBuilder->buildForm($form_arg, $form_state);
+    $this->assertNotSame($original_build_id, $form['#build_id']);
+    $this->assertFalse($form_state->isCached());
   }
 
   /**
@@ -338,6 +381,7 @@ class FormBuilderTest extends FormTestBase {
     // Do an initial build of the form and track the build ID.
     $form_state = (new FormState())
       ->addBuildInfo('files', [['module' => 'node', 'type' => 'pages.inc']])
+      ->setRequestMethod('POST')
       ->setCached();
     $form = $this->formBuilder->buildForm($form_arg, $form_state);
 
@@ -407,6 +451,7 @@ class FormBuilderTest extends FormTestBase {
       ->with($form_build_id);
 
     $form_state = new FormState();
+    $form_state->setRequestMethod('POST');
     $form_state->setCached();
     $this->simulateFormSubmission($form_id, $form_arg, $form_state);
   }

core/tests/Drupal/Tests/Core/Form/FormStateTest.php

@@ -422,6 +422,11 @@ class FormStateTest extends UnitTestCase {
       'cache' => $cache_key,
       'no_cache' => $no_cache_key,
     ]);
+
+    $form_state->setMethod('POST');
+    $this->assertSame($expected, $form_state->isCached());
+
+    $form_state->setMethod('GET');
     $this->assertSame($expected, $form_state->isCached());
   }
 
@@ -463,6 +468,28 @@ class FormStateTest extends UnitTestCase {
     return $data;
   }
 
+  /**
+   * @covers ::setCached
+   */
+  public function testSetCachedPost() {
+    $form_state = new FormState();
+    $form_state->setRequestMethod('POST');
+    $form_state->setCached();
+    $this->assertTrue($form_state->isCached());
+  }
+
+  /**
+   * @covers ::setCached
+   *
+   * @expectedException \LogicException
+   * @expectedExceptionMessage Form state caching on GET requests is not allowed.
+   */
+  public function testSetCachedGet() {
+    $form_state = new FormState();
+    $form_state->setRequestMethod('GET');
+    $form_state->setCached();
+  }
+
   /**
    * @covers ::isMethodType
    * @covers ::setMethod

core/tests/Drupal/Tests/Core/Render/Element/RenderElementTest.php

@@ -0,0 +1,116 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Render\Element\RenderElementTest.
+ */
+
+namespace Drupal\Tests\Core\Render\Element;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\Form\FormBuilderInterface;
+use Drupal\Core\Render\Element\RenderElement;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Render\Element\RenderElement
+ * @group Render
+ */
+class RenderElementTest extends UnitTestCase {
+
+  /**
+   * The request stack.
+   *
+   * @var \Symfony\Component\HttpFoundation\RequestStack
+   */
+  protected $requestStack;
+
+  /**
+   * The container.
+   *
+   * @var \Drupal\Core\DependencyInjection\ContainerBuilder
+   */
+  protected $container;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->requestStack = new RequestStack();
+    $this->container = new ContainerBuilder();
+    $this->container->set('request_stack', $this->requestStack);
+    \Drupal::setContainer($this->container);
+  }
+
+  /**
+   * @covers ::preRenderAjaxForm
+   */
+  public function testPreRenderAjaxForm() {
+    $request = Request::create('/test');
+    $request->query->set('foo', 'bar');
+    $this->requestStack->push($request);
+
+    $prophecy = $this->prophesize('Drupal\Core\Routing\UrlGeneratorInterface');
+    $url = '/test?foo=bar&ajax_form=1';
+    $prophecy->generateFromRoute('<current>', [], ['query' => ['foo' => 'bar', FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]], FALSE)
+      ->willReturn($url);
+
+    $url_generator = $prophecy->reveal();
+    $this->container->set('url_generator', $url_generator);
+
+    $element = [
+      '#type' => 'select',
+      '#id' => 'test',
+      '#ajax' => [
+        'wrapper' => 'foo',
+        'callback' => 'test-callback',
+      ],
+    ];
+
+    $element = RenderElement::preRenderAjaxForm($element);
+
+    $this->assertTrue($element['#ajax_processed']);
+    $this->assertEquals($url, $element['#attached']['drupalSettings']['ajax']['test']['url']);
+  }
+
+  /**
+   * @covers ::preRenderAjaxForm
+   */
+  public function testPreRenderAjaxFormWithQueryOptions() {
+    $request = Request::create('/test');
+    $request->query->set('foo', 'bar');
+    $this->requestStack->push($request);
+
+    $prophecy = $this->prophesize('Drupal\Core\Routing\UrlGeneratorInterface');
+    $url = '/test?foo=bar&other=query&ajax_form=1';
+    $prophecy->generateFromRoute('<current>', [], ['query' => ['foo' => 'bar', 'other' => 'query', FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]], FALSE)
+      ->willReturn($url);
+
+    $url_generator = $prophecy->reveal();
+    $this->container->set('url_generator', $url_generator);
+
+    $element = [
+      '#type' => 'select',
+      '#id' => 'test',
+      '#ajax' => [
+        'wrapper' => 'foo',
+        'callback' => 'test-callback',
+        'options' => [
+          'query' => [
+            'other' => 'query',
+          ],
+        ],
+      ],
+    ];
+
+    $element = RenderElement::preRenderAjaxForm($element);
+
+    $this->assertTrue($element['#ajax_processed']);
+    $this->assertEquals($url, $element['#attached']['drupalSettings']['ajax']['test']['url']);
+  }
+
+}