diff --git a/includes/common.inc b/includes/common.inc index 5c6d86d..e127854 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -686,7 +686,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. // We do not allow absolute URLs to be passed via $_GET, as this can be an attack vector. - if (isset($_GET['destination']) && !url_is_external($_GET['destination'])) { + if (isset($_GET['destination']) && (!url_is_external($_GET['destination']) || _external_url_is_local($_GET['destination']))) { $destination = drupal_parse_url($_GET['destination']); $path = $destination['path']; $options['query'] = $destination['query']; @@ -709,6 +709,25 @@ function drupal_goto($path = '', array $options = array(), $http_response_code = } /** + * Determines if an external URL points to this Drupal installation. + * + * @param $url + * A string containing an external URL, such as "http://example.com/foo". + * + * @return + * TRUE if the URL has the same domain and base path. + */ +function _external_url_is_local($url) { + $url_parts = parse_url($url); + $base_host = parse_url($GLOBALS['base_url'], PHP_URL_HOST); + + // When comparing base paths, we need a trailing slash to make sure a + // partial URL match isn't occuring. Since base_path() always returns with + // a trailing slash, we don't need to add the trailing slash here. + return ($url_parts['host'] == $base_host && stripos($url_parts['path'], base_path()) === 0); +} + +/** * Delivers a "site is under maintenance" message to the browser. * * Page callback functions wanting to report a "site offline" message should diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test index e8e4033..4fb2562 100644 --- a/modules/simpletest/tests/common.test +++ b/modules/simpletest/tests/common.test @@ -1133,12 +1133,26 @@ class DrupalGotoTest extends DrupalWebTestCase { $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 + // Test that drupal_goto() respects ?destination=xxx. Use a 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 that drupal_goto() respects ?destination=xxx with an absolute URL + // that points to this Drupal installation. + $destination = url('common-test/drupal_goto/alt', array('absolute' => TRUE)); + $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination))); + $this->assertText('drupal_goto_alt', 'Drupal goto redirect with absolute URL destination that points to this Drupal installation succeeded.'); + $this->assertEqual($this->getUrl(), url('common-test/drupal_goto/alt', array('absolute' => TRUE)), 'Drupal goto redirected to given query string destination with absolute URL that points to this Drupal installation.'); + + // Test that drupal_goto() fails to respect ?destination=xxx with an absolute URL + // that does not point to this Drupal installation. + $destination = 'http://pagedoesnotexist'; + $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination))); + $this->assertText('drupal_goto', 'Drupal goto fails to redirect with absolute URL destination that does not point to this Drupal installation.'); + $this->assertNotEqual($this->getUrl(), $destination, 'Drupal goto failed to redirect to given query string destination with absolute URL that does not point to this Drupal installation.'); } /** diff --git a/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module index e75b452..ce2e4e4 100644 --- a/modules/simpletest/tests/common_test.module +++ b/modules/simpletest/tests/common_test.module @@ -15,6 +15,12 @@ function common_test_menu() { 'access arguments' => array('access content'), 'type' => MENU_CALLBACK, ); + $items['common-test/drupal_goto/alt'] = array( + 'title' => 'Drupal Goto', + 'page callback' => 'common_test_drupal_goto_land_alt', + 'access arguments' => array('access content'), + 'type' => MENU_CALLBACK, + ); $items['common-test/drupal_goto/fail'] = array( 'title' => 'Drupal Goto', 'page callback' => 'common_test_drupal_goto_land_fail', @@ -77,6 +83,15 @@ function common_test_drupal_goto_land() { } /** + * Page callback: Provides a landing page for drupal_goto(). + * + * @see common_test_menu() + */ +function common_test_drupal_goto_land_alt() { + print "drupal_goto_alt"; +} + +/** * Fail landing page for drupal_goto(). */ function common_test_drupal_goto_land_fail() {