Skip to content

Commit

Permalink
Detect redirect loops before redirecting.
Browse files Browse the repository at this point in the history
- Redirects are searched for recursively to avoid loops.
- Addresses issue md-systems#16.
  • Loading branch information
jhedstrom committed May 4, 2015
1 parent 404401c commit 76f0698
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 4 deletions.
14 changes: 13 additions & 1 deletion src/EventSubscriber/RedirectRequestSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\Core\Routing\UrlGenerator;
use Drupal\redirect\Exception\RedirectLoopException;
use Drupal\redirect\RedirectChecker;
use Drupal\redirect\RedirectRepository;
use Symfony\Component\HttpFoundation\RedirectResponse;
Expand Down Expand Up @@ -89,7 +90,18 @@ public function onKernelRequestCheckRedirect(GetResponseEvent $event) {

$this->context->fromRequest($request);

$redirect = $this->redirectRepository->findMatchingRedirect($path, $request_query, $this->languageManager->getCurrentLanguage()->getId());
try {
$redirect = $this->redirectRepository->findMatchingRedirect($path, $request_query, $this->languageManager->getCurrentLanguage()
->getId());
}
catch (RedirectLoopException $e) {
\Drupal::logger('redirect')->warning($e->getMessage());
$response = new Response();
$response->setStatusCode(503);
$response->setContent('Service unavailable');
$event->setResponse($response);
return;
}

if (!empty($redirect)) {

Expand Down
29 changes: 29 additions & 0 deletions src/Exception/RedirectLoopException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/**
* @file
* Contains \Drupal\redirect\Exception\RedirectLoopException.
*/

namespace Drupal\redirect\Exception;

use Drupal\Component\Utility\SafeMarkup;

/**
* Exception for when a redirect loop is detected.
*/
class RedirectLoopException extends \RuntimeException {

/**
* Formats a redirect loop exception message.
*
* @param string $path
* The path that results in a redirect loop.
* @param int $rid
* The redirect ID that is involved in a loop.
*/
public function __construct($path, $rid) {
parent::__construct(SafeMarkup::format('Redirect loop identified at %path for redirect %rid', ['%path' => $path, '%rid' => $rid]));
}

}
43 changes: 41 additions & 2 deletions src/RedirectRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Drupal\Core\Entity\EntityManagerInterface;
use Drupal\Core\Language\Language;
use Drupal\redirect\Entity\Redirect;
use Drupal\redirect\Exception\RedirectLoopException;

class RedirectRepository {

Expand All @@ -24,6 +25,13 @@ class RedirectRepository {
*/
protected $connection;

/**
* An array of found redirect IDs to avoid recursion.
*
* @var array
*/
protected $foundRedirects = [];

/**
* Constructs a \Drupal\redirect\EventSubscriber\RedirectRequestSubscriber object.
*
Expand All @@ -49,9 +57,10 @@ public function __construct(EntityManagerInterface $manager, Connection $connect
*
* @return \Drupal\redirect\Entity\Redirect
* The matched redirect entity.
*
* @throws \Drupal\redirect\Exception\RedirectLoopException
*/
public function findMatchingRedirect($source_path, array $query = [], $language = Language::LANGCODE_NOT_SPECIFIED) {

$hashes = [Redirect::generateHash($source_path, $query, $language)];
if ($language != Language::LANGCODE_NOT_SPECIFIED) {
$hashes[] = Redirect::generateHash($source_path, $query, Language::LANGCODE_NOT_SPECIFIED);
Expand All @@ -61,12 +70,42 @@ public function findMatchingRedirect($source_path, array $query = [], $language
$rid = $this->connection->query('SELECT rid FROM {redirect} WHERE hash IN (:hashes[])', [':hashes[]' => $hashes])->fetchField();

if (!empty($rid)) {
return $this->load($rid);
// Check if this is a loop.
if (in_array($rid, $this->foundRedirects)) {
throw new RedirectLoopException('/' . $source_path, $rid);
}
$this->foundRedirects[] = $rid;

$redirect = $this->load($rid);

// Find chained redirects.
if ($recursive = $this->findByRedirect($redirect, $language)) {
// Reset found redirects.
$this->foundRedirects = [];
return $recursive;
}

return $redirect;
}

return NULL;
}

/**
* Helper function to find recursive redirects.
*
* @param \Drupal\redirect\Entity\Redirect
* The redirect object.
* @param string $language
* The language to use.
*/
protected function findByRedirect(Redirect $redirect, $language) {
$uri = $redirect->getRedirectUrl();
$path = ltrim(parse_url($uri->toString(), PHP_URL_PATH), '/');
$query = $uri->getOption('query') ?: [];
return $this->findMatchingRedirect($path, $query, $language);
}

/**
* Finds redirects based on the source path.
*
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/RedirectUITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ function testRedirectLoop() {
$log = reset($log);
$this->assertEqual($log->severity, RfcLogLevel::WARNING);
$this->assertEqual(SafeMarkup::format($log->message, unserialize($log->variables)),
SafeMarkup::format('Redirect loop identified at %path for redirect %id', array('%path' => '/admin', '%id' => $redirect2->id())));
SafeMarkup::format('Redirect loop identified at %path for redirect %id', array('%path' => '/node', '%id' => $redirect1->id())));
}
}

Expand Down

0 comments on commit 76f0698

Please sign in to comment.