ENGCOM-6995: Fix #20309 - URL Rewrites redirect loop #26902

magento · Mar 6, 2020 · 7ee65f0 · 7ee65f0
2 parents 5f191e1 + a12d705
commit 7ee65f0
Show file tree

Hide file tree

Showing 7 changed files with 428 additions and 306 deletions.
diff --git a/app/code/Magento/UrlRewrite/Controller/Router.php b/app/code/Magento/UrlRewrite/Controller/Router.php
@@ -3,15 +3,22 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\UrlRewrite\Controller;
 
+use Magento\Framework\App\Action\Forward;
 use Magento\Framework\App\Action\Redirect;
+use Magento\Framework\App\ActionFactory;
 use Magento\Framework\App\ActionInterface;
 use Magento\Framework\App\Request\Http as HttpRequest;
 use Magento\Framework\App\RequestInterface;
 use Magento\Framework\App\Response\Http as HttpResponse;
+use Magento\Framework\App\ResponseInterface;
+use Magento\Framework\App\RouterInterface;
 use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Framework\UrlInterface;
+use Magento\Store\Model\StoreManagerInterface;
 use Magento\UrlRewrite\Controller\Adminhtml\Url\Rewrite;
 use Magento\UrlRewrite\Model\UrlFinderInterface;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
@@ -21,10 +28,10 @@
  *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-class Router implements \Magento\Framework\App\RouterInterface
+class Router implements RouterInterface
 {
     /**
-     * @var \Magento\Framework\App\ActionFactory
+     * @var ActionFactory
      */
     protected $actionFactory;
 
@@ -34,7 +41,7 @@ class Router implements \Magento\Framework\App\RouterInterface
     protected $url;
 
     /**
-     * @var \Magento\Store\Model\StoreManagerInterface
+     * @var StoreManagerInterface
      */
     protected $storeManager;
 
@@ -49,17 +56,17 @@ class Router implements \Magento\Framework\App\RouterInterface
     protected $urlFinder;
 
     /**
-     * @param \Magento\Framework\App\ActionFactory $actionFactory
+     * @param ActionFactory $actionFactory
      * @param UrlInterface $url
-     * @param \Magento\Store\Model\StoreManagerInterface $storeManager
-     * @param \Magento\Framework\App\ResponseInterface $response
+     * @param StoreManagerInterface $storeManager
+     * @param ResponseInterface $response
      * @param UrlFinderInterface $urlFinder
      */
     public function __construct(
-        \Magento\Framework\App\ActionFactory $actionFactory,
+        ActionFactory $actionFactory,
         UrlInterface $url,
-        \Magento\Store\Model\StoreManagerInterface $storeManager,
-        \Magento\Framework\App\ResponseInterface $response,
+        StoreManagerInterface $storeManager,
+        ResponseInterface $response,
         UrlFinderInterface $urlFinder
     ) {
         $this->actionFactory = $actionFactory;
@@ -84,23 +91,41 @@ public function match(RequestInterface $request)
         );
 
         if ($rewrite === null) {
-            //No rewrite rule matching current URl found, continuing with
-            //processing of this URL.
+            // No rewrite rule matching current URl found, continuing with
+            // processing of this URL.
             return null;
         }
+
+        $requestStringTrimmed = ltrim($request->getRequestString(), '/');
+        $rewriteRequestPath = $rewrite->getRequestPath();
+        $rewriteTargetPath = $rewrite->getTargetPath();
+        $rewriteTargetPathTrimmed = ltrim($rewriteTargetPath, '/');
+
+        if (preg_replace('/\?.*/', '', $rewriteRequestPath) === preg_replace('/\?.*/', '', $rewriteTargetPath) &&
+            (
+                !$requestStringTrimmed ||
+                !$rewriteTargetPathTrimmed ||
+                strpos($requestStringTrimmed, $rewriteTargetPathTrimmed) === 0
+            )
+        ) {
+            // Request and target paths of rewrite found without query params are equal and current request string
+            // starts with request target path, continuing with processing of this URL.
+            return null;
+        }
+
         if ($rewrite->getRedirectType()) {
-            //Rule requires the request to be redirected to another URL
-            //and cannot be processed further.
+            // Rule requires the request to be redirected to another URL
+            // and cannot be processed further.
             return $this->processRedirect($request, $rewrite);
         }
-        //Rule provides actual URL that can be processed by a controller.
+        // Rule provides actual URL that can be processed by a controller.
         $request->setAlias(
             UrlInterface::REWRITE_REQUEST_PATH_ALIAS,
-            $rewrite->getRequestPath()
+            $rewriteRequestPath
         );
-        $request->setPathInfo('/' . $rewrite->getTargetPath());
+        $request->setPathInfo('/' . $rewriteTargetPath);
         return $this->actionFactory->create(
-            \Magento\Framework\App\Action\Forward::class
+            Forward::class
         );
     }
 

diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php
@@ -138,17 +138,22 @@ private function extractMostRelevantUrlRewrite(string $requestPath, array $urlRe
     {
         $prioritizedUrlRewrites = [];
         foreach ($urlRewrites as $urlRewrite) {
+            $urlRewriteRequestPath = $urlRewrite[UrlRewrite::REQUEST_PATH];
+            $urlRewriteTargetPath = $urlRewrite[UrlRewrite::TARGET_PATH];
             switch (true) {
-                case $urlRewrite[UrlRewrite::REQUEST_PATH] === $requestPath:
+                case rtrim($urlRewriteRequestPath, '/') === rtrim($urlRewriteTargetPath, '/'):
+                    $priority = 99;
+                    break;
+                case $urlRewriteRequestPath === $requestPath:
                     $priority = 1;
                     break;
-                case $urlRewrite[UrlRewrite::REQUEST_PATH] === urldecode($requestPath):
+                case $urlRewriteRequestPath === urldecode($requestPath):
                     $priority = 2;
                     break;
-                case rtrim($urlRewrite[UrlRewrite::REQUEST_PATH], '/') === rtrim($requestPath, '/'):
+                case rtrim($urlRewriteRequestPath, '/') === rtrim($requestPath, '/'):
                     $priority = 3;
                     break;
-                case rtrim($urlRewrite[UrlRewrite::REQUEST_PATH], '/') === rtrim(urldecode($requestPath), '/'):
+                case rtrim($urlRewriteRequestPath, '/') === rtrim(urldecode($requestPath), '/'):
                     $priority = 4;
                     break;
                 default: