- Patch #796120 by c960657: do not urldecode() parameters in drupal_goto().

4 files changed, 65 insertions, 32 deletions

diff --git a/includes/common.inc b/includes/common.inc
index 57bff0233..452013589 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -660,7 +660,7 @@ function drupal_encode_path($path) {
 function drupal_goto($path = '', array $options = array(), $http_response_code = 302) {
   // A destination in $_GET always overrides the function arguments.
   if (isset($_GET['destination'])) {
-    $destination = drupal_parse_url(urldecode($_GET['destination']));
+    $destination = drupal_parse_url($_GET['destination']);
     $path = $destination['path'];
     $options['query'] = $destination['query'];
     $options['fragment'] = $destination['fragment'];
diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test
index 07a72463c..7728b8d8f 100644
--- a/modules/simpletest/tests/common.test
+++ b/modules/simpletest/tests/common.test
@@ -929,19 +929,6 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
     $redirect_307 = drupal_http_request(url('system-test/redirect/307', array('absolute' => TRUE)), array('max_redirects' => 0));
     $this->assertFalse(isset($redirect_307->redirect_code), t('drupal_http_request does not follow 307 redirect if max_redirects = 0.'));
   }
-
-  function testDrupalGetDestination() {
-    $query = $this->randomName(10);
-
-    // Verify that a 'destination' query string is used as destination.
-    $this->drupalGet('system-test/destination', array('query' => array('destination' => $query)));
-    $this->assertText('The destination: ' . $query, t('The given query string destination is determined as destination.'));
-
-    // Verify that the current path is used as destination.
-    $this->drupalGet('system-test/destination', array('query' => array($query => NULL)));
-    $url = 'system-test/destination?' . $query;
-    $this->assertText('The destination: ' . $url, t('The current path is determined as destination.'));
-  }
 }
 
 /**
@@ -1007,17 +994,33 @@ class DrupalGotoTest extends DrupalWebTestCase {
   }
 
   /**
-   * Test setting and retrieving content for theme regions.
+   * Test drupal_goto().
    */
   function testDrupalGoto() {
     $this->drupalGet('common-test/drupal_goto/redirect');
+    $headers = $this->drupalGetHeaders(TRUE);
+    list(, $status) = explode(' ', $headers[0][':status'], 3);
+    $this->assertEqual($status, 302, t('Expected response code was sent.'));
+    $this->assertText('drupal_goto', t('Drupal goto redirect succeeded.'));
+    $this->assertEqual($this->getUrl(), url('common-test/drupal_goto', array('absolute' => TRUE)), t('Drupal goto redirected to expected URL.'));
 
-    $this->assertNoText(t("Drupal goto failed to stop program"), t("Drupal goto stopped program."));
-    $this->assertText('drupal_goto', t("Drupal goto redirect failed."));
+    $this->drupalGet('common-test/drupal_goto/redirect_advanced');
+    $headers = $this->drupalGetHeaders(TRUE);
+    list(, $status) = explode(' ', $headers[0][':status'], 3);
+    $this->assertEqual($status, 301, t('Expected response code was sent.'));
+    $this->assertText('drupal_goto', t('Drupal goto redirect succeeded.'));
+    $this->assertEqual($this->getUrl(), url('common-test/drupal_goto', array('query' => array('foo' => '123'), 'absolute' => TRUE)), t('Drupal goto redirected to expected URL.'));
+
+    // Test that drupal_goto() respects ?destination=xxx. Use an complicated URL
+    // to test that the path is encoded and decoded properly.
+    $destination = 'common-test/drupal_goto/destination?foo=%2525&bar=123';
+    $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination)));
+    $this->assertText('drupal_goto', t('Drupal goto redirect with destination succeeded.'));
+    $this->assertEqual($this->getUrl(), url('common-test/drupal_goto/destination', array('query' => array('foo' => '%25', 'bar' => '123'), 'absolute' => TRUE)), t('Drupal goto redirected to given query string destination. '));
   }
 
   /**
-   * Test setting and retrieving content for theme regions.
+   * Test hook_drupal_goto_alter().
    */
   function testDrupalGotoAlter() {
     $this->drupalGet('common-test/drupal_goto/redirect_fail');
@@ -1025,6 +1028,22 @@ class DrupalGotoTest extends DrupalWebTestCase {
     $this->assertNoText(t("Drupal goto failed to stop program"), t("Drupal goto stopped program."));
     $this->assertNoText('drupal_goto_fail', t("Drupal goto redirect failed."));
   }
+
+  /**
+   * Test drupal_get_destination().
+   */
+  function testDrupalGetDestination() {
+    $query = $this->randomName(10);
+
+    // Verify that a 'destination' query string is used as destination.
+    $this->drupalGet('common-test/destination', array('query' => array('destination' => $query)));
+    $this->assertText('The destination: ' . $query, t('The given query string destination is determined as destination.'));
+
+    // Verify that the current path is used as destination.
+    $this->drupalGet('common-test/destination', array('query' => array($query => NULL)));
+    $url = 'common-test/destination?' . $query;
+    $this->assertText('The destination: ' . $url, t('The current path is determined as destination.'));
+  }
 }
 
 /**
diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module
index a80f00f74..1b5dbc351 100644
--- a/modules/simpletest/tests/common_test.module
+++ b/modules/simpletest/tests/common_test.module
@@ -28,6 +28,12 @@ function common_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_CALLBACK,
   );
+  $items['common-test/drupal_goto/redirect_advanced'] = array(
+    'title' => 'Drupal Goto',
+    'page callback' => 'common_test_drupal_goto_redirect_advanced',
+    'access arguments' => array('access content'),
+    'type' => MENU_CALLBACK,
+  );
   $items['common-test/drupal_goto/redirect_fail'] = array(
     'title' => 'Drupal Goto Failure',
     'page callback' => 'drupal_goto',
@@ -35,6 +41,12 @@ function common_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_CALLBACK,
   );
+  $items['common-test/destination'] = array(
+    'title' => 'Drupal Get Destination',
+    'page callback' => 'common_test_destination',
+    'access arguments' => array('access content'),
+    'type' => MENU_CALLBACK,
+  );
   $items['common-test/query-string'] = array(
     'title' => 'Test querystring',
     'page callback' => 'common_test_js_and_css_querystring',
@@ -45,11 +57,17 @@ function common_test_menu() {
 }
 
 /**
- * Check that drupal_goto() exits once called.
+ * Redirect using drupal_goto().
  */
 function common_test_drupal_goto_redirect() {
   drupal_goto('common-test/drupal_goto');
-  print t("Drupal goto failed to stop program");
+}
+
+/**
+ * Redirect using drupal_goto().
+ */
+function common_test_drupal_goto_redirect_advanced() {
+  drupal_goto('common-test/drupal_goto', array('query' => array('foo' => '123')), 301);
 }
 
 /**
@@ -76,6 +94,14 @@ function common_test_drupal_goto_alter(&$path, &$options, &$http_response_code)
 }
 
 /**
+ * Print destination query parameter.
+ */
+function common_test_destination() {
+  $destination = drupal_get_destination();
+  print "The destination: " . check_plain($destination['destination']);
+}
+
+/**
  * Implements hook_TYPE_alter().
  */
 function common_test_drupal_alter_alter(&$data, &$arg2 = NULL, &$arg3 = NULL) {
diff --git a/modules/simpletest/tests/system_test.module b/modules/simpletest/tests/system_test.module
index 24167ced4..1f1e535c6 100644
--- a/modules/simpletest/tests/system_test.module
+++ b/modules/simpletest/tests/system_test.module
@@ -43,13 +43,6 @@ function system_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_CALLBACK,
   );
-  $items['system-test/destination'] = array(
-    'title' => 'Redirect',
-    'page callback' => 'system_test_destination',
-    'page arguments' => array(2),
-    'access arguments' => array('access content'),
-    'type' => MENU_CALLBACK,
-  );
 
   $items['system-test/variable-get'] = array(
     'title' => 'Variable Get',
@@ -143,11 +136,6 @@ function system_test_redirect_invalid_scheme() {
   exit;
 }
 
-function system_test_destination() {
-  $destination = drupal_get_destination();
-  return 'The destination: ' . $destination['destination'];
-}
-
 /**
  * Implements hook_modules_installed().
  */