Remove flood-based redirect loop detection.

md-systems · May 4, 2015 · 899f259 · 899f259
1 parent 76f0698
commit 899f259
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 73 deletions.
diff --git a/redirect.services.yml b/redirect.services.yml
@@ -6,7 +6,7 @@ services:
       - { name: backend_overridable }
   redirect.checker:
     class: Drupal\redirect\RedirectChecker
-    arguments: ['@config.factory', '@state', '@flood']
+    arguments: ['@config.factory', '@state']
   redirect.request_subscriber:
     class: Drupal\redirect\EventSubscriber\RedirectRequestSubscriber
     arguments: ['@redirect.repository', '@language_manager', '@config.factory', '@redirect.checker', '@router.request_context']

diff --git a/src/EventSubscriber/RedirectRequestSubscriber.php b/src/EventSubscriber/RedirectRequestSubscriber.php
@@ -91,8 +91,7 @@ public function onKernelRequestCheckRedirect(GetResponseEvent $event) {
     $this->context->fromRequest($request);
 
     try {
-      $redirect = $this->redirectRepository->findMatchingRedirect($path, $request_query, $this->languageManager->getCurrentLanguage()
-        ->getId());
+      $redirect = $this->redirectRepository->findMatchingRedirect($path, $request_query, $this->languageManager->getCurrentLanguage()->getId());
     }
     catch (RedirectLoopException $e) {
       \Drupal::logger('redirect')->warning($e->getMessage());
@@ -105,16 +104,6 @@ public function onKernelRequestCheckRedirect(GetResponseEvent $event) {
 
     if (!empty($redirect)) {
 
-      // If we are in a loop log it and send 503 response.
-      if ($this->checker->isLoop($request)) {
-        \Drupal::logger('redirect')->warning('Redirect loop identified at %path for redirect %id', array('%path' => $request->getRequestUri(), '%id' => $redirect->id()));
-        $response = new Response();
-        $response->setStatusCode(503);
-        $response->setContent('Service unavailable');
-        $event->setResponse($response);
-        return;
-      }
-
       // Handle internal path.
       $url = $redirect->getRedirectUrl();
       if ($this->config->get('passthrough_querystring')) {

diff --git a/src/RedirectChecker.php b/src/RedirectChecker.php
@@ -7,7 +7,6 @@
 namespace Drupal\redirect;
 
 use Drupal\Core\Config\ConfigFactoryInterface;
-use Drupal\Core\Flood\FloodInterface;
 use Drupal\Core\State\StateInterface;
 use Symfony\Cmf\Component\Routing\RouteObjectInterface;
 use Symfony\Component\HttpFoundation\Request;
@@ -27,15 +26,9 @@ class RedirectChecker {
    */
   protected $state;
 
-  /**
-   * @var \Drupal\Core\Flood\FloodInterface
-   */
-  protected $flood;
-
-  public function __construct(ConfigFactoryInterface $config, StateInterface $state, FloodInterface $flood) {
+  public function __construct(ConfigFactoryInterface $config, StateInterface $state) {
     $this->config = $config->get('redirect.settings');
     $this->state = $state;
-    $this->flood = $flood;
   }
 
   /**
@@ -75,22 +68,4 @@ public function canRedirect(Request $request) {
     return $can_redirect;
   }
 
-  /**
-   * Checks if for the current session we are not in a loop.
-   *
-   * @param Request $request
-   *   Current request object.
-   *
-   * @return bool
-   *   TRUE if a redirect loop has been identified.
-   */
-  public function isLoop(Request $request) {
-    if ($this->flood->isAllowed('redirection', 5, 15)) {
-      $this->flood->register('redirection', 60);
-      return FALSE;
-    }
-    else {
-      return TRUE;
-    }
-  }
 }
diff --git a/tests/src/Unit/RedirectCheckerTest.php b/tests/src/Unit/RedirectCheckerTest.php
@@ -34,10 +34,7 @@ public function testCanRedirect() {
       ->with('system.maintenance_mode')
       ->will($this->returnValue(FALSE));
 
-    $flood = $this->getMockBuilder('Drupal\Core\Flood\FloodInterface')
-      ->getMock();
-
-    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state, $flood);
+    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state);
 
     // All fine - we can redirect.
     $request = $this->getRequestStub('index.php', 'GET');
@@ -59,7 +56,7 @@ public function testCanRedirect() {
       ->with('system.maintenance_mode')
       ->will($this->returnValue(TRUE));
 
-    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state, $flood);
+    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state);
 
     $request = $this->getRequestStub('index.php', 'GET');
     $this->assertFalse($checker->canRedirect($request), 'Cannot redirect if maintenance mode is on');
@@ -72,7 +69,7 @@ public function testCanRedirect() {
       ->with('system.maintenance_mode')
       ->will($this->returnValue(FALSE));
 
-    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state, $flood);
+    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state);
 
     $route = $this->getMockBuilder('Symfony\Component\Routing\Route')
       ->disableOriginalConstructor()
@@ -88,40 +85,13 @@ public function testCanRedirect() {
 
     // We are at admin path with global_admin_paths set to TRUE.
     $config['redirect.settings']['global_admin_paths'] = TRUE;
-    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state, $flood);
+    $checker = new RedirectChecker($this->getConfigFactoryStub($config), $state);
 
     $request = $this->getRequestStub('index.php', 'GET',
       array(RouteObjectInterface::ROUTE_OBJECT => $route));
     $this->assertTrue($checker->canRedirect($request), 'Can redirect a admin with global_admin_paths set to TRUE');
   }
 
-  /**
-   * Tests the loop checker.
-   */
-  public function testIsLoop() {
-    $state = $this->getMockBuilder('Drupal\Core\State\StateInterface')
-      ->getMock();
-    $request = $this->getRequestStub('index.php', 'GET');
-
-    $flood = $this->getMockBuilder('Drupal\Core\Flood\FloodInterface')
-      ->getMock();
-    $flood->expects($this->any())
-      ->method('isAllowed')
-      ->will($this->returnValue(TRUE));
-
-    $checker = new RedirectChecker($this->getConfigFactoryStub(), $state, $flood);
-    $this->assertFalse($checker->isLoop($request), 'Not in a loop');
-
-    $flood = $this->getMockBuilder('Drupal\Core\Flood\FloodInterface')
-      ->getMock();
-    $flood->expects($this->any())
-      ->method('isAllowed')
-      ->will($this->returnValue(FALSE));
-
-    $checker = new RedirectChecker($this->getConfigFactoryStub(), $state, $flood);
-    $this->assertTrue($checker->isLoop($request), 'In a loop');
-  }
-
   /**
    * Gets request mock object.
    *
@@ -149,4 +119,5 @@ protected function getRequestStub($script_name, $method, array $attributes = arr
 
     return $request;
   }
+
 }