diff --git a/core/2242749-form-cache-33.patch b/core/2242749-form-cache-33.patch deleted file mode 100644 index d4bd9c3..0000000 --- a/core/2242749-form-cache-33.patch +++ /dev/null @@ -1,1062 +0,0 @@ -diff --git a/core/core.services.yml b/core/core.services.yml -index 6b8334a..c068880 100644 ---- a/core/core.services.yml -+++ b/core/core.services.yml -@@ -182,7 +182,7 @@ services: - arguments: ['@request_stack', '@url_generator'] - form_cache: - class: Drupal\Core\Form\FormCache -- arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token'] -+ arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token', '@logger.channel.form', '@config.factory', '@request_stack', '@page_cache_request_policy'] - public: false # Private to form_builder - keyvalue: - class: Drupal\Core\KeyValueStore\KeyValueFactory -diff --git a/core/lib/Drupal/Core/Ajax/UpdateBuildIdCommand.php b/core/lib/Drupal/Core/Ajax/UpdateBuildIdCommand.php -new file mode 100644 -index 0000000..5eefc6f ---- /dev/null -+++ b/core/lib/Drupal/Core/Ajax/UpdateBuildIdCommand.php -@@ -0,0 +1,65 @@ -+old = $old; -+ $this->new = $new; -+ } -+ -+ /** -+ * {@inheritdoc} -+ */ -+ public function render() { -+ return [ -+ 'command' => 'update_build_id', -+ 'old' => $this->old, -+ 'new' => $this->new, -+ ]; -+ } -+ -+} -diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php -index 6fb6467..ed99596 100644 ---- a/core/lib/Drupal/Core/Form/FormBuilder.php -+++ b/core/lib/Drupal/Core/Form/FormBuilder.php -@@ -286,17 +286,25 @@ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form - $form_state->setCached(); - - // If only parts of the form will be returned to the browser (e.g., Ajax or -- // RIA clients), re-use the old #build_id to not require client-side code to -- // manually update the hidden 'build_id' input element. -+ // RIA clients), or if the form already had a new build ID regenerated when it -+ // was retrieved from the form cache, reuse the existing #build_id. - // Otherwise, a new #build_id is generated, to not clobber the previous - // build's data in the form cache; also allowing the user to go back to an - // earlier build, make changes, and re-submit. - // @see self::prepareForm() - $rebuild_info = $form_state->getRebuildInfo(); -- if (isset($old_form['#build_id']) && !empty($rebuild_info['copy']['#build_id'])) { -+ $enforce_old_build_id = isset($old_form['#build_id']) && !empty($rebuild_info['copy']['#build_id']); -+ $old_form_is_mutable_copy = isset($old_form['#build_id_old']); -+ if ($enforce_old_build_id || $old_form_is_mutable_copy) { - $form['#build_id'] = $old_form['#build_id']; -+ if ($old_form_is_mutable_copy) { -+ $form['#build_id_old'] = $old_form['#build_id_old']; -+ } - } - else { -+ if (isset($old_form['#build_id'])) { -+ $form['#build_id_old'] = $old_form['#build_id']; -+ } - $form['#build_id'] = 'form-' . Crypt::randomBytesBase64(); - } - -diff --git a/core/lib/Drupal/Core/Form/FormCache.php b/core/lib/Drupal/Core/Form/FormCache.php -index 93da48c..05c6cf0 100644 ---- a/core/lib/Drupal/Core/Form/FormCache.php -+++ b/core/lib/Drupal/Core/Form/FormCache.php -@@ -7,11 +7,16 @@ - - namespace Drupal\Core\Form; - -+use Drupal\Component\Utility\Crypt; - use Drupal\Component\Utility\SafeMarkup; - use Drupal\Core\Access\CsrfTokenGenerator; -+use Drupal\Core\Config\ConfigFactoryInterface; - use Drupal\Core\Extension\ModuleHandlerInterface; - use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; -+use Drupal\Core\PageCache\RequestPolicyInterface; - use Drupal\Core\Session\AccountInterface; -+use Psr\Log\LoggerInterface; -+use Symfony\Component\HttpFoundation\RequestStack; - - /** - * Encapsulates the caching of a form and its form state. -@@ -49,6 +54,34 @@ class FormCache implements FormCacheInterface { - protected $moduleHandler; - - /** -+ * Logger channel. -+ * -+ * @var \Drupal\Core\Logger\LoggerChannelInterface -+ */ -+ protected $logger; -+ -+ /** -+ * The config factory. -+ * -+ * @var \Drupal\Core\Config\ConfigFactoryInterface -+ */ -+ protected $configFactory; -+ -+ /** -+ * The request stack. -+ * -+ * @var \Symfony\Component\HttpFoundation\RequestStack -+ */ -+ protected $requestStack; -+ -+ /** -+ * A policy rule determining the cacheability of a request. -+ * -+ * @var \Drupal\Core\PageCache\RequestPolicyInterface -+ */ -+ protected $requestPolicy; -+ -+ /** - * Constructs a new FormCache. - * - * @param \Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface $key_value_expirable_factory -@@ -60,12 +93,24 @@ class FormCache implements FormCacheInterface { - * The current user. - * @param \Drupal\Core\Access\CsrfTokenGenerator $csrf_token - * The CSRF token generator. -+ * @param \Psr\Log\LoggerInterface $logger -+ * A logger instance. -+ * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory -+ * The configuration factory. -+ * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack -+ * The request stack. -+ * @param \Drupal\Core\PageCache\RequestPolicyInterface $request_policy -+ * A policy rule determining the cacheability of a request. - */ -- public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token = NULL) { -+ public function __construct(KeyValueExpirableFactoryInterface $key_value_expirable_factory, ModuleHandlerInterface $module_handler, AccountInterface $current_user, CsrfTokenGenerator $csrf_token, LoggerInterface $logger, ConfigFactoryInterface $config_factory, RequestStack $request_stack, RequestPolicyInterface $request_policy) { - $this->keyValueExpirableFactory = $key_value_expirable_factory; - $this->moduleHandler = $module_handler; - $this->currentUser = $current_user; -+ $this->logger = $logger; -+ $this->configFactory = $config_factory; - $this->csrfToken = $csrf_token; -+ $this->requestStack = $request_stack; -+ $this->requestPolicy = $request_policy; - } - - /** -@@ -75,6 +120,18 @@ public function getCache($form_build_id, FormStateInterface $form_state) { - if ($form = $this->keyValueExpirableFactory->get('form')->get($form_build_id)) { - if ((isset($form['#cache_token']) && $this->csrfToken->validate($form['#cache_token'])) || (!isset($form['#cache_token']) && $this->currentUser->isAnonymous())) { - $this->loadCachedFormState($form_build_id, $form_state); -+ -+ // Generate a new #build_id if the cached form was rendered on a cacheable -+ // page. -+ $build_info = $form_state->getBuildInfo(); -+ if (!empty($build_info['immutable'])) { -+ $form['#build_id_old'] = $form['#build_id']; -+ $form['#build_id'] = 'form-' . Crypt::randomBytesBase64(); -+ $form['form_build_id']['#value'] = $form['#build_id']; -+ $form['form_build_id']['#id'] = $form['#build_id']; -+ unset($build_info['immutable']); -+ $form_state->setBuildInfo($build_info); -+ } - return $form; - } - } -@@ -124,15 +181,29 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state) - // 6 hours cache life time for forms should be plenty. - $expire = 21600; - -+ // Ensure that the form build_id embedded in the form structure is the same as -+ // the one passed in as a parameter. This is an additional safety measure to -+ // prevent legacy code operating directly with form_get_cache and -+ // form_set_cache from accidentally overwriting immutable form state. -+ if (isset($form['#build_id']) && $form['#build_id'] != $form_build_id) { -+ $this->logger->error('Form build-id mismatch detected while attempting to store a form in the cache.'); -+ return; -+ } -+ - // Cache form structure. - if (isset($form)) { - if ($this->currentUser->isAuthenticated()) { - $form['#cache_token'] = $this->csrfToken->get(); - } -+ unset($form['#build_id_old']); - $this->keyValueExpirableFactory->get('form')->setWithExpire($form_build_id, $form, $expire); - } - - // Cache form state. -+ if ($this->configFactory->get('system.performance')->get('cache.page.use_internal') && $this->isPageCacheable()) { -+ $form_state->addBuildInfo('immutable', TRUE); -+ } -+ - // Store the known list of safe strings for form re-use. - // @todo Ensure we are not storing an excessively large string list in: - // https://www.drupal.org/node/2295823 -@@ -143,4 +214,11 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state) - } - } - -+ /** -+ * Checks if the page is cacheable. -+ */ -+ protected function isPageCacheable($allow_caching = NULL) { -+ return ($this->requestPolicy->check($this->requestStack->getCurrentRequest()) === RequestPolicyInterface::ALLOW); -+ } -+ - } -diff --git a/core/lib/Drupal/Core/Form/FormState.php b/core/lib/Drupal/Core/Form/FormState.php -index 1c469d0..09db977 100644 ---- a/core/lib/Drupal/Core/Form/FormState.php -+++ b/core/lib/Drupal/Core/Form/FormState.php -@@ -57,6 +57,12 @@ class FormState implements FormStateInterface { - * processed. - * - base_form_id: Identification for a base form, as declared in the form - * class's \Drupal\Core\Form\BaseFormIdInterface::getBaseFormId() method. -+ * - immutable: If this flag is set to TRUE, a new form build id is -+ * generated when the form is loaded from the cache. If it is subsequently -+ * saved to the cache again, it will have another cache id and therefore -+ * the original form and form-state will remain unaltered. This is -+ * important when page caching is enabled in order to prevent form state -+ * from leaking between anonymous users. - * - * @var array - */ -diff --git a/core/misc/ajax.js b/core/misc/ajax.js -index 705d592..c432706 100644 ---- a/core/misc/ajax.js -+++ b/core/misc/ajax.js -@@ -719,6 +719,13 @@ - }, - - /** -+ * Command to update a form's build ID. -+ */ -+ update_build_id: function(ajax, response, status) { -+ $('input[name="form_build_id"][value="' + response.old + '"]').val(response.new); -+ }, -+ -+ /** - * Command to add css. - * - * Uses the proprietary addImport method if available as browsers which -diff --git a/core/modules/file/src/Controller/FileWidgetAjaxController.php b/core/modules/file/src/Controller/FileWidgetAjaxController.php -index 1198672..1d225cd 100644 ---- a/core/modules/file/src/Controller/FileWidgetAjaxController.php -+++ b/core/modules/file/src/Controller/FileWidgetAjaxController.php -@@ -43,7 +43,7 @@ public function upload(Request $request) { - } - - try { -- list($form, $form_state) = $this->getForm($request); -+ list($form, $form_state, $form_id, $form_build_id, $commands) = $this->getForm($request); - } - catch (HttpExceptionInterface $e) { - // Invalid form_build_id. -@@ -80,6 +80,9 @@ public function upload(Request $request) { - $settings = drupal_merge_js_settings($js['settings']['data']); - - $response = new AjaxResponse(); -+ foreach ($commands as $command) { -+ $response->addCommand($command, TRUE); -+ } - return $response->addCommand(new ReplaceCommand(NULL, $output, $settings)); - } - -diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php -index 98f68fc..686b9bc 100644 ---- a/core/modules/simpletest/src/WebTestBase.php -+++ b/core/modules/simpletest/src/WebTestBase.php -@@ -1897,6 +1897,12 @@ protected function drupalProcessAjaxResponse($content, array $ajax_response, arr - break; - case 'add_css': - break; -+ case 'update_build_id': -+ $buildId = $xpath->query('//input[@name="form_build_id" and @value="' . $command['old'] . '"]')->item(0); -+ if ($buildId) { -+ $buildId->setAttribute('value', $command['new']); -+ } -+ break; - } - } - $content = $dom->saveHTML(); -diff --git a/core/modules/system/src/Controller/FormAjaxController.php b/core/modules/system/src/Controller/FormAjaxController.php -index c3c678c..b8b9069 100644 ---- a/core/modules/system/src/Controller/FormAjaxController.php -+++ b/core/modules/system/src/Controller/FormAjaxController.php -@@ -7,6 +7,8 @@ - - namespace Drupal\system\Controller; - -+use Drupal\Core\Ajax\AjaxResponse; -+use Drupal\Core\Ajax\UpdateBuildIdCommand; - use Drupal\Core\Form\FormState; - use Drupal\Core\DependencyInjection\ContainerInjectionInterface; - use Psr\Log\LoggerInterface; -@@ -63,7 +65,7 @@ public static function create(ContainerInterface $container) { - * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface - */ - public function content(Request $request) { -- list($form, $form_state) = $this->getForm($request); -+ list($form, $form_state, $form_id, $form_build_id, $commands) = $this->getForm($request); - drupal_process_form($form['#form_id'], $form, $form_state); - - // We need to return the part of the form (or some other content) that needs -@@ -81,7 +83,12 @@ public function content(Request $request) { - if (empty($callback) || !is_callable($callback)) { - throw new HttpException(500, t('Internal Server Error')); - } -- return call_user_func_array($callback, array(&$form, &$form_state)); -+ /** @var \Drupal\Core\Ajax\AjaxResponse $response */ -+ $response = call_user_func_array($callback, [&$form, &$form_state]); -+ foreach ($commands as $command) { -+ $response->addCommand($command, TRUE); -+ } -+ return $response; - } - - /** -@@ -94,13 +101,14 @@ public function content(Request $request) { - * The current request object. - * - * @return array -- * An array containing the $form and $form_state. Use the list() function -- * to break these apart: -+ * An array containing the $form, $form_state, $form_id, $form_build_id and an -+ * initial list of Ajax $commands. Use the list() function to break these -+ * apart: - * @code -- * list($form, $form_state, $form_id, $form_build_id) = $this->getForm(); -+ * list($form, $form_state, $form_id, $form_build_id, $commands) = $this->getForm(); - * @endcode - * -- * @throws Symfony\Component\HttpKernel\Exception\HttpExceptionInterface -+ * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface - */ - protected function getForm(Request $request) { - $form_state = new FormState(); -@@ -118,6 +126,17 @@ protected function getForm(Request $request) { - throw new BadRequestHttpException(); - } - -+ // When a page level cache is enabled, the form-build id might have been -+ // replaced from within form_get_cache. If this is the case, it is also -+ // necessary to update it in the browser by issuing an appropriate Ajax -+ // command. -+ $commands = []; -+ if (isset($form['#build_id_old']) && $form['#build_id_old'] != $form['#build_id']) { -+ // If the form build ID has changed, issue an Ajax command to update it. -+ $commands[] = new UpdateBuildIdCommand($form['#build_id_old'], $form['#build_id']); -+ $form_build_id = $form['#build_id']; -+ } -+ - // Since some of the submit handlers are run, redirects need to be disabled. - $form_state->disableRedirect(); - -@@ -134,7 +153,7 @@ protected function getForm(Request $request) { - $form_state->setUserInput($request->request->all()); - $form_id = $form['#form_id']; - -- return array($form, $form_state, $form_id, $form_build_id); -+ return [$form, $form_state, $form_id, $form_build_id, $commands]; - } - - } -diff --git a/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php b/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php -new file mode 100644 -index 0000000..3d9ffa2 ---- /dev/null -+++ b/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php -@@ -0,0 +1,86 @@ -+set('cache.page.use_internal', 1); -+ $config->set('cache.page.max_age', 300); -+ $config->save(); -+ } -+ -+ /** -+ * Return the build id of the current form. -+ */ -+ protected function getFormBuildId() { -+ $build_id_fields = $this->xpath('//input[@name="form_build_id"]'); -+ $this->assertEqual(count($build_id_fields), 1, 'One form build id field on the page'); -+ return (string) $build_id_fields[0]['value']; -+ } -+ -+ /** -+ * Create a simple form, then POST to system/ajax to change to it. -+ */ -+ public function testSimpleAJAXFormValue() { -+ $this->drupalGet('ajax_forms_test_get_form'); -+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); -+ $build_id_initial = $this->getFormBuildId(); -+ -+ $edit = ['select' => 'green']; -+ $commands = $this->drupalPostAjaxForm(NULL, $edit, 'select'); -+ $build_id_first_ajax = $this->getFormBuildId(); -+ $this->assertNotEqual($build_id_initial, $build_id_first_ajax, 'Build id is changed in the simpletest-DOM on first AJAX submission'); -+ $expected = [ -+ 'command' => 'update_build_id', -+ 'old' => $build_id_initial, -+ 'new' => $build_id_first_ajax, -+ ]; -+ $this->assertCommand($commands, $expected, 'Build id change command issued on first AJAX submission'); -+ -+ $edit = ['select' => 'red']; -+ $this->drupalPostAjaxForm(NULL, $edit, 'select'); -+ $build_id_second_ajax = $this->getFormBuildId(); -+ $this->assertEqual($build_id_first_ajax, $build_id_second_ajax, 'Build id remains the same on subsequent AJAX submissions'); -+ -+ // Repeat the test sequence but this time with a page loaded from the cache. -+ $this->drupalGet('ajax_forms_test_get_form'); -+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Page was cached.'); -+ $build_id_from_cache_initial = $this->getFormBuildId(); -+ $this->assertEqual($build_id_initial, $build_id_from_cache_initial, 'Build id is the same as on the first request'); -+ -+ $edit = ['select' => 'green']; -+ $commands = $this->drupalPostAjaxForm(NULL, $edit, 'select'); -+ $build_id_from_cache_first_ajax = $this->getFormBuildId(); -+ $this->assertNotEqual($build_id_from_cache_initial, $build_id_from_cache_first_ajax, 'Build id is changed in the simpletest-DOM on first AJAX submission'); -+ $this->assertNotEqual($build_id_first_ajax, $build_id_from_cache_first_ajax, 'Build id from first user is not reused'); -+ $expected = [ -+ 'command' => 'update_build_id', -+ 'old' => $build_id_from_cache_initial, -+ 'new' => $build_id_from_cache_first_ajax, -+ ]; -+ $this->assertCommand($commands, $expected, 'Build id change command issued on first AJAX submission'); -+ -+ $edit = ['select' => 'red']; -+ $this->drupalPostAjaxForm(NULL, $edit, 'select'); -+ $build_id_from_cache_second_ajax = $this->getFormBuildId(); -+ $this->assertEqual($build_id_from_cache_first_ajax, $build_id_from_cache_second_ajax, 'Build id remains the same on subsequent AJAX submissions'); -+ } -+ -+} -diff --git a/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php b/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php -new file mode 100644 -index 0000000..1838956 ---- /dev/null -+++ b/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php -@@ -0,0 +1,115 @@ -+set('cache.page.use_internal', 1); -+ $config->set('cache.page.max_age', 300); -+ $config->save(); -+ } -+ -+ /** -+ * Return the build id of the current form. -+ */ -+ protected function getFormBuildId() { -+ $build_id_fields = $this->xpath('//input[@name="form_build_id"]'); -+ $this->assertEqual(count($build_id_fields), 1, 'One form build id field on the page'); -+ return (string) $build_id_fields[0]['value']; -+ } -+ -+ /** -+ * Build-id is regenerated when validating cached form. -+ */ -+ public function testValidateFormStorageOnCachedPage() { -+ $this->drupalGet('form-test/form-storage-page-cache'); -+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); -+ $this->assertText('No old build id', 'No old build id on the page'); -+ $build_id_initial = $this->getFormBuildId(); -+ -+ // 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'); -+ $build_id_first_validation = $this->getFormBuildId(); -+ $this->assertNotEqual($build_id_initial, $build_id_first_validation, 'Build id changes when form validation fails'); -+ -+ // Trigger validation error by again submitting an empty title. -+ $edit = ['title' => '']; -+ $this->drupalPostForm(NULL, $edit, 'Save'); -+ $this->assertText('No old build id', 'No old build id on the page'); -+ $build_id_second_validation = $this->getFormBuildId(); -+ $this->assertEqual($build_id_first_validation, $build_id_second_validation, 'Build id remains the same when form validation fails subsequently'); -+ -+ // Repeat the test sequence but this time with a page loaded from the cache. -+ $this->drupalGet('form-test/form-storage-page-cache'); -+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Page was cached.'); -+ $this->assertText('No old build id', 'No old build id on the page'); -+ $build_id_from_cache_initial = $this->getFormBuildId(); -+ $this->assertEqual($build_id_initial, $build_id_from_cache_initial, 'Build id is the same as on the first request'); -+ -+ // 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'); -+ $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'); -+ -+ // Trigger validation error by again submitting an empty title. -+ $edit = ['title' => '']; -+ $this->drupalPostForm(NULL, $edit, 'Save'); -+ $this->assertText('No old build id', 'No old build id on the page'); -+ $build_id_from_cache_second_validation = $this->getFormBuildId(); -+ $this->assertEqual($build_id_from_cache_first_validation, $build_id_from_cache_second_validation, 'Build id remains the same when form validation fails subsequently'); -+ } -+ -+ /** -+ * Build-id is regenerated when rebuilding cached form. -+ */ -+ public function testRebuildFormStorageOnCachedPage() { -+ $this->drupalGet('form-test/form-storage-page-cache'); -+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.'); -+ $this->assertText('No old build id', 'No old build id on the page'); -+ $build_id_initial = $this->getFormBuildId(); -+ -+ // Trigger rebuild, should regenerate 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'); -+ $build_id_first_rebuild = $this->getFormBuildId(); -+ $this->assertNotEqual($build_id_initial, $build_id_first_rebuild, 'Build id changes on first rebuild.'); -+ -+ // Trigger subsequent rebuild, should regenerate the build id again. -+ $edit = ['title' => 'something']; -+ $this->drupalPostForm(NULL, $edit, 'Rebuild'); -+ $this->assertText($build_id_first_rebuild, 'First build id as old build id on the page'); -+ $build_id_second_rebuild = $this->getFormBuildId(); -+ $this->assertNotEqual($build_id_first_rebuild, $build_id_second_rebuild, 'Build id changes on second rebuild.'); -+ } -+ -+} -diff --git a/core/modules/system/src/Tests/Form/StorageTest.php b/core/modules/system/src/Tests/Form/StorageTest.php -index 01189d1..c8a4dc8 100644 ---- a/core/modules/system/src/Tests/Form/StorageTest.php -+++ b/core/modules/system/src/Tests/Form/StorageTest.php -@@ -15,9 +15,9 @@ - * - * The tested form puts data into the storage during the initial form - * construction. These tests verify that there are no duplicate form -- * constructions, with and without manual form caching activiated. Furthermore -+ * constructions, with and without manual form caching activated. Furthermore - * when a validation error occurs, it makes sure that changed form element -- * values aren't lost due to a wrong form rebuild. -+ * values are not lost due to a wrong form rebuild. - * - * @group Form - */ -@@ -28,7 +28,7 @@ class StorageTest extends WebTestBase { - * - * @var array - */ -- public static $modules = array('form_test'); -+ public static $modules = array('form_test', 'dblog'); - - protected function setUp() { - parent::setUp(); -@@ -159,4 +159,81 @@ function testFormStatePersist() { - $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. -+ */ -+ public function testImmutableForm() { -+ // Request the form with 'cache' query parameter to enable form caching. -+ $this->drupalGet('form_test/form-storage', ['query' => ['cache' => 1, 'immutable' => 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 change. -+ $this->assertNoFieldByName('form_build_id', $buildId, 'Build id changes when form validation fails'); -+ -+ // Retrieve the new build-id. -+ $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 again submitting an empty title. -+ $edit = ['title' => '']; -+ $this->drupalPostForm(NULL, $edit, 'Continue submit'); -+ -+ // Verify that the build-id does not change the second time. -+ $this->assertFieldByName('form_build_id', $buildId, 'Build id remains the same when form validation fails subsequently'); -+ } -+ -+ /** -+ * Verify that existing contrib code cannot overwrite immutable form state. -+ */ -+ public function testImmutableFormLegacyProtection() { -+ $this->drupalGet('form_test/form-storage', ['query' => ['cache' => 1, 'immutable' => 1]]); -+ $build_id_fields = $this->xpath('//input[@name="form_build_id"]'); -+ $this->assertEqual(count($build_id_fields), 1, 'One form build id field on the page'); -+ $build_id = (string) $build_id_fields[0]['value']; -+ -+ // Try to poison the form cache. -+ $original = $this->drupalGetAJAX('form-test/form-storage-legacy/' . $build_id); -+ $this->assertEqual($original['form']['#build_id_old'], $build_id, 'Original build_id was recorded'); -+ $this->assertNotEqual($original['form']['#build_id'], $build_id, 'New build_id was generated'); -+ -+ // Assert that a watchdog message was logged by form_set_cache. -+ $status = (bool) db_query_range('SELECT 1 FROM {watchdog} WHERE message = :message', 0, 1, [':message' => 'Form build-id mismatch detected while attempting to store a form in the cache.']); -+ $this->assert($status, 'A watchdog message was logged by form_set_cache'); -+ -+ // Ensure that the form state was not poisoned by the preceeding call. -+ $original = $this->drupalGetAJAX('form-test/form-storage-legacy/' . $build_id); -+ $this->assertEqual($original['form']['#build_id_old'], $build_id, 'Original build_id was recorded'); -+ $this->assertNotEqual($original['form']['#build_id'], $build_id, 'New build_id was generated'); -+ $this->assert(empty($original['form']['#poisoned']), 'Original form structure was preserved'); -+ $this->assert(empty($original['form_state']['poisoned']), 'Original form state was preserved'); -+ } -+ - } -diff --git a/core/modules/system/tests/modules/form_test/form_test.routing.yml b/core/modules/system/tests/modules/form_test/form_test.routing.yml -index 109d8f3..bebb71c 100644 ---- a/core/modules/system/tests/modules/form_test/form_test.routing.yml -+++ b/core/modules/system/tests/modules/form_test/form_test.routing.yml -@@ -433,3 +433,19 @@ form_test.two_instances: - requirements: - _module_dependencies: 'node' - _permission: 'create page content' -+ -+form_test.storage_legacy_handler: -+ path: '/form-test/form-storage-legacy/{form_build_id}' -+ defaults: -+ _controller: '\Drupal\form_test\Controller\FormTestController::storageLegacyHandler' -+ form_build_id: NULL -+ requirements: -+ _access: 'TRUE' -+ -+form_test.form_storage_page_cache: -+ path: '/form-test/form-storage-page-cache' -+ defaults: -+ _form: '\Drupal\form_test\Form\FormTestStoragePageCacheForm' -+ _title: 'Form storage with page cache test' -+ requirements: -+ _access: 'TRUE' -diff --git a/core/modules/system/tests/modules/form_test/src/Controller/FormTestController.php b/core/modules/system/tests/modules/form_test/src/Controller/FormTestController.php -index 600b1d9..12a11cc 100644 ---- a/core/modules/system/tests/modules/form_test/src/Controller/FormTestController.php -+++ b/core/modules/system/tests/modules/form_test/src/Controller/FormTestController.php -@@ -7,7 +7,9 @@ - namespace Drupal\form_test\Controller; - - use Drupal\Core\Controller\ControllerBase; -+use Drupal\Core\Form\FormState; - use Drupal\Core\Language\LanguageInterface; -+use Symfony\Component\HttpFoundation\JsonResponse; - - /** - * Controller routines for form_test routes. -@@ -35,4 +37,25 @@ public function twoFormInstances() { - return $return; - } - -+ /** -+ * Emulate legacy AHAH-style ajax callback. -+ * -+ * Drupal 6 AHAH callbacks used to operate directly on forms retrieved using -+ * form_get_cache and stored using form_set_cache after manipulation. This -+ * callback helps testing whether form_set_cache prevents resaving of -+ * immutable forms. -+ */ -+ public function storageLegacyHandler($form_build_id) { -+ $form_state = new FormState(); -+ $form = $this->formBuilder()->getCache($form_build_id, $form_state); -+ $result = [ -+ 'form' => $form, -+ 'form_state' => $form_state, -+ ]; -+ $form['#poisoned'] = TRUE; -+ $form_state->set('poisoned', TRUE); -+ $this->formBuilder()->setCache($form_build_id, $form, $form_state); -+ return new JsonResponse($result); -+ } -+ - } -diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php -index c5f166b..60e0cdb 100644 ---- a/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php -+++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php -@@ -88,6 +88,10 @@ public function buildForm(array $form, FormStateInterface $form_state) { - $form_state->setCached(); - } - -+ if ($this->getRequest()->get('immutable')) { -+ $form_state->addBuildInfo('immutable', TRUE); -+ } -+ - return $form; - } - -diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php -new file mode 100644 -index 0000000..223ed80 ---- /dev/null -+++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php -@@ -0,0 +1,80 @@ -+ 'textfield', -+ '#title' => 'Title', -+ '#required' => TRUE, -+ ); -+ -+ $form['test_build_id_old'] = array( -+ '#type' => 'item', -+ '#title' => 'Old build id', -+ '#markup' => 'No old build id', -+ ); -+ -+ $form['submit'] = array( -+ '#type' => 'submit', -+ '#value' => 'Save', -+ ); -+ -+ $form['rebuild'] = array( -+ '#type' => 'submit', -+ '#value' => 'Rebuild', -+ '#submit' => array(array($this, 'form_test_storage_page_cache_rebuild')), -+ ); -+ -+ $form['#after_build'] = array(array($this, 'form_test_storage_page_cache_old_build_id')); -+ $form_state->setCached(); -+ -+ return $form; -+ } -+ -+ /** -+ * Form element #after_build callback: output the old form build-id. -+ */ -+ function form_test_storage_page_cache_old_build_id($form) { -+ if (isset($form['#build_id_old'])) { -+ $form['test_build_id_old']['#markup'] = String::checkPlain($form['#build_id_old']); -+ } -+ return $form; -+ } -+ -+ /** -+ * Form submit callback: Rebuild the form and continue. -+ */ -+ function form_test_storage_page_cache_rebuild($form, FormStateInterface $form_state) { -+ $form_state->setRebuild(); -+ } -+ -+ /** -+ * {@inheritdoc} -+ */ -+ public function submitForm(array &$form, FormStateInterface $form_state) { -+ // Nothing must happen. -+ } -+ -+} -diff --git a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -index d08557a..41d76df 100644 ---- a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -+++ b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -@@ -10,7 +10,6 @@ - use Drupal\Component\Utility\SafeMarkup; - use Drupal\Core\Form\FormCache; - use Drupal\Core\Form\FormState; --use Drupal\Core\Session\AccountInterface; - use Drupal\Tests\UnitTestCase; - - /** -@@ -69,6 +68,34 @@ class FormCacheTest extends UnitTestCase { - protected $formStateCacheStore; - - /** -+ * The logger channel. -+ * -+ * @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $logger; -+ -+ /** -+ * The config factory. -+ * -+ * @var \Drupal\Core\Config\ConfigFactoryInterface|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $configFactory; -+ -+ /** -+ * The request stack. -+ * -+ * @var \Symfony\Component\HttpFoundation\RequestStack|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $requestStack; -+ -+ /** -+ * A policy rule determining the cacheability of a request. -+ * -+ * @var \Drupal\Core\PageCache\RequestPolicyInterface|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $requestPolicy; -+ -+ /** - * {@inheritdoc} - */ - protected function setUp() { -@@ -92,7 +119,14 @@ protected function setUp() { - ->disableOriginalConstructor() - ->getMock(); - $this->account = $this->getMock('Drupal\Core\Session\AccountInterface'); -- $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken); -+ -+ $this->logger = $this->getMock('Psr\Log\LoggerInterface'); -+ $this->configFactory = $this->getConfigFactoryStub(['system.performance' => ['cache.page.use_internal' => FALSE]]); -+ $this->requestStack = $this->getMock('\Symfony\Component\HttpFoundation\RequestStack'); -+ $this->requestPolicy = $this->getMock('\Drupal\Core\PageCache\RequestPolicyInterface'); -+ -+ -+ $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory, $this->requestStack, $this->requestPolicy); - } - - /** -@@ -213,6 +247,32 @@ public function testGetCacheNoForm() { - } - - /** -+ * @covers ::getCache -+ */ -+ public function testGetCacheImmutableForm() { -+ $form_build_id = 'the_form_build_id'; -+ $form_state = (new FormState()) -+ ->addBuildInfo('immutable', TRUE); -+ $cached_form = [ -+ '#build_id' => 'the_old_build_form_id', -+ ]; -+ -+ $this->account->expects($this->once()) -+ ->method('isAnonymous') -+ ->willReturn(TRUE); -+ $this->formCacheStore->expects($this->once()) -+ ->method('get') -+ ->with($form_build_id) -+ ->willReturn($cached_form); -+ -+ $form = $this->formCache->getCache($form_build_id, $form_state); -+ $this->assertSame($cached_form['#build_id'], $form['#build_id_old']); -+ $this->assertNotSame($cached_form['#build_id'], $form['#build_id']); -+ $this->assertSame($form['#build_id'], $form['form_build_id']['#value']); -+ $this->assertSame($form['#build_id'], $form['form_build_id']['#id']); -+ } -+ -+ /** - * @covers ::loadCachedFormState - */ - public function testLoadCachedFormState() { -@@ -407,6 +467,63 @@ public function testSetCacheWithSafeStrings() { - } - - /** -+ * @covers ::setCache -+ */ -+ public function testSetCacheBuildIdMismatch() { -+ $form_build_id = 'the_form_build_id'; -+ $form = [ -+ '#form_id' => 'the_form_id', -+ '#build_id' => 'stale_form_build_id', -+ ]; -+ $form_state = new FormState(); -+ -+ $this->formCacheStore->expects($this->never()) -+ ->method('setWithExpire'); -+ $this->formStateCacheStore->expects($this->never()) -+ ->method('setWithExpire'); -+ $this->logger->expects($this->once()) -+ ->method('error') -+ ->with('Form build-id mismatch detected while attempting to store a form in the cache.'); -+ $this->formCache->setCache($form_build_id, $form, $form_state); -+ } -+ -+ /** -+ * @covers ::setCache -+ */ -+ public function testSetCacheImmutableForm() { -+ $form_build_id = 'the_form_build_id'; -+ $form = [ -+ '#form_id' => 'the_form_id', -+ ]; -+ $form_state = new FormState(); -+ -+ $this->formCacheStore->expects($this->once()) -+ ->method('setWithExpire') -+ ->with($form_build_id, $form, $this->isType('int')); -+ $form_state_data = $form_state->getCacheableArray(); -+ $form_state_data['build_info']['safe_strings'] = []; -+ // Ensure that the form is marked immutable. -+ $form_state_data['build_info']['immutable'] = TRUE; -+ $this->formStateCacheStore->expects($this->once()) -+ ->method('setWithExpire') -+ ->with($form_build_id, $form_state_data, $this->isType('int')); -+ -+ // Rebuild the FormCache with a config factory that will return a config -+ // object with the internal page cache enabled. -+ $this->configFactory = $this->getConfigFactoryStub(['system.performance' => ['cache.page.use_internal' => TRUE]]); -+ $this->formCache = $this->getMockBuilder('Drupal\Core\Form\FormCache') -+ ->setConstructorArgs([$this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory, $this->requestStack, $this->requestPolicy]) -+ ->setMethods(['isPageCacheable']) -+ ->getMock(); -+ -+ $this->formCache->expects($this->once()) -+ ->method('isPageCacheable') -+ ->willReturn(TRUE); -+ -+ $this->formCache->setCache($form_build_id, $form, $form_state); -+ } -+ -+ /** - * Ensures SafeMarkup does not bleed from one test to another. - */ - protected function resetSafeMarkup() { diff --git a/core/2242749-form-cache.interdiff.31-33.txt b/core/2242749-form-cache.interdiff.31-33.txt deleted file mode 100644 index 74a28a2..0000000 --- a/core/2242749-form-cache.interdiff.31-33.txt +++ /dev/null @@ -1,78 +0,0 @@ -diff --git a/core/lib/Drupal/Core/Form/FormCache.php b/core/lib/Drupal/Core/Form/FormCache.php -index 5c77c30..05c6cf0 100644 ---- a/core/lib/Drupal/Core/Form/FormCache.php -+++ b/core/lib/Drupal/Core/Form/FormCache.php -@@ -72,7 +72,7 @@ class FormCache implements FormCacheInterface { - * - * @var \Symfony\Component\HttpFoundation\RequestStack - */ -- protected $request; -+ protected $requestStack; - - /** - * A policy rule determining the cacheability of a request. -@@ -109,7 +109,7 @@ public function __construct(KeyValueExpirableFactoryInterface $key_value_expirab - $this->logger = $logger; - $this->configFactory = $config_factory; - $this->csrfToken = $csrf_token; -- $this->request = $request_stack; -+ $this->requestStack = $request_stack; - $this->requestPolicy = $request_policy; - } - -@@ -218,7 +218,7 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state) - * Checks if the page is cacheable. - */ - protected function isPageCacheable($allow_caching = NULL) { -- return ($this->requestPolicy->check($this->request) === RequestPolicyInterface::ALLOW); -+ return ($this->requestPolicy->check($this->requestStack->getCurrentRequest()) === RequestPolicyInterface::ALLOW); - } - - } -diff --git a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -index f76bbc4..41d76df 100644 ---- a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -+++ b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php -@@ -82,6 +82,20 @@ class FormCacheTest extends UnitTestCase { - protected $configFactory; - - /** -+ * The request stack. -+ * -+ * @var \Symfony\Component\HttpFoundation\RequestStack|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $requestStack; -+ -+ /** -+ * A policy rule determining the cacheability of a request. -+ * -+ * @var \Drupal\Core\PageCache\RequestPolicyInterface|\PHPUnit_Framework_MockObject_MockObject -+ */ -+ protected $requestPolicy; -+ -+ /** - * {@inheritdoc} - */ - protected function setUp() { -@@ -108,8 +122,11 @@ protected function setUp() { - - $this->logger = $this->getMock('Psr\Log\LoggerInterface'); - $this->configFactory = $this->getConfigFactoryStub(['system.performance' => ['cache.page.use_internal' => FALSE]]); -+ $this->requestStack = $this->getMock('\Symfony\Component\HttpFoundation\RequestStack'); -+ $this->requestPolicy = $this->getMock('\Drupal\Core\PageCache\RequestPolicyInterface'); -+ - -- $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory); -+ $this->formCache = new FormCache($this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory, $this->requestStack, $this->requestPolicy); - } - - /** -@@ -495,7 +512,7 @@ public function testSetCacheImmutableForm() { - // object with the internal page cache enabled. - $this->configFactory = $this->getConfigFactoryStub(['system.performance' => ['cache.page.use_internal' => TRUE]]); - $this->formCache = $this->getMockBuilder('Drupal\Core\Form\FormCache') -- ->setConstructorArgs([$this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory]) -+ ->setConstructorArgs([$this->keyValueExpirableFactory, $this->moduleHandler, $this->account, $this->csrfToken, $this->logger, $this->configFactory, $this->requestStack, $this->requestPolicy]) - ->setMethods(['isPageCacheable']) - ->getMock(); - diff --git a/core/lib/Drupal/Core/Ajax/AjaxForm.php b/core/lib/Drupal/Core/Ajax/AjaxForm.php index b40538c..d2f9bb4 100644 --- a/core/lib/Drupal/Core/Ajax/AjaxForm.php +++ b/core/lib/Drupal/Core/Ajax/AjaxForm.php @@ -1,38 +1,52 @@ form = $form; @@ -60,13 +74,18 @@ public function __construct(array $form, FormStateInterface $form_state, $form_i } /** + * Gets all AJAX commands. + * * @return \Drupal\Core\Ajax\CommandInterface[] + * Returns all previously added AJAX commands. */ public function getCommands() { return $this->commands; } /** + * Gets the form definition. + * * @return array */ public function getForm() { @@ -74,6 +93,8 @@ public function getForm() { } /** + * Gets the unique form build ID. + * * @return string */ public function getFormBuildId() { @@ -81,6 +102,8 @@ public function getFormBuildId() { } /** + * Gets the unique form ID. + * * @return string */ public function getFormId() { @@ -88,6 +111,8 @@ public function getFormId() { } /** + * Gets the form state. + * * @return \Drupal\Core\Form\FormStateInterface */ public function getFormState() { diff --git a/core/modules/system/src/Controller/FormAjaxController.php b/core/modules/system/src/Controller/FormAjaxController.php index 08a1f40..317eaaa 100644 --- a/core/modules/system/src/Controller/FormAjaxController.php +++ b/core/modules/system/src/Controller/FormAjaxController.php @@ -106,13 +106,9 @@ public function content(Request $request) { * @param \Symfony\Component\HttpFoundation\Request $request * The current request object. * - * @return array - * An array containing the $form, $form_state, $form_id, $form_build_id and an - * initial list of Ajax $commands. Use the list() function to break these - * apart: - * @code - * list($form, $form_state, $form_id, $form_build_id, $commands) = $this->getForm(); - * @endcode + * @return \Drupal\Core\Ajax\AjaxForm + * 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 */