[6.x] Refactor action request routing

src/Http/Middleware/HandleActionRequest.php

@@ -5,23 +5,30 @@
 namespace CraftCms\Cms\Http\Middleware;
 
 use Closure;
+use CraftCms\Cms\Http\Routing\ActionRoute;
+use CraftCms\Cms\Http\Routing\ActionRouteResolver;
 use Illuminate\Http\Request;
 
 readonly class HandleActionRequest
 {
+    public function __construct(
+        private ActionRouteResolver $actionRoutes,
+    ) {}
+
     public function handle(Request $request, Closure $next): mixed
     {
-        if (! $request->isActionRequest()) {
+        $actionRoute = $this->actionRoutes->resolve($request);
+
+        if ($actionRoute === null) {
             return $next($request);
         }
 
-        $route = $request->actionSegmentsToRoute();
-
-        if ($request->path() === $route) {
+        if ($actionRoute->matches($request)) {
             return $next($request);
         }
 
-        $newRequest = $request->duplicateWithUri($route);
+        $newRequest = $request->duplicateWithUri($actionRoute->uri);
+        $newRequest->attributes->set(ActionRoute::class, $actionRoute);
 
         app()->instance('request', $newRequest);
 

src/Http/Mixins/RequestMixin.php

@@ -7,6 +7,8 @@
 use Closure;
 use CraftCms\Cms\Cms;
 use CraftCms\Cms\Http\Middleware\HandleTokenRequest;
+use CraftCms\Cms\Http\Routing\ActionRoute;
+use CraftCms\Cms\Http\Routing\ActionRouteResolver;
 use Illuminate\Contracts\Encryption\DecryptException;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Context;
@@ -167,7 +169,7 @@ public function isActionRequest(): Closure
              */
             $request = $this;
 
-            return $request->actionSegments() !== [];
+            return app(ActionRouteResolver::class)->resolve($request) !== null;
         };
     }
 
@@ -241,45 +243,22 @@ public function actionSegments(): Closure
              * @phpstan-ignore-next-line
              */
             $request = $this;
-            $actionTrigger = Cms::config()->actionTrigger;
-            $segmentIndex = $request->isCpRequest() ? 2 : 1;
 
-            if ($request->segment($segmentIndex) === $actionTrigger && count($request->segments()) > $segmentIndex) {
-                return array_slice($request->segments(), $segmentIndex);
-            }
-
-            $actionParam = $request->input('action');
-
-            if ($actionParam !== null) {
-                if (! is_string($actionParam)) {
-                    abort(400, 'Invalid action param');
-                }
-
-                return array_values(array_filter(explode('/', $actionParam)));
-            }
-
-            return [];
+            return app(ActionRouteResolver::class)->resolve($request)->segments ?? [];
         };
     }
 
     public function actionSegmentsToRoute(): Closure
     {
-        return function (?array $actionSegments = null): string {
+        return function (): string {
             /**
              * @var Request $request
              *
              * @phpstan-ignore-next-line
              */
             $request = $this;
 
-            $actionSegments ??= $request->actionSegments();
-
-            return implode('/', array_filter([
-                '',
-                $request->isCpRequest() ? Cms::config()->cpTrigger : null,
-                Cms::config()->actionTrigger,
-                ...$actionSegments,
-            ], fn ($value) => $value !== null));
+            return app(ActionRouteResolver::class)->resolve($request)->uri ?? '';
         };
     }
 
@@ -304,6 +283,8 @@ public function duplicateWithUri(): Closure
                 $duplicatedRequest->setLaravelSession($request->session());
             }
 
+            $duplicatedRequest->attributes->remove(ActionRoute::class);
+
             return $duplicatedRequest;
         };
     }

src/Http/Routing/ActionRoute.php

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CraftCms\Cms\Http\Routing;
+
+use CraftCms\Cms\Cms;
+use Illuminate\Http\Request;
+
+readonly class ActionRoute
+{
+    public function __construct(
+        public array $segments,
+        public string $uri,
+        public bool $isCp,
+    ) {}
+
+    public static function fromSegments(array $segments, bool $isCp): ?self
+    {
+        $segments = array_values($segments);
+
+        if ($segments === []) {
+            return null;
+        }
+
+        return new self(
+            segments: $segments,
+            uri: self::uriForSegments($segments, $isCp),
+            isCp: $isCp,
+        );
+    }
+
+    public static function uriForSegments(array $segments, bool $isCp): string
+    {
+        return implode('/', array_filter([
+            '',
+            $isCp ? Cms::config()->cpTrigger : null,
+            Cms::config()->actionTrigger,
+            ...$segments,
+        ], fn ($value) => $value !== null));
+    }
+
+    public function matches(Request $request): bool
+    {
+        return '/'.ltrim($request->path(), '/') === $this->uri;
+    }
+}

src/Http/Routing/ActionRouteResolver.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CraftCms\Cms\Http\Routing;
+
+use CraftCms\Cms\Cms;
+use Illuminate\Http\Request;
+
+readonly class ActionRouteResolver
+{
+    public function resolve(Request $request): ?ActionRoute
+    {
+        $cached = $request->attributes->get(ActionRoute::class);
+
+        if ($cached instanceof ActionRoute) {
+            return $cached;
+        }
+
+        $segments = $this->segmentsFromPath($request) ?? $this->segmentsFromActionParam($request);
+
+        if ($segments === null) {
+            return null;
+        }
+
+        $actionRoute = ActionRoute::fromSegments($segments, $request->isCpRequest());
+
+        if ($actionRoute !== null) {
+            $request->attributes->set(ActionRoute::class, $actionRoute);
+        }
+
+        return $actionRoute;
+    }
+
+    private function segmentsFromPath(Request $request): ?array
+    {
+        $actionTrigger = Cms::config()->actionTrigger;
+        $segmentIndex = $request->isCpRequest() ? 2 : 1;
+
+        if ($request->segment($segmentIndex) === $actionTrigger && count($request->segments()) > $segmentIndex) {
+            return array_slice($request->segments(), $segmentIndex);
+        }
+
+        return null;
+    }
+
+    private function segmentsFromActionParam(Request $request): ?array
+    {
+        $actionParam = $request->input('action');
+
+        if ($actionParam === null) {
+            return null;
+        }
+
+        if (! is_string($actionParam)) {
+            abort(400, 'Invalid action param');
+        }
+
+        $segments = array_values(array_filter(explode('/', $actionParam)));
+
+        return $segments === [] ? null : $segments;
+    }
+}

src/Route/DynamicRoute.php

@@ -5,6 +5,7 @@
 namespace CraftCms\Cms\Route;
 
 use CraftCms\Cms\Cms;
+use CraftCms\Cms\Http\Routing\ActionRoute;
 use CraftCms\Cms\Support\Arr;
 use CraftCms\Cms\Support\Str;
 use CraftCms\Cms\Twig\Exceptions\TemplateLoaderException;
@@ -36,7 +37,7 @@ public function handle(Request $request): Response
         }
 
         return app()->make(Kernel::class)->handle($request->duplicateWithUri(
-            newUri: $request->actionSegmentsToRoute(explode('/', trim($this->route, '/'))),
+            newUri: ActionRoute::uriForSegments(explode('/', trim($this->route, '/')), $request->isCpRequest()),
             query: $variables,
         ));
     }

tests/Feature/Http/Middleware/CheckForUpdatesTest.php

@@ -123,8 +123,6 @@
 
     $middleware = app(CheckForUpdates::class);
     $request = Request::create('/actions/updater/migrate');
-    $request->attributes->set('isActionRequest', true);
-    $request->attributes->set('actionSegments', ['updater', 'migrate']);
 
     $result = $middleware->handle($request, fn () => 'passed');
 
@@ -136,8 +134,6 @@
 
     $middleware = app(CheckForUpdates::class);
     $request = Request::create('/actions/app/health-check');
-    $request->attributes->set('isActionRequest', true);
-    $request->attributes->set('actionSegments', ['app', 'health-check']);
 
     $result = $middleware->handle($request, fn () => 'passed');
 
@@ -149,8 +145,6 @@
 
     $middleware = app(CheckForUpdates::class);
     $request = Request::create('/actions/app/migrate');
-    $request->attributes->set('isActionRequest', true);
-    $request->attributes->set('actionSegments', ['app', 'migrate']);
 
     $result = $middleware->handle($request, fn () => 'passed');
 
@@ -162,8 +156,6 @@
 
     $middleware = app(CheckForUpdates::class);
     $request = Request::create('/actions/pluginstore/install/migrate');
-    $request->attributes->set('isActionRequest', true);
-    $request->attributes->set('actionSegments', ['pluginstore', 'install', 'migrate']);
 
     $result = $middleware->handle($request, fn () => 'passed');
 
@@ -189,8 +181,6 @@
 
     $middleware = app(CheckForUpdates::class);
     $request = Request::create('/actions/entries/save');
-    $request->attributes->set('isActionRequest', true);
-    $request->attributes->set('actionSegments', ['entries', 'save']);
 
     $middleware->handle($request, fn () => 'passed');
 })->throws(HttpException::class);

tests/Unit/Http/Middleware/HandleActionRequestTest.php

@@ -4,6 +4,7 @@
 
 use CraftCms\Cms\Cms;
 use CraftCms\Cms\Http\Middleware\HandleActionRequest;
+use CraftCms\Cms\Http\Routing\ActionRoute;
 use Illuminate\Http\Request;
 
 beforeEach(function () {
@@ -23,7 +24,21 @@
     );
 
     expect($handledRequest->path())->toBe('admin/actions/query/execute')
-        ->and(request())->toBe($handledRequest);
+        ->and(request())->toBe($handledRequest)
+        ->and($handledRequest->attributes->get(ActionRoute::class))->toBeInstanceOf(ActionRoute::class);
+});
+
+it('does not rebind action requests that already use the normalized action uri', function () {
+    $request = Request::create('/admin/actions/query/execute', 'POST');
+    app()->instance('request', $request);
+
+    $handledRequest = app(HandleActionRequest::class)->handle(
+        $request,
+        fn (Request $request) => $request,
+    );
+
+    expect($handledRequest)->toBe($request)
+        ->and(request())->toBe($request);
 });
 
 it('does not rebind non-action requests', function () {

tests/Unit/Http/RequestMixinTest.php

@@ -4,6 +4,7 @@
 
 use CraftCms\Cms\Cms;
 use CraftCms\Cms\Http\Middleware\HandleTokenRequest;
+use CraftCms\Cms\Http\Routing\ActionRoute;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Context;
 use Illuminate\Support\Facades\Crypt;
@@ -145,9 +146,9 @@
             ->toBe('/admin/actions/users/login');
     });
 
-    it('builds a route from explicit action segments', function () {
-        expect(Request::create('/news')->actionSegmentsToRoute(['users', 'login']))
-            ->toBe('/actions/users/login');
+    it('returns an empty string when the current request is not an action request', function () {
+        expect(Request::create('/news')->actionSegmentsToRoute())
+            ->toBe('');
     });
 });
 
@@ -201,6 +202,15 @@
         expect($duplicate->hasSession())->toBeTrue()
             ->and($duplicate->session())->toBe($request->session());
     });
+
+    it('does not preserve resolved action routes on the duplicated request', function () {
+        $request = Request::create('/actions/users/login');
+        $request->attributes->set(ActionRoute::class, ActionRoute::fromSegments(['users', 'login'], false));
+
+        $duplicate = $request->duplicateWithUri('/entries');
+
+        expect($duplicate->attributes->has(ActionRoute::class))->toBeFalse();
+    });
 });
 
 describe('getSigned', function () {