diff --git a/core/lib/Drupal/Component/Utility/UrlHelper.php b/core/lib/Drupal/Component/Utility/UrlHelper.php index c286db954be7208b12b9856d7e9913bd4ca36d26..4ff656a737296044c24499882b2fd399c4718c75 100644 --- a/core/lib/Drupal/Component/Utility/UrlHelper.php +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php @@ -224,21 +224,29 @@ public static function isExternal($path) { * @param string $base_url * The base URL string to check against, such as "http://example.com/" * - * @return + * @return bool * TRUE if the URL has the same domain and base path. + * + * @throws \InvalidArgumentException + * Exception thrown when a either $url or $bath_url are not fully qualified. */ public static function externalIsLocal($url, $base_url) { $url_parts = parse_url($url); - $base_host = parse_url($base_url, PHP_URL_HOST); + $base_parts = parse_url($base_url); + + if (empty($base_parts['host']) || empty($url_parts['host'])) { + throw new \InvalidArgumentException(String::format('A path was passed when a fully qualified domain was expected.')); + } - if (!isset($url_parts['path'])) { - return ($url_parts['host'] == $base_host); + if (!isset($url_parts['path']) || !isset($base_parts['path'])) { + return (!isset($base_parts['path']) || $base_parts['path'] == '/') + && ($url_parts['host'] == $base_parts['host']); } else { // When comparing base paths, we need a trailing slash to make sure a // partial URL match isn't occurring. 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_url) === 0); + return ($url_parts['host'] == $base_parts['host'] && stripos($url_parts['path'], $base_parts['path']) === 0); } } diff --git a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php index c7687facf6906a15d82974986a10fd8718928a37..c67fbf5a4928c4a9a05f277d39df3100edef59d0 100644 --- a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php @@ -40,7 +40,7 @@ public function __construct(UrlGeneratorInterface $url_generator) { /** * Allows manipulation of the response object when performing a redirect. * - * @param Symfony\Component\HttpKernel\Event\FilterResponseEvent $event + * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event * The Event to process. */ public function checkRedirectUrl(FilterResponseEvent $event) { @@ -55,7 +55,7 @@ public function checkRedirectUrl(FilterResponseEvent $event) { // the following exception: // - Absolute URLs that point to this site (i.e. same base URL and // base path) are allowed. - if ($destination && (!UrlHelper::isExternal($destination) || UrlHelper::externalIsLocal($destination, base_path()))) { + if ($destination && (!UrlHelper::isExternal($destination) || UrlHelper::externalIsLocal($destination, $GLOBALS['base_url']))) { $destination = UrlHelper::parse($destination); $path = $destination['path']; diff --git a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php index 6fa2e4e81560d648eaeda4a568f2473f73827c22..ba92c161d507de9a7d09a10454e18a8aa305aa5b 100644 --- a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php @@ -465,4 +465,89 @@ protected function dataEnhanceWithPrefix(array $urls) { return $data; } + /** + * Test detecting external urls that point to local resources. + * + * @param string $url + * The external url to test. + * @param string $base_url + * The base url. + * @param bool $expected + * TRUE if an external URL points to this installation as determined by the + * base url. + * + * @covers ::externalIsLocal + * @dataProvider providerTestExternalIsLocal + */ + public function testExternalIsLocal($url, $base_url, $expected) { + $this->assertSame($expected, UrlHelper::externalIsLocal($url, $base_url)); + } + + /** + * Provider for local external url detection. + * + * @see \Drupal\Tests\Component\Utility\UrlHelperTest::testExternalIsLocal() + */ + public function providerTestExternalIsLocal() { + return array( + // Different mixes of trailing slash. + array('http://example.com', 'http://example.com', TRUE), + array('http://example.com/', 'http://example.com', TRUE), + array('http://example.com', 'http://example.com/', TRUE), + array('http://example.com/', 'http://example.com/', TRUE), + // Sub directory of site. + array('http://example.com/foo', 'http://example.com/', TRUE), + array('http://example.com/foo/bar', 'http://example.com/foo', TRUE), + array('http://example.com/foo/bar', 'http://example.com/foo/', TRUE), + // Different sub-domain. + array('http://example.com', 'http://www.example.com/', FALSE), + array('http://example.com/', 'http://www.example.com/', FALSE), + array('http://example.com/foo', 'http://www.example.com/', FALSE), + // Different TLD. + array('http://example.com', 'http://example.ca', FALSE), + array('http://example.com', 'http://example.ca/', FALSE), + array('http://example.com/', 'http://example.ca/', FALSE), + array('http://example.com/foo', 'http://example.ca', FALSE), + array('http://example.com/foo', 'http://example.ca/', FALSE), + // Different site path. + array('http://example.com/foo', 'http://example.com/bar', FALSE), + array('http://example.com', 'http://example.com/bar', FALSE), + array('http://example.com/bar', 'http://example.com/bar/', FALSE), + ); + } + + /** + * Test invalid url arguments. + * + * @param string $url + * The url to test. + * @param string $base_url + * The base url. + * + * @covers ::externalIsLocal + * @dataProvider providerTestExternalIsLocalInvalid + * @expectedException \InvalidArgumentException + */ + public function testExternalIsLocalInvalid($url, $base_url) { + UrlHelper::externalIsLocal($url, $base_url); + } + + /** + * Provides invalid argument data for local external url detection. + * + * @see \Drupal\Tests\Component\Utility\UrlHelperTest::testExternalIsLocalInvalid() + */ + public function providerTestExternalIsLocalInvalid() { + return array( + array('http://example.com/foo', ''), + array('http://example.com/foo', 'bar'), + array('http://example.com/foo', 'http://'), + // Invalid destination urls. + array('', 'http://example.com/foo'), + array('bar', 'http://example.com/foo'), + array('/bar', 'http://example.com/foo'), + array('bar/', 'http://example.com/foo'), + array('http://', 'http://example.com/foo'), + ); + } } diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php new file mode 100644 index 0000000000000000000000000000000000000000..db093f8c202ccb7f14e481a349bb4696a2430efb --- /dev/null +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php @@ -0,0 +1,113 @@ +<?php + +/** + * @file + * Contains \Drupal\Tests\Core\EventSubscriber\RedirectResponseSubscriberTest. + */ + +namespace Drupal\Tests\Core\EventSubscriber; + +use Drupal\Core\EventSubscriber\RedirectResponseSubscriber; +use Drupal\Tests\UnitTestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Event\FilterResponseEvent; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\HttpKernel\KernelEvents; + +/** + * Tests response redirection using destination get argument. + * + * @group Drupal + * @group Routing + * + * @coversDefaultClass \Drupal\Core\EventSubscriber\RedirectResponseSubscriber + */ +class RedirectResponseSubscriberTest extends UnitTestCase { + + /** + * {@inheritdoc} + */ + public static function getInfo() { + return array( + 'name' => 'Tests response redirection using destination get argument.', + 'description' => '', + 'group' => 'Routing' + ); + } + + /** + * {@inheritdoc} + */ + public function setUp() { + parent::setUp(); + $GLOBALS['base_url'] = 'http://example.com/drupal'; + } + + /** + * {@inheritdoc} + */ + public function tearDown() { + unset($GLOBALS['base_url']); + parent::tearDown(); + } + + /** + * Test destination detection and redirection. + * + * @param Request $request + * The request object with destination query set. + * @param bool $expected + * Whether or not a redirect will occur. + * + * @covers ::checkRedirectUrl + * @dataProvider providerTestDestinationRedirect + */ + public function testDestinationRedirect(Request $request, $expected) { + $dispatcher = new EventDispatcher(); + $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'); + $response = new RedirectResponse('http://example.com/drupal'); + $url_generator = $this->getMockBuilder('Drupal\Core\Routing\UrlGenerator') + ->disableOriginalConstructor() + ->setMethods(array('generateFromPath')) + ->getMock(); + + if ($expected) { + $url_generator + ->expects($this->once()) + ->method('generateFromPath') + ->will($this->returnValue('success')); + } + + $listener = new RedirectResponseSubscriber($url_generator); + $dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl')); + $event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response); + $dispatcher->dispatch(KernelEvents::RESPONSE, $event); + + $target_url = $event->getResponse()->getTargetUrl(); + if ($expected) { + $this->assertEquals('success', $target_url); + } + else { + $this->assertEquals('http://example.com/drupal', $target_url); + } + } + + /** + * Data provider for testDestinationRedirect(). + * + * @see \Drupal\Tests\Core\EventSubscriber\RedirectResponseSubscriberTest::testDestinationRedirect() + */ + public static function providerTestDestinationRedirect() { + return array( + array(new Request(), FALSE), + array(new Request(array('destination' => 'http://example.com')), FALSE), + array(new Request(array('destination' => 'http://example.com/foobar')), FALSE), + array(new Request(array('destination' => 'http://example.ca/drupal')), FALSE), + array(new Request(array('destination' => 'test')), TRUE), + array(new Request(array('destination' => 'http://example.com/drupal/')), TRUE), + array(new Request(array('destination' => 'http://example.com/drupal/test')), TRUE), + ); + } +}