[BUGFIX] Replace middlewares in stack on duplicate occurence 40/58340/4
authorSusanne Moog <susanne.moog@typo3.org>
Wed, 19 Sep 2018 14:39:12 +0000 (16:39 +0200)
committerAndreas Fernandez <a.fernandez@scripting-base.de>
Thu, 20 Sep 2018 05:46:54 +0000 (07:46 +0200)
Use array_replace_recursive instead of array_merge_recursive to
avoid an exception if a middleware is defined twice.

Additionally move the replacement out of the loop.

Resolves: #86319
Releases: master
Change-Id: Ie59fe1d54b9498475c630fd0043b0b5d193b608c
Reviewed-on: https://review.typo3.org/58340
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Richard Haeser <richard@maxserv.com>
Tested-by: Richard Haeser <richard@maxserv.com>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
typo3/sysext/core/Classes/Http/MiddlewareStackResolver.php
typo3/sysext/core/Tests/Unit/Http/Fixtures/Package2Replaces1/Configuration/RequestMiddlewares.php [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Http/MiddlewareStackResolverTest.php

index b28229b..930f9d4 100644 (file)
@@ -92,18 +92,17 @@ class MiddlewareStackResolver
     protected function loadConfiguration(): array
     {
         $packages = $this->packageManager->getActivePackages();
-        $allMiddlewares = [];
+        $allMiddlewares = [[]];
         foreach ($packages as $package) {
             $packageConfiguration = $package->getPackagePath() . 'Configuration/RequestMiddlewares.php';
             if (file_exists($packageConfiguration)) {
                 $middlewaresInPackage = require $packageConfiguration;
                 if (is_array($middlewaresInPackage)) {
-                    $allMiddlewares = array_merge_recursive($allMiddlewares, $middlewaresInPackage);
+                    $allMiddlewares[] = $middlewaresInPackage;
                 }
             }
         }
-
-        return $allMiddlewares;
+        return array_replace_recursive(...$allMiddlewares);
     }
 
     /**
diff --git a/typo3/sysext/core/Tests/Unit/Http/Fixtures/Package2Replaces1/Configuration/RequestMiddlewares.php b/typo3/sysext/core/Tests/Unit/Http/Fixtures/Package2Replaces1/Configuration/RequestMiddlewares.php
new file mode 100644 (file)
index 0000000..c015284
--- /dev/null
@@ -0,0 +1,11 @@
+<?php
+return [
+    'testStack' => [
+        'firstMiddleware' => [
+            'target' => 'replacedClassName',
+        ],
+        'secondMiddleware' => [
+            'target' => 'anotherClassName',
+        ],
+    ]
+];
index 5074711..2b19639 100644 (file)
@@ -60,6 +60,29 @@ class MiddlewareStackResolverTest extends UnitTestCase
     /**
      * @test
      */
+    public function resolveReturnsEmptyMiddlewareStackForZeroPackages()
+    {
+        $packageManagerProphecy = $this->prophesize(PackageManager::class);
+        $packageManagerProphecy->getActivePackages()->willReturn([]);
+        $dependencyOrderingServiceProphecy = $this->prophesize(DependencyOrderingService::class);
+        $dependencyOrderingServiceProphecy->orderByDependencies(Argument::cetera())->willReturnArgument(0);
+        $phpFrontendCacheProphecy = $this->prophesize(PhpFrontend::class);
+        $phpFrontendCacheProphecy->has(Argument::cetera())->willReturn(false);
+        $phpFrontendCacheProphecy->set(Argument::cetera())->willReturn(false);
+
+        $subject = new MiddlewareStackResolver(
+            $packageManagerProphecy->reveal(),
+            $dependencyOrderingServiceProphecy->reveal(),
+            $phpFrontendCacheProphecy->reveal()
+        );
+        // empty array expected
+        $expected = [];
+        $this->assertEquals($expected, $subject->resolve('testStack'));
+    }
+
+    /**
+     * @test
+     */
     public function resolveAllowsDisablingAMiddleware()
     {
         $package1 = $this->prophesize(Package::class);
@@ -85,4 +108,34 @@ class MiddlewareStackResolverTest extends UnitTestCase
         ];
         $this->assertEquals($expected, $subject->resolve('testStack'));
     }
+
+    /**
+     * @test
+     */
+    public function resolveAllowsReplacingAMiddleware()
+    {
+        $package1 = $this->prophesize(Package::class);
+        $package1->getPackagePath()->willReturn(__DIR__ . '/' . 'Fixtures/Package1/');
+        $package2 = $this->prophesize(Package::class);
+        $package2->getPackagePath()->willReturn(__DIR__ . '/' . 'Fixtures/Package2Replaces1/');
+        $packageManagerProphecy = $this->prophesize(PackageManager::class);
+        $packageManagerProphecy->getActivePackages()->willReturn([$package1->reveal(), $package2->reveal()]);
+        $dependencyOrderingServiceProphecy = $this->prophesize(DependencyOrderingService::class);
+        $dependencyOrderingServiceProphecy->orderByDependencies(Argument::cetera())->willReturnArgument(0);
+        $phpFrontendCacheProphecy = $this->prophesize(PhpFrontend::class);
+        $phpFrontendCacheProphecy->has(Argument::cetera())->willReturn(false);
+        $phpFrontendCacheProphecy->set(Argument::cetera())->willReturn(false);
+
+        $subject = new MiddlewareStackResolver(
+            $packageManagerProphecy->reveal(),
+            $dependencyOrderingServiceProphecy->reveal(),
+            $phpFrontendCacheProphecy->reveal()
+        );
+        $expected = [
+            // firstMiddleware has been replaced, RequestMiddlewares.php of $package2 sets a new value for firstMiddleware
+            'firstMiddleware' => 'replacedClassName',
+            'secondMiddleware' => 'anotherClassName',
+        ];
+        $this->assertEquals($expected, $subject->resolve('testStack'));
+    }
 }