Issue #2206909 by jhodgdon, dawehner: Regression: Form submit redirects from 403/404 pages are no longer possible.

core/lib/Drupal/Core/Controller/ExceptionController.php

@@ -141,14 +141,8 @@ class ExceptionController extends HtmlControllerBase implements ContainerAwareIn
     $system_config = $this->container->get('config.factory')->get('system.site');
     $path = $this->container->get('path.alias_manager')->getSystemPath($system_config->get('page.403'));
     if ($path && $path != $system_path) {
-      // Keep old path for reference, and to allow forms to redirect to it.
-      if (!$request->query->has('destination')) {
-        $request->query->set('destination', $system_path);
-      }
-
       if ($request->getMethod() === 'POST') {
         $subrequest = Request::create($request->getBaseUrl() . '/' . $path, 'POST', array('destination' => $system_path, '_exception_statuscode' => 403) + $request->request->all(), $request->cookies->all(), array(), $request->server->all());
-        $subrequest->query->set('destination', $system_path);
       }
       else {
         $subrequest = Request::create($request->getBaseUrl() . '/' . $path, 'GET', array('destination' => $system_path, '_exception_statuscode' => 403), $request->cookies->all(), array(), $request->server->all());
@@ -217,21 +211,14 @@ class ExceptionController extends HtmlControllerBase implements ContainerAwareIn
 
     $system_path = $request->attributes->get('_system_path');
 
-    // Keep old path for reference, and to allow forms to redirect to it.
-    if (!$request->query->has('destination')) {
-      $request->query->set('destination', $system_path);
-    }
-
     $path = $this->container->get('path.alias_manager')->getSystemPath(\Drupal::config('system.site')->get('page.404'));
     if ($path && $path != $system_path) {
       // @todo Um, how do I specify an override URL again? Totally not clear. Do
       //   that and sub-call the kernel rather than using meah().
       // @todo The create() method expects a slash-prefixed path, but we store a
       //   normal system path in the site_404 variable.
-
       if ($request->getMethod() === 'POST') {
         $subrequest = Request::create($request->getBaseUrl() . '/' . $path, 'POST', array('destination' => $system_path, '_exception_statuscode' => 404) + $request->request->all(), $request->cookies->all(), array(), $request->server->all());
-        $subrequest->query->set('destination', $system_path);
       }
       else {
         $subrequest = Request::create($request->getBaseUrl() . '/' . $path, 'GET', array('destination' => $system_path, '_exception_statuscode' => 404), $request->cookies->all(), array(), $request->server->all());

core/modules/system/lib/Drupal/system/Tests/Form/RedirectTest.php

@@ -19,12 +19,12 @@ class RedirectTest extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('form_test');
+  public static $modules = array('form_test', 'block');
 
   public static function getInfo() {
     return array(
       'name' => 'Form redirecting',
-      'description' => 'Tests functionality of drupal_redirect_form().',
+      'description' => 'Tests form redirection functionality.',
       'group' => 'Form API',
     );
   }
@@ -85,4 +85,32 @@ class RedirectTest extends WebTestBase {
     $this->assertUrl($path, $options, 'When using an empty redirection string, there should be no redirection, and the query parameters should be passed along.');
   }
 
+  /**
+   * Tests form redirection from 404/403 pages with the Block form.
+   */
+  public function testRedirectFromErrorPages() {
+    // Make sure the block containing the redirect form is placed.
+    $this->drupalPlaceBlock('redirect_form_block');
+
+    // Create a user that does not have permission to administer blocks.
+    $user = $this->drupalCreateUser(array('administer themes'));
+    $this->drupalLogin($user);
+
+    // Visit page 'foo' (404 page) and submit the form. Verify it ends up
+    // at the right URL.
+    $expected = \Drupal::url('form_test.route1', array(), array('query' => array('test1' => 'test2'), 'absolute' => TRUE));
+    $this->drupalGet('foo');
+    $this->assertResponse(404);
+    $this->drupalPostForm(NULL, array(), t('Submit'));
+    $this->assertResponse(200);
+    $this->assertEqual($this->getUrl(), $expected, 'Redirected to correct url/query.');
+
+    // Visit the block admin page (403 page) and submit the form. Verify it
+    // ends up at the right URL.
+    $this->drupalGet('admin/structure/block');
+    $this->assertResponse(403);
+    $this->drupalPostForm(NULL, array(), t('Submit'));
+    $this->assertResponse(200);
+    $this->assertEqual($this->getUrl(), $expected, 'Redirected to correct url/query.');
+  }
 }

core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Form/RedirectBlockForm.php

@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\form_test\Form\RedirectBlockForm.
+ */
+
+namespace Drupal\form_test\Form;
+
+use Drupal\Core\Form\FormBase;
+use Drupal\Core\Url;
+
+/**
+ * Builds a simple form that redirects on submit.
+ *
+ * @see \Drupal\form_test\Plugin\Block\RedirectFormBlock
+ */
+class RedirectBlockForm extends FormBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFormId() {
+    return 'redirect_block_form';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function buildForm(array $form, array &$form_state) {
+    $form['actions'] = array('#type' => 'actions');
+    $form['actions']['submit'] = array('#type' => 'submit', '#value' => $this->t('Submit'));
+
+    return $form;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function submitForm(array &$form, array &$form_state) {
+    $form_state['redirect_route'] = new Url('form_test.route1', array(), array('query' => array('test1' => 'test2')));
+  }
+}

core/modules/system/tests/modules/form_test/lib/Drupal/form_test/Plugin/Block/RedirectFormBlock.php

@@ -0,0 +1,79 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\form_test\Plugin\Block\RedirectFormBlock.
+ */
+
+namespace Drupal\form_test\Plugin\Block;
+
+use Drupal\block\BlockBase;
+use Drupal\Core\Form\FormBuilderInterface;
+use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
+use Drupal\Core\Session\AccountInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Provides a block containing a simple redirect form.
+ *
+ * @see \Drupal\form_test\Form\RedirectBlockForm
+ *
+ * @Block(
+ *   id = "redirect_form_block",
+ *   admin_label = @Translation("Redirecting form"),
+ *   category = @Translation("Forms")
+ * )
+ */
+class RedirectFormBlock extends BlockBase implements ContainerFactoryPluginInterface {
+
+  /**
+   * The form builder.
+   *
+   * @var \Drupal\Core\Form\FormBuilderInterface
+   */
+  protected $formBuilder;
+
+  /**
+   * Constructs a new RedirectFormBlock.
+   *
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin_id for the plugin instance.
+   * @param mixed $plugin_definition
+   *   The plugin implementation definition.
+   * @param \Drupal\Core\Form\FormBuilderInterface $form_builder
+   *   The form builder.
+   */
+  public function __construct(array $configuration, $plugin_id, $plugin_definition, FormBuilderInterface $form_builder) {
+    parent::__construct($configuration, $plugin_id, $plugin_definition);
+
+    $this->formBuilder = $form_builder;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
+    return new static(
+      $configuration,
+      $plugin_id,
+      $plugin_definition,
+      $container->get('form_builder')
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function access(AccountInterface $account) {
+    return TRUE;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function build() {
+    return $this->formBuilder->getForm('Drupal\form_test\Form\RedirectBlockForm');
+  }
+}

core/modules/user/lib/Drupal/user/Form/UserLoginForm.php

@@ -116,10 +116,17 @@ class UserLoginForm extends FormBase {
    */
   public function submitForm(array &$form, array &$form_state) {
     $account = $this->userStorage->load($form_state['uid']);
-    $form_state['redirect_route'] = array(
+
-      'route_name' => 'user.view',
+    // A destination was set, probably on an exception controller,
-      'route_parameters' => array('user' => $account->id()),
+    if (!$this->request->request->has('destination')) {
-    );
+      $form_state['redirect_route'] = array(
+        'route_name' => 'user.view',
+        'route_parameters' => array('user' => $account->id()),
+      );
+    }
+    else {
+      $this->request->query->set('destination', $this->request->request->get('destination'));
+    }
 
     user_login_finalize($account);
   }

core/modules/user/lib/Drupal/user/Tests/UserLoginTest.php

@@ -22,6 +22,18 @@ class UserLoginTest extends WebTestBase {
     );
   }
 
+  /**
+   * Tests login with destination.
+   */
+  function testLoginDestination() {
+    $user = $this->drupalCreateUser(array());
+    $this->drupalGet('user', array('query' => array('destination' => 'foo')));
+    $edit = array('name' => $user->getUserName(), 'pass' => $user->pass_raw);
+    $this->drupalPostForm(NULL, $edit, t('Log in'));
+    $expected = url('foo', array('absolute' => TRUE));
+    $this->assertEqual($this->getUrl(), $expected, 'Redirected to the correct URL');
+  }
+
   /**
    * Test the global login flood control.
    */