From 484b071dd4ab7e9a9a5e956924ac22496f920d1d Mon Sep 17 00:00:00 2001
From: Nathaniel Catchpole <catch@35733.no-reply.drupal.org>
Date: Mon, 20 Jul 2015 12:57:44 +0100
Subject: [PATCH] Issue #2507831 by dawehner, tim.plunkett, effulgentsia:
 Harden redirect responses to make external URIs opt in (was SA-CORE-2015-002
 foward-port)

---
 .../SecuredRedirectResponse.php               |  61 ++++++++
 .../RedirectResponseSubscriber.php            | 108 +++++++++-----
 .../LocalAwareRedirectResponseTrait.php       |  57 ++++++++
 .../Core/Routing/LocalRedirectResponse.php    |  21 +++
 .../Core/Routing/TrustedRedirectResponse.php  |  56 +++++++
 .../field_ui/src/Tests/ManageFieldsTest.php   |  14 ++
 .../RedirectResponseSubscriberTest.php        | 138 ++++++++++++++++--
 .../Routing/TrustedRedirectResponseTest.php   |  57 ++++++++
 8 files changed, 466 insertions(+), 46 deletions(-)
 create mode 100644 core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php
 create mode 100644 core/lib/Drupal/Core/Routing/LocalAwareRedirectResponseTrait.php
 create mode 100644 core/lib/Drupal/Core/Routing/LocalRedirectResponse.php
 create mode 100644 core/lib/Drupal/Core/Routing/TrustedRedirectResponse.php
 create mode 100644 core/tests/Drupal/Tests/Core/Routing/TrustedRedirectResponseTest.php

diff --git a/core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php b/core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php
new file mode 100644
index 000000000000..77c978c7d7a8
--- /dev/null
+++ b/core/lib/Drupal/Component/HttpFoundation/SecuredRedirectResponse.php
@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Component\HttpFoundation\SecuredRedirectResponse.
+ */
+
+namespace Drupal\Component\HttpFoundation;
+
+use \Symfony\Component\HttpFoundation\RedirectResponse;
+
+/**
+ * Provides a common base class for safe redirects.
+ *
+ * In case you want to redirect to external URLs use
+ * TrustedRedirectResponse.
+ *
+ * For local URLs we use LocalRedirectResponse which opts
+ * out of external redirects.
+ */
+abstract class SecuredRedirectResponse extends RedirectResponse {
+
+  /**
+   * Copies an existing redirect response into a safe one.
+   *
+   * The safe one cannot accidentally redirect to an external URL, unless
+   * actively wanted (see TrustedRedirectResponse).
+   *
+   * @param \Symfony\Component\HttpFoundation\RedirectResponse $response
+   *   The original redirect.
+   *
+   * @return static
+   */
+  public static function createFromRedirectResponse(RedirectResponse $response) {
+    $safe_response = new static($response->getTargetUrl(), $response->getStatusCode(), $response->headers->allPreserveCase());
+    $safe_response->setProtocolVersion($response->getProtocolVersion());
+    $safe_response->setCharset($response->getCharset());
+    return $safe_response;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setTargetUrl($url) {
+    if (!$this->isSafe($url)) {
+      throw new \InvalidArgumentException(sprintf('It is not safe to redirect to %s', $url));
+    }
+    return parent::setTargetUrl($url);
+  }
+
+  /**
+   * Returns whether the URL is considered as safe to redirect to.
+   *
+   * @param string $url
+   *   The URL checked for safety.
+   *
+   * @return bool
+   */
+  abstract protected function isSafe($url);
+
+}
diff --git a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
index 0573eea259a6..efaf5c8026dc 100644
--- a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
@@ -7,13 +7,15 @@
 
 namespace Drupal\Core\EventSubscriber;
 
+use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
 use Drupal\Component\Utility\UrlHelper;
+use Drupal\Core\Routing\LocalRedirectResponse;
 use Drupal\Core\Routing\RequestContext;
 use Drupal\Core\Routing\UrlGeneratorInterface;
+use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
-use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
@@ -51,46 +53,86 @@ public function __construct(UrlGeneratorInterface $url_generator, RequestContext
   public function checkRedirectUrl(FilterResponseEvent $event) {
     $response = $event->getResponse();
     if ($response instanceOf RedirectResponse) {
-      $options = array();
-
       $request = $event->getRequest();
+
+      // Let the 'destination' query parameter override the redirect target.
+      // If $response is already a SecuredRedirectResponse, it might reject the
+      // new target as invalid, in which case proceed with the old target.
       $destination = $request->query->get('destination');
-      // A destination from \Drupal::request()->query always overrides the
-      // current RedirectResponse. We do not allow absolute URLs to be passed
-      // via \Drupal::request()->query, as this can be an attack vector, with
-      // the following exception:
-      // - Absolute URLs that point to this site (i.e. same base URL and
-      //   base path) are allowed.
       if ($destination) {
-        if (!UrlHelper::isExternal($destination)) {
-          // The destination query parameter can be a relative URL in the sense
-          // of not including the scheme and host, but its path is expected to
-          // be absolute (start with a '/'). For such a case, prepend the
-          // scheme and host, because the 'Location' header must be absolute.
-          if (strpos($destination, '/') === 0) {
-            $destination = $request->getSchemeAndHttpHost() . $destination;
-          }
-          else {
-            // Legacy destination query parameters can be relative paths that
-            // have not yet been converted to URLs (outbound path processors
-            // and other URL handling still needs to be performed).
-            // @todo As generateFromPath() is deprecated, remove this in
-            //   https://www.drupal.org/node/2418219.
-            $destination = UrlHelper::parse($destination);
-            $path = $destination['path'];
-            $options['query'] = $destination['query'];
-            $options['fragment'] = $destination['fragment'];
-            // The 'Location' HTTP header must always be absolute.
-            $options['absolute'] = TRUE;
-            $destination = $this->urlGenerator->generateFromPath($path, $options);
-          }
+        // The 'Location' HTTP header must always be absolute.
+        $destination = $this->getDestinationAsAbsoluteUrl($destination, $request->getSchemeAndHttpHost());
+        try {
           $response->setTargetUrl($destination);
         }
-        elseif (UrlHelper::externalIsLocal($destination, $this->requestContext->getCompleteBaseUrl())) {
-          $response->setTargetUrl($destination);
+        catch (\InvalidArgumentException $e) {
+        }
+      }
+
+      // Regardless of whether the target is the original one or the overridden
+      // destination, ensure that all redirects are safe.
+      if (!($response instanceOf SecuredRedirectResponse)) {
+        try {
+          // SecuredRedirectResponse is an abstract class that requires a
+          // concrete implementation. Default to LocalRedirectResponse, which
+          // considers only redirects to within the same site as safe.
+          $safe_response = LocalRedirectResponse::createFromRedirectResponse($response);
+          $safe_response->setRequestContext($this->requestContext);
         }
+        catch (\InvalidArgumentException $e) {
+          // If the above failed, it's because the redirect target wasn't
+          // local. Do not follow that redirect. Display an error message
+          // instead. We're already catching one exception, so trigger_error()
+          // rather than throw another one.
+          // We don't throw an exception, because this is a client error rather than a
+          // server error.
+          $message = 'Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.';
+          trigger_error($message, E_USER_ERROR);
+          $safe_response = new Response($message, 400);
+        }
+        $event->setResponse($safe_response);
+      }
+    }
+  }
+
+  /**
+   * Converts the passed in destination into an absolute URL.
+   *
+   * @param string $destination
+   *   The path for the destination. In case it starts with a slash it should
+   *   have the base path included already.
+   * @param string $scheme_and_host
+   *   The scheme and host string of the current request.
+   *
+   * @return string
+   *   The destination as absolute URL.
+   */
+  protected function getDestinationAsAbsoluteUrl($destination, $scheme_and_host) {
+    if (!UrlHelper::isExternal($destination)) {
+      // The destination query parameter can be a relative URL in the sense of
+      // not including the scheme and host, but its path is expected to be
+      // absolute (start with a '/'). For such a case, prepend the scheme and
+      // host, because the 'Location' header must be absolute.
+      if (strpos($destination, '/') === 0) {
+        $destination = $scheme_and_host . $destination;
+      }
+      else {
+        // Legacy destination query parameters can be relative paths that have
+        // not yet been converted to URLs (outbound path processors and other
+        // URL handling still needs to be performed).
+        // @todo As generateFromPath() is deprecated, remove this in
+        //   https://www.drupal.org/node/2418219.
+        $destination = UrlHelper::parse($destination);
+        $path = $destination['path'];
+        $options = [
+          'query' => $destination['query'],
+          'fragment' => $destination['fragment'],
+          'absolute' => TRUE,
+        ];
+        $destination = $this->urlGenerator->generateFromPath($path, $options);
       }
     }
+    return $destination;
   }
 
   /**
diff --git a/core/lib/Drupal/Core/Routing/LocalAwareRedirectResponseTrait.php b/core/lib/Drupal/Core/Routing/LocalAwareRedirectResponseTrait.php
new file mode 100644
index 000000000000..8ffa2f67782f
--- /dev/null
+++ b/core/lib/Drupal/Core/Routing/LocalAwareRedirectResponseTrait.php
@@ -0,0 +1,57 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\LocalAwareRedirectResponseTrait.
+ */
+
+namespace Drupal\Core\Routing;
+
+use Drupal\Component\Utility\UrlHelper;
+
+/**
+ * Provides a trait which ensures that a URL is safe to redirect to.
+ */
+trait LocalAwareRedirectResponseTrait {
+
+  /**
+   * The request context.
+   *
+   * @var \Drupal\Core\Routing\RequestContext
+   */
+  protected $requestContext;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function isLocal($url) {
+    return !UrlHelper::isExternal($url) || UrlHelper::externalIsLocal($url, $this->getRequestContext()->getCompleteBaseUrl());
+  }
+
+  /**
+   * Returns the request context.
+   *
+   * @return \Drupal\Core\Routing\RequestContext
+   */
+  protected function getRequestContext() {
+    if (!isset($this->requestContext)) {
+      $this->requestContext = \Drupal::service('router.request_context');
+    }
+    return $this->requestContext;
+  }
+
+  /**
+   * Sets the request context.
+   *
+   * @param \Drupal\Core\Routing\RequestContext $request_context
+   *   The request context.
+   *
+   * @return $this
+   */
+  public function setRequestContext(RequestContext $request_context) {
+    $this->requestContext = $request_context;
+
+    return $this;
+  }
+
+}
diff --git a/core/lib/Drupal/Core/Routing/LocalRedirectResponse.php b/core/lib/Drupal/Core/Routing/LocalRedirectResponse.php
new file mode 100644
index 000000000000..d8ad719bcc69
--- /dev/null
+++ b/core/lib/Drupal/Core/Routing/LocalRedirectResponse.php
@@ -0,0 +1,21 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\LocalRedirectResponse.
+ */
+
+namespace Drupal\Core\Routing;
+
+use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
+
+/**
+ * Provides a redirect response which cannot redirect to an external URL.
+ */
+class LocalRedirectResponse extends SecuredRedirectResponse {
+
+  use LocalAwareRedirectResponseTrait {
+    LocalAwareRedirectResponseTrait::isLocal as isSafe;
+  }
+
+}
diff --git a/core/lib/Drupal/Core/Routing/TrustedRedirectResponse.php b/core/lib/Drupal/Core/Routing/TrustedRedirectResponse.php
new file mode 100644
index 000000000000..c7a43a21c6b5
--- /dev/null
+++ b/core/lib/Drupal/Core/Routing/TrustedRedirectResponse.php
@@ -0,0 +1,56 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\TrustedRedirectResponse.
+ */
+
+namespace Drupal\Core\Routing;
+
+use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
+
+/**
+ * Provides a redirect response which contains trusted URLs.
+ *
+ * Use this class in case you know that you want to redirect to an external URL.
+ */
+class TrustedRedirectResponse extends SecuredRedirectResponse {
+
+  use LocalAwareRedirectResponseTrait;
+
+  /**
+   * A list of trusted URLs, which are safe to redirect to.
+   *
+   * @var string[]
+   */
+  protected $trustedUrls = array();
+
+  /**
+   * {@inheritdoc}
+   */
+  public function __construct($url, $status = 302, $headers = array()) {
+    $this->trustedUrls[$url] = TRUE;
+    parent::__construct($url, $status, $headers);
+  }
+
+  /**
+   * Sets the target URL to a trusted URL.
+   *
+   * @param string $url
+   *   A trusted URL.
+   *
+   * @return $this
+   */
+  public function setTrustedTargetUrl($url) {
+    $this->trustedUrls[$url] = TRUE;
+    return $this->setTargetUrl($url);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function isSafe($url) {
+    return !empty($this->trustedUrls[$url]) || $this->isLocal($url);
+  }
+
+}
diff --git a/core/modules/field_ui/src/Tests/ManageFieldsTest.php b/core/modules/field_ui/src/Tests/ManageFieldsTest.php
index 3b90bffc5614..8bd86885ca1e 100644
--- a/core/modules/field_ui/src/Tests/ManageFieldsTest.php
+++ b/core/modules/field_ui/src/Tests/ManageFieldsTest.php
@@ -616,6 +616,20 @@ function testDuplicateFieldName() {
     $this->assertUrl($url, array(), 'Stayed on the same page.');
   }
 
+  /**
+   * Tests that external URLs in the 'destinations' query parameter are blocked.
+   */
+  public function testExternalDestinations() {
+    $options = [
+      'query' => ['destinations' => ['http://example.com']],
+    ];
+    $this->drupalPostForm('admin/structure/types/manage/article/fields/node.article.body/storage', [], 'Save field settings', $options);
+    // The external redirect should not fire.
+    $this->assertUrl('admin/structure/types/manage/article/fields/node.article.body/storage', $options);
+    $this->assertResponse(200);
+    $this->assertRaw('Attempt to update field <em class="placeholder">Body</em> failed: <em class="placeholder">The internal path component "http://example.com" is external. You are not allowed to specify an external URL together with internal:/.</em>.');
+  }
+
   /**
    * Tests that deletion removes field storages and fields as expected for a term.
    */
diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
index e1002056bd5b..193c25b0cc8e 100644
--- a/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
+++ b/core/tests/Drupal/Tests/Core/EventSubscriber/RedirectResponseSubscriberTest.php
@@ -9,7 +9,9 @@
 
 use Drupal\Core\EventSubscriber\RedirectResponseSubscriber;
 use Drupal\Core\Routing\RequestContext;
+use Drupal\Core\Routing\TrustedRedirectResponse;
 use Drupal\Tests\UnitTestCase;
+use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -24,6 +26,31 @@
  */
 class RedirectResponseSubscriberTest extends UnitTestCase {
 
+  /**
+   * The mocked request context.
+   *
+   * @var \Drupal\Core\Routing\RequestContext|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $requestContext;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->requestContext = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $this->requestContext->expects($this->any())
+      ->method('getCompleteBaseUrl')
+      ->willReturn('http://example.com/drupal');
+
+    $container = new Container();
+    $container->set('router.request_context', $this->requestContext);
+    \Drupal::setContainer($container);
+  }
+
   /**
    * Test destination detection and redirection.
    *
@@ -57,15 +84,9 @@ public function testDestinationRedirect(Request $request, $expected) {
           ]);
     }
 
-    $request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
-      ->disableOriginalConstructor()
-      ->getMock();
-    $request_context->expects($this->any())
-      ->method('getCompleteBaseUrl')
-      ->willReturn('http://example.com/drupal');
     $request->headers->set('HOST', 'example.com');
 
-    $listener = new RedirectResponseSubscriber($url_generator, $request_context);
+    $listener = new RedirectResponseSubscriber($url_generator, $this->requestContext);
     $dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
     $event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
     $dispatcher->dispatch(KernelEvents::RESPONSE, $event);
@@ -87,12 +108,9 @@ public function testDestinationRedirect(Request $request, $expected) {
   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')), 'http://example.com/drupal/test'),
-      array(new Request(array('destination' => '/test')), 'http://example.com/test'),
-      array(new Request(array('destination' => '/example.com')), 'http://example.com/example.com'),
+      array(new Request(array('destination' => '/drupal/test')), 'http://example.com/drupal/test'),
+      array(new Request(array('destination' => 'example.com')), 'http://example.com/drupal/example.com'),
       array(new Request(array('destination' => 'example:com')), 'http://example.com/drupal/example:com'),
       array(new Request(array('destination' => 'javascript:alert(0)')), 'http://example.com/drupal/javascript:alert(0)'),
       array(new Request(array('destination' => 'http://example.com/drupal/')), 'http://example.com/drupal/'),
@@ -101,7 +119,94 @@ public static function providerTestDestinationRedirect() {
   }
 
   /**
-   * @expectedException \InvalidArgumentException
+   * @dataProvider providerTestDestinationRedirectToExternalUrl
+   *
+   * @expectedException \PHPUnit_Framework_Error
+   */
+  public function testDestinationRedirectToExternalUrl($request, $expected) {
+    $dispatcher = new EventDispatcher();
+    $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+    $response = new RedirectResponse('http://other-example.com');
+    $url_generator = $this->getMockBuilder('Drupal\Core\Routing\UrlGenerator')
+      ->disableOriginalConstructor()
+      ->setMethods(array('generateFromPath'))
+      ->getMock();
+
+    $request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $request_context->expects($this->any())
+      ->method('getCompleteBaseUrl')
+      ->willReturn('http://example.com/drupal');
+
+    if ($expected) {
+      $url_generator
+        ->expects($this->any())
+        ->method('generateFromPath')
+        ->willReturnMap([
+          ['test', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/test'],
+          ['example.com', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/example.com'],
+          ['example:com', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/example:com'],
+          ['javascript:alert(0)', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/javascript:alert(0)'],
+          ['/test', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/test'],
+        ]);
+    }
+
+    $listener = new RedirectResponseSubscriber($url_generator, $request_context);
+    $dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
+    $event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
+    $dispatcher->dispatch(KernelEvents::RESPONSE, $event);
+
+    $this->assertEquals(400, $event->getResponse()->getStatusCode());
+  }
+
+  /**
+   * @covers ::checkRedirectUrl
+   */
+  public function testRedirectWithOptInExternalUrl() {
+    $dispatcher = new EventDispatcher();
+    $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+    $response = new TrustedRedirectResponse('http://external-url.com');
+    $url_generator = $this->getMockBuilder('Drupal\Core\Routing\UrlGenerator')
+      ->disableOriginalConstructor()
+      ->setMethods(array('generateFromPath'))
+      ->getMock();
+
+    $request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
+      ->disableOriginalConstructor()
+      ->getMock();
+    $request_context->expects($this->any())
+      ->method('getCompleteBaseUrl')
+      ->willReturn('http://example.com/drupal');
+
+    $request = Request::create('');
+    $request->headers->set('HOST', 'example.com');
+
+    $listener = new RedirectResponseSubscriber($url_generator, $request_context);
+    $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();
+    $this->assertEquals('http://external-url.com', $target_url);
+  }
+
+  /**
+   * Data provider for testDestinationRedirectToExternalUrl().
+   */
+  public function providerTestDestinationRedirectToExternalUrl() {
+    return [
+      'absolute external url' => [new Request(['destination' => 'http://example.com']), 'http://example.com'],
+      'absolute external url with folder' => [new Request(['destination' => 'http://example.com/foobar']), 'http://example.com/foobar'],
+      'absolute external url with folder2' => [new Request(['destination' => 'http://example.ca/drupal']), 'http://example.ca/drupal'],
+      'path without drupal basepath' => [new Request(['destination' => '/test']), 'http://example.com/test'],
+      'path with URL' => [new Request(['destination' => '/example.com']), 'http://example.com/example.com'],
+      'path with URL and two slashes' => [new Request(['destination' => '//example.com']), 'http://example.com//example.com'],
+    ];
+  }
+
+  /**
+   * @expectedException \PHPUnit_Framework_Error
    *
    * @dataProvider providerTestDestinationRedirectWithInvalidUrl
    */
@@ -119,6 +224,8 @@ public function testDestinationRedirectWithInvalidUrl(Request $request) {
     $dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
     $event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
     $dispatcher->dispatch(KernelEvents::RESPONSE, $event);
+
+    $this->assertEquals(400, $event->getResponse()->getStatusCode());
   }
 
   /**
@@ -128,6 +235,11 @@ public function providerTestDestinationRedirectWithInvalidUrl() {
     $data = [];
     $data[] = [new Request(array('destination' => '//example:com'))];
     $data[] = [new Request(array('destination' => '//example:com/test'))];
+    $data['absolute external url'] = [new Request(['destination' => 'http://example.com'])];
+    $data['absolute external url with folder'] = [new Request(['destination' => 'http://example.ca/drupal'])];
+    $data['path without drupal basepath'] = [new Request(['destination' => '/test'])];
+    $data['path with URL'] = [new Request(['destination' => '/example.com'])];
+    $data['path with URL and two slashes'] = [new Request(['destination' => '//example.com'])];
 
     return $data;
   }
diff --git a/core/tests/Drupal/Tests/Core/Routing/TrustedRedirectResponseTest.php b/core/tests/Drupal/Tests/Core/Routing/TrustedRedirectResponseTest.php
new file mode 100644
index 000000000000..4ac1333fac8c
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Routing/TrustedRedirectResponseTest.php
@@ -0,0 +1,57 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Routing\TrustedRedirectResponseTest.
+ */
+
+namespace Drupal\Tests\Core\Routing;
+
+use Drupal\Core\Routing\RequestContext;
+use Drupal\Core\Routing\TrustedRedirectResponse;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Routing\TrustedRedirectResponse
+ * @group Routing
+ */
+class TrustedRedirectResponseTest extends UnitTestCase {
+
+  /**
+   * @covers ::setTargetUrl
+   */
+  public function testSetTargetUrlWithInternalUrl() {
+    $redirect_response = new TrustedRedirectResponse('/example');
+    $redirect_response->setTargetUrl('/example2');
+
+    $this->assertEquals('/example2', $redirect_response->getTargetUrl());
+  }
+
+  /**
+   * @covers ::setTargetUrl
+   * @expectedException \InvalidArgumentException
+   */
+  public function testSetTargetUrlWithUntrustedUrl() {
+    $request_context = new RequestContext();
+    $request_context->setCompleteBaseUrl('https://www.drupal.org');
+    $container = new ContainerBuilder();
+    $container->set('router.request_context', $request_context);
+    \Drupal::setContainer($container);
+
+    $redirect_response = new TrustedRedirectResponse('/example');
+
+    $redirect_response->setTargetUrl('http://evil-url.com/example');
+  }
+
+  /**
+   * @covers ::setTargetUrl
+   */
+  public function testSetTargetUrlWithTrustedUrl() {
+    $redirect_response = new TrustedRedirectResponse('/example');
+
+    $redirect_response->setTrustedTargetUrl('http://good-external-url.com/example');
+    $this->assertEquals('http://good-external-url.com/example', $redirect_response->getTargetUrl());
+  }
+
+}
-- 
GitLab