AC-904: PHP Error instead of 404/getting redirected when accessing any directory in <root>/catalog/product/compare/

Author: yvoronoy

lib/internal/Magento/Framework/App/Router/ActionList.php

@@ -12,6 +12,7 @@
 use Magento\Framework\App\State;
 use Magento\Framework\App\Utility\ReflectionClassFactory;
 use Magento\Framework\Config\CacheInterface;
+use Magento\Framework\Exception\FileSystemException;
 use Magento\Framework\Module\Dir\Reader as ModuleReader;
 use Magento\Framework\Serialize\Serializer\Serialize;
 use Magento\Framework\Serialize\SerializerInterface;
@@ -61,7 +62,7 @@ class ActionList
     /**
      * @var ReflectionClassFactory|null
      */
-    private $classReflectionFactory;
+    private $reflectionClassFactory;
 
     /**
      * @param CacheInterface $cache
@@ -70,6 +71,10 @@ class ActionList
      * @param string $cacheKey
      * @param array $reservedWords
      * @param SerializerInterface|null $serializer
+     * @param State|null $state
+     * @param DirectoryList|null $directoryList
+     * @param ReflectionClassFactory|null $reflectionClassFactory
+     * @throws FileSystemException
      */
     public function __construct(
         CacheInterface $cache,
@@ -78,16 +83,19 @@ public function __construct(
         $cacheKey = 'app_action_list',
         $reservedWords = [],
         SerializerInterface $serializer = null,
+        State $state = null,
+        DirectoryList $directoryList = null,
         ReflectionClassFactory $reflectionClassFactory = null
     ) {
         $this->reservedWords = array_merge($reservedWords, $this->reservedWords);
         $this->actionInterface = $actionInterface;
-        $objectManager = ObjectManager::getInstance();
-        $this->serializer = $serializer ?: $objectManager->get(Serialize::class);
-        $state = $objectManager->get(State::class);
+        $this->serializer = $serializer ?: ObjectManager::getInstance()->get(Serialize::class);
+        $state = $state ?: ObjectManager::getInstance()->get(State::class);
+        $this->reflectionClassFactory = $reflectionClassFactory
+            ?: ObjectManager::getInstance()->get(ReflectionClassFactory::class);
 
         if ($state->getMode() === State::MODE_PRODUCTION) {
-            $directoryList = $objectManager->get(DirectoryList::class);
+            $directoryList = $directoryList ?: ObjectManager::getInstance()->get(DirectoryList::class);
             $file = $directoryList->getPath(DirectoryList::GENERATED_METADATA)
                 . '/' . $cacheKey . '.' . 'php';
 
@@ -105,8 +113,6 @@ public function __construct(
                 $this->actions = $this->serializer->unserialize($data);
             }
         }
-        $this->classReflectionFactory = $reflectionClassFactory ?:
-            ObjectManager::getInstance()->get(ReflectionClassFactory::class);
     }
 
     /**
@@ -119,7 +125,7 @@ public function __construct(
      * @return null|string
      * @throws ReflectionException
      */
-    public function get($module, $area, $namespace, $action, $reflectionClass = null)
+    public function get($module, $area, $namespace, $action)
     {
         if ($area) {
             $area = '\\' . $area;
@@ -139,7 +145,7 @@ public function get($module, $area, $namespace, $action, $reflectionClass = null
             )
         );
         try {
-            if ($this->validateActionClass($fullPath, $reflectionClass)) {
+            if ($this->validateActionClass($fullPath)) {
                 return $this->actions[$fullPath];
             }
         } catch (ReflectionException $e) {
@@ -153,19 +159,16 @@ public function get($module, $area, $namespace, $action, $reflectionClass = null
      * Validate Action Class
      *
      * @param string $fullPath
-     * @param ReflectionClass|null $reflectionClass
      * @return bool
      * @throws ReflectionException
      */
-    private function validateActionClass(string $fullPath, $reflectionClass): bool
+    private function validateActionClass(string $fullPath): bool
     {
         if (isset($this->actions[$fullPath])) {
             if (!is_subclass_of($this->actions[$fullPath], $this->actionInterface)) {
                 return false;
             }
-            if (!$reflectionClass) {
-                $reflectionClass = $this->classReflectionFactory->create($this->actions[$fullPath]);
-            }
+            $reflectionClass = $this->reflectionClassFactory->create($this->actions[$fullPath]);
             if ($reflectionClass->isInstantiable()) {
                 return true;
             }

lib/internal/Magento/Framework/App/Test/Unit/Router/ActionListTest.php

@@ -9,6 +9,7 @@
 
 use Magento\Framework\App\ActionInterface;
 use Magento\Framework\App\Router\ActionList;
+use Magento\Framework\App\Utility\ReflectionClassFactory;
 use Magento\Framework\Config\CacheInterface;
 use Magento\Framework\Module\Dir\Reader;
 use Magento\Framework\Serialize\SerializerInterface;
@@ -49,15 +50,22 @@ class ActionListTest extends TestCase
      */
     private $reflectionClass;
 
+    /**
+     * @var ReflectionClassFactory|MockObject
+     */
+    private $reflectionClassFactory;
+
     protected function setUp(): void
     {
         $this->objectManager = new ObjectManager($this);
         $this->cacheMock = $this->getMockForAbstractClass(CacheInterface::class);
         $this->readerMock = $this->createMock(Reader::class);
         $this->serializerMock = $this->getMockForAbstractClass(SerializerInterface::class);
-        $this->reflectionClass = $this->getMockBuilder(ReflectionClass::class)
+        $this->reflectionClass = $this->createStub(ReflectionClass::class);
+        $this->reflectionClassFactory = $this->getMockBuilder(ReflectionClassFactory::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->reflectionClassFactory->method('create')->willReturn($this->reflectionClass);
     }
 
     public function testConstructActionsCached()
@@ -101,6 +109,7 @@ public function testConstructActionsNoCached()
     public function testGet($module, $area, $namespace, $action, $data, $isInstantiable, $expected)
     {
         $this->reflectionClass->method('isInstantiable')->willReturn($isInstantiable);
+
         $this->cacheMock->expects($this->once())
             ->method('load')
             ->willReturn(false);
@@ -114,8 +123,7 @@ public function testGet($module, $area, $namespace, $action, $data, $isInstantia
             $module,
             $area,
             $namespace,
-            $action,
-            $this->reflectionClass
+            $action
         ));
     }
 
@@ -198,6 +206,7 @@ private function createActionListInstance()
                 'cache' => $this->cacheMock,
                 'moduleReader' => $this->readerMock,
                 'serializer' => $this->serializerMock,
+                'reflectionClassFactory' => $this->reflectionClassFactory
             ]
         );
     }