diff --git a/packages/docs-markdown/docs/packages/layout.md b/packages/docs-markdown/docs/packages/layout.md index 89c8b9ed9..88813bb56 100644 --- a/packages/docs-markdown/docs/packages/layout.md +++ b/packages/docs-markdown/docs/packages/layout.md @@ -140,9 +140,25 @@ Handles control which pages a component appears on: ### LayoutMiddleware -Register `LayoutMiddleware` in your application middleware stack. When a controller has a `#[Layout]` attribute, it delegates rendering to `LayoutProcessor` automatically. +`marko/layout` registers `LayoutMiddleware` as global middleware, so there is nothing to wire up. When a controller has a `#[Layout]` attribute, it delegates rendering to `LayoutProcessor` automatically. -`LayoutMiddleware` is registered as global middleware, so it wraps every route-level middleware. It runs the route middleware and controller action first, and only replaces a **successful** result (a 2xx response without a `Location` header) with the rendered layout. Any other response passes through untouched — an auth redirect to `/login`, a 401/403 from `#[Can]`, a 419 CSRF mismatch, a 429 rate limit, or a 404 thrown by the controller reaches the client instead of the layout, so layout components never render for a denied request. +`LayoutMiddleware` is marked [`#[RunsInnermost]`](/docs/packages/routing/#innermost-global-middleware): unlike other global middleware, it runs **after** every route middleware, directly around the controller action. That has two consequences: + +- **Denied requests never render a layout.** Route middleware such as authentication, `#[Can]` or a rate limiter answers before `LayoutMiddleware` runs. And `LayoutMiddleware` only replaces a **successful** controller result (a 2xx response without a `Location` header) with the rendered layout; any other result --- a 404 thrown by the controller, a redirect it returns --- passes through untouched. +- **Headers and cookies survive.** Route middleware wraps the rendered layout response, so a `Set-Cookie`, a `Cache-Control` or any custom header it adds on the way out reaches the client. Headers and cookies the controller put on its own response are copied onto the layout response too. Where both set the same header or cookie, the layout's wins, and `Content-Type` and `Content-Length` always describe the rendered layout. + +```php title="app/web/src/Http/Middleware/NoStoreMiddleware.php" +class NoStoreMiddleware implements MiddlewareInterface +{ + public function handle(Request $request, callable $next): Response + { + // $next() returns the rendered layout for a #[Layout] route. + return $next($request)->withHeader('Cache-Control', 'no-store'); + } +} +``` + +Global middleware (sessions, CSRF, security headers, page cache) still wraps everything, including the layout. ## Customization diff --git a/packages/docs-markdown/docs/packages/routing.md b/packages/docs-markdown/docs/packages/routing.md index 324a64620..d5813d3d8 100644 --- a/packages/docs-markdown/docs/packages/routing.md +++ b/packages/docs-markdown/docs/packages/routing.md @@ -367,10 +367,27 @@ class WebhookController With [`marko/security`](/docs/packages/security/) installed, `CsrfMiddleware` is global as well, and a webhook receiver like this one excludes it alongside the session: `#[WithoutMiddleware([SessionMiddleware::class, CsrfMiddleware::class])]`. -The route's stack is global middleware, then route middleware, minus the excluded classes. Unmatched requests (404/405) have no route to exclude anything; they run only the global middleware marked `#[RunsOnUnmatched]` (see [Which middleware runs](#which-middleware-runs)). +The route's stack is global middleware, then route middleware, then the global middleware marked `#[RunsInnermost]` (see [Innermost global middleware](#innermost-global-middleware)), minus the excluded classes. Unmatched requests (404/405) have no route to exclude anything; they run only the global middleware marked `#[RunsOnUnmatched]` (see [Which middleware runs](#which-middleware-runs)). If a route excludes middleware that is neither global nor on the route, boot fails with a `RouteException`. That catches a typo in the class name, and it catches a driver package that isn't installed. A silent no-op would leave the middleware running. The [stateless API recipe](/docs/packages/session/#stateless-routes) shows the session case in full. +### Innermost Global Middleware + +Global middleware normally wraps every route middleware. A global middleware that turns the controller's result into the final response (layout rendering in [`marko/layout`](/docs/packages/layout/), for example) needs the opposite: it should run directly around the controller, so route middleware sees, and can decorate, the final response. Mark it `#[RunsInnermost]`: + +```php title="app/web/src/Http/Middleware/WrapInFrameMiddleware.php" +use Marko\Routing\Attributes\RunsInnermost; +use Marko\Routing\Middleware\MiddlewareInterface; + +#[RunsInnermost] +class WrapInFrameMiddleware implements MiddlewareInterface +{ + // ... +} +``` + +For a matched route the stack becomes: global middleware, route middleware, then the `#[RunsInnermost]` global middleware in their declaration order, then the controller. A route middleware that denies the request (an auth redirect, a 403) answers before the innermost middleware runs, and headers or cookies a route middleware adds to the response returned by `$next()` are kept. `#[WithoutMiddleware]` excludes innermost middleware like any other. As with `#[RunsOnUnmatched]`, the attribute is read from the class listed under `globalMiddleware`, not from a `#[Preference]` that replaces it. + ### Decorating Responses Middleware that adds headers, cookies, or changes the status code should decorate the response returned by `$next()` rather than construct a new one: @@ -753,6 +770,7 @@ marko route:list --method=GET --path=blog #[WithoutMiddleware(MiddlewareClass::class)] // class or method; class-string or array #[RoutePrefix(prefix: '/api', namePrefix: 'api.')] // class only #[RunsOnUnmatched] // middleware class only; runs on 404/405/automatic OPTIONS +#[RunsInnermost] // global middleware class only; runs after route middleware ``` ### UrlGeneratorInterface diff --git a/packages/layout/src/Middleware/LayoutMiddleware.php b/packages/layout/src/Middleware/LayoutMiddleware.php index 2d90604dd..d03bfdad1 100644 --- a/packages/layout/src/Middleware/LayoutMiddleware.php +++ b/packages/layout/src/Middleware/LayoutMiddleware.php @@ -7,11 +7,21 @@ use Marko\Layout\Exceptions\LayoutNotFoundException; use Marko\Layout\LayoutProcessorInterface; use Marko\Layout\LayoutResolver; +use Marko\Routing\Attributes\RunsInnermost; use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; use Marko\Routing\Middleware\MiddlewareInterface; use Marko\Routing\RouteMatcherInterface; +/** + * Renders the layout of a matched route whose controller declares #[Layout]. + * + * Marked #[RunsInnermost], so it runs after every route middleware, directly + * around the controller: route middleware that denies the request answers + * before any layout renders, and route middleware that adds headers or + * cookies on the way out decorates the rendered layout response. + */ +#[RunsInnermost] readonly class LayoutMiddleware implements MiddlewareInterface { public function __construct( @@ -39,9 +49,9 @@ public function handle( return $next($request); } - // Run the rest of the pipeline (route middleware + controller) first. - // Inner denials — auth redirects, 401/403/419/429, 404s thrown by the - // controller — arrive here as ordinary responses because the pipeline + // Run the controller (and anything inside this middleware) first. + // Denials that reach here — 404s thrown by the controller, a redirect + // it returns — arrive as ordinary responses because the pipeline // renders HTTP exceptions at the depth they are thrown. They must reach // the client untouched; only a successful controller result is replaced // by the layout, whose components provide their own data. @@ -51,13 +61,50 @@ public function handle( return $response; } - return $this->layoutProcessor->process( + $layout = $this->layoutProcessor->process( controllerClass: $controllerClass, action: $action, routePath: $matched->route->path, routeParameters: $matched->parameters, request: $request, ); + + return $this->carryOver($response, $layout); + } + + /** + * Keep the headers and cookies the controller put on its result. The + * layout's own headers and cookies win on a conflict, and Content-Type + * and Content-Length always describe the rendered layout body. + */ + private function carryOver( + Response $result, + Response $layout, + ): Response { + $layoutHeaders = array_map(strtolower(...), array_keys($layout->headers())); + $headers = []; + + foreach ($result->headers() as $name => $value) { + $lower = strtolower($name); + + if (in_array($lower, ['content-type', 'content-length'], true) || in_array($lower, $layoutHeaders, true)) { + continue; + } + + $headers[$name] = $value; + } + + $merged = $layout->withHeaders($headers); + + foreach ($result->cookies() as $cookie) { + $merged = $merged->withCookie($cookie); + } + + foreach ($layout->cookies() as $cookie) { + $merged = $merged->withCookie($cookie); + } + + return $merged; } private function isSuccessfulResult( diff --git a/packages/layout/tests/Unit/Middleware/LayoutMiddlewareRouterTest.php b/packages/layout/tests/Unit/Middleware/LayoutMiddlewareRouterTest.php index fc003b0ee..7bd0d2d88 100644 --- a/packages/layout/tests/Unit/Middleware/LayoutMiddlewareRouterTest.php +++ b/packages/layout/tests/Unit/Middleware/LayoutMiddlewareRouterTest.php @@ -12,6 +12,7 @@ use Marko\Layout\LayoutProcessorInterface; use Marko\Layout\Middleware\LayoutMiddleware; use Marko\Routing\Exceptions\HttpException; +use Marko\Routing\Http\Cookie; use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; use Marko\Routing\Middleware\MiddlewareInterface; @@ -35,6 +36,29 @@ public function missing(): Response { throw HttpException::notFound('Order not found.'); } + + public function decorated(): Response + { + return new Response('controller body', 200, ['X-Controller' => 'set', 'Content-Type' => 'text/plain']) + ->withCookie(new Cookie('controller_cookie', 'from-controller')); + } +} + +/** + * Route middleware that decorates the response on the way out, as a session, + * remember-me or cache-control middleware would. + */ +class DecoratesResponseMiddleware implements MiddlewareInterface +{ + public function handle( + Request $request, + callable $next, + ): Response { + return $next($request) + ->withHeader('X-Route-Middleware', 'kept') + ->withHeader('Cache-Control', 'no-store') + ->withCookie(new Cookie('route_cookie', 'from-middleware')); + } } /** @@ -80,7 +104,8 @@ public function process( ): Response { $this->calls++; - return Response::html('secret account data'); + return Response::html('secret account data') + ->withCookie(new Cookie('controller_cookie', 'from-layout')); } } @@ -123,6 +148,18 @@ function dashboardRequest( return new Request(server: ['REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/dashboard', ...$server]); } +/** + * @return list + */ +function cookiePairs( + Response $response, +): array { + return array_map( + fn (Cookie $cookie): string => $cookie->name() . '=' . $cookie->value(), + $response->cookies(), + ); +} + describe('LayoutMiddleware as global middleware through Router::handle()', function (): void { it('returns the auth redirect to an anonymous request instead of rendering the layout', function (): void { $processor = new RecordingLayoutProcessor(); @@ -168,4 +205,32 @@ function dashboardRequest( ->and($response->body())->toContain('Order not found.') ->and($processor->calls)->toBe(0); }); + + it('keeps a cookie and a custom header set by route middleware on a layout-rendered 2xx', function (): void { + $processor = new RecordingLayoutProcessor(); + $router = bootLayoutRouter('index', [DecoratesResponseMiddleware::class], $processor); + + $response = $router->handle(dashboardRequest()); + $cookies = cookiePairs($response); + + expect($response->statusCode())->toBe(200) + ->and($response->body())->toBe('secret account data') + ->and($response->headers()['X-Route-Middleware'])->toBe('kept') + ->and($response->headers()['Cache-Control'])->toBe('no-store') + ->and($cookies)->toContain('route_cookie=from-middleware') + ->and($processor->calls)->toBe(1); + }); + + it('keeps headers and cookies the controller set, letting the layout win on conflicts', function (): void { + $processor = new RecordingLayoutProcessor(); + $router = bootLayoutRouter('decorated', [], $processor); + + $response = $router->handle(dashboardRequest()); + $cookies = cookiePairs($response); + + expect($response->body())->toBe('secret account data') + ->and($response->headers()['X-Controller'])->toBe('set') + ->and($response->headers()['Content-Type'])->toBe('text/html; charset=utf-8') + ->and($cookies)->toBe(['controller_cookie=from-layout']); + }); }); diff --git a/packages/layout/tests/Unit/Middleware/LayoutMiddlewareTest.php b/packages/layout/tests/Unit/Middleware/LayoutMiddlewareTest.php index 09d698be7..f9b0cfb21 100644 --- a/packages/layout/tests/Unit/Middleware/LayoutMiddlewareTest.php +++ b/packages/layout/tests/Unit/Middleware/LayoutMiddlewareTest.php @@ -7,6 +7,7 @@ use Marko\Layout\LayoutProcessorInterface; use Marko\Layout\LayoutResolver; use Marko\Layout\Middleware\LayoutMiddleware; +use Marko\Routing\Attributes\RunsInnermost; use Marko\Routing\Http\Request; use Marko\Routing\Http\Response; use Marko\Routing\MatchedRoute; @@ -129,6 +130,12 @@ function makeRequest(string $path = '/test'): Request } describe('LayoutMiddleware', function (): void { + it('is marked #[RunsInnermost] so route middleware wraps the rendered layout', function (): void { + $attributes = new ReflectionClass(LayoutMiddleware::class)->getAttributes(RunsInnermost::class); + + expect($attributes)->toHaveCount(1); + }); + it('delegates to LayoutProcessor when controller has Layout attribute', function (): void { $route = makeRoute(LmFixtureController::class, 'index'); $matched = new MatchedRoute($route, []); diff --git a/packages/routing/src/Attributes/RunsInnermost.php b/packages/routing/src/Attributes/RunsInnermost.php new file mode 100644 index 000000000..027038164 --- /dev/null +++ b/packages/routing/src/Attributes/RunsInnermost.php @@ -0,0 +1,26 @@ + */ private function middlewareFor( RouteDefinition $route, ): array { - $middleware = [...$this->globalMiddleware, ...$route->middleware]; + $outer = []; + $innermost = []; + + foreach ($this->globalMiddleware as $global) { + if ($this->hasClassAttribute($global, RunsInnermost::class)) { + $innermost[] = $global; + } else { + $outer[] = $global; + } + } + + $middleware = [...$outer, ...$route->middleware, ...$innermost]; if ($route->withoutMiddleware === []) { return $middleware; @@ -132,11 +145,24 @@ private function unmatchedMiddleware(): array { return array_values(array_filter( $this->globalMiddleware, - fn (string $middleware): bool => class_exists($middleware) - && new ReflectionClass($middleware)->getAttributes(RunsOnUnmatched::class) !== [], + fn (string $middleware): bool => $this->hasClassAttribute($middleware, RunsOnUnmatched::class), )); } + /** + * Whether a loadable class carries the attribute. A class that cannot be + * loaded carries none; the container fails loudly when it resolves it. + * + * @param class-string $attribute + */ + private function hasClassAttribute( + string $class, + string $attribute, + ): bool { + return class_exists($class) + && new ReflectionClass($class)->getAttributes($attribute) !== []; + } + /** * Terminal handler for a request no route matched. * diff --git a/packages/routing/tests/RunsInnermostTest.php b/packages/routing/tests/RunsInnermostTest.php new file mode 100644 index 000000000..f0e03d33d --- /dev/null +++ b/packages/routing/tests/RunsInnermostTest.php @@ -0,0 +1,193 @@ + */ + public array $entries = []; +} + +/** + * @noinspection PhpUnused - Action is invoked by the router + */ +class InnermostTestController +{ + public function __construct( + private readonly InnermostOrderLog $log, + ) {} + + public function index(): Response + { + $this->log->entries[] = 'controller'; + + return new Response('controller body'); + } +} + +class InnermostOuterGlobalMiddleware implements MiddlewareInterface +{ + public function __construct( + private readonly InnermostOrderLog $log, + ) {} + + public function handle( + Request $request, + callable $next, + ): Response { + $this->log->entries[] = 'outer-global'; + + return $next($request); + } +} + +#[RunsInnermost] +class InnermostWrappingMiddleware implements MiddlewareInterface +{ + public function __construct( + private readonly InnermostOrderLog $log, + ) {} + + public function handle( + Request $request, + callable $next, + ): Response { + $this->log->entries[] = 'innermost'; + $next($request); + + return new Response('replaced by innermost'); + } +} + +#[RunsInnermost] +class InnermostSecondWrappingMiddleware implements MiddlewareInterface +{ + public function __construct( + private readonly InnermostOrderLog $log, + ) {} + + public function handle( + Request $request, + callable $next, + ): Response { + $this->log->entries[] = 'innermost-second'; + + return $next($request); + } +} + +class InnermostRouteMiddleware implements MiddlewareInterface +{ + public function __construct( + private readonly InnermostOrderLog $log, + ) {} + + public function handle( + Request $request, + callable $next, + ): Response { + $this->log->entries[] = 'route'; + + return $next($request)->withHeader('X-Route', 'ran'); + } +} + +/** + * @param array> $globalMiddleware + * @param array> $routeMiddleware + * @param array> $withoutMiddleware + * @return array{router: Router, log: InnermostOrderLog} + */ +function createInnermostRouter( + array $globalMiddleware, + array $routeMiddleware = [], + array $withoutMiddleware = [], +): array { + $container = new Container(new PreferenceRegistry()); + $log = new InnermostOrderLog(); + $container->instance(InnermostOrderLog::class, $log); + + $collection = new RouteCollection(); + $collection->add(new RouteDefinition( + method: 'GET', + path: '/page', + controller: InnermostTestController::class, + action: 'index', + middleware: $routeMiddleware, + withoutMiddleware: $withoutMiddleware, + )); + + return [ + 'router' => new Router(new RouteMatcher($collection), $container, $globalMiddleware), + 'log' => $log, + ]; +} + +function innermostRequest(): Request +{ + return new Request(server: ['REQUEST_METHOD' => 'GET', 'REQUEST_URI' => '/page']); +} + +describe('#[RunsInnermost]', function (): void { + it('targets classes only', function (): void { + $flags = new ReflectionClass(RunsInnermost::class)->getAttributes(Attribute::class)[0] + ->newInstance()->flags; + + expect($flags)->toBe(Attribute::TARGET_CLASS); + }); + + it('runs a marked global middleware after route middleware, directly around the controller', function (): void { + ['router' => $router, 'log' => $log] = createInnermostRouter( + [InnermostOuterGlobalMiddleware::class, InnermostWrappingMiddleware::class], + [InnermostRouteMiddleware::class], + ); + + $response = $router->handle(innermostRequest()); + + expect($log->entries)->toBe(['outer-global', 'route', 'innermost', 'controller']) + ->and($response->body())->toBe('replaced by innermost') + ->and($response->headers()['X-Route'])->toBe('ran'); + }); + + it('keeps the declaration order among marked global middleware', function (): void { + ['router' => $router, 'log' => $log] = createInnermostRouter( + [ + InnermostSecondWrappingMiddleware::class, + InnermostOuterGlobalMiddleware::class, + InnermostWrappingMiddleware::class, + ], + [InnermostRouteMiddleware::class], + ); + + $router->handle(innermostRequest()); + + expect($log->entries)->toBe(['outer-global', 'route', 'innermost-second', 'innermost', 'controller']); + }); + + it('still lets a route exclude a marked global middleware with #[WithoutMiddleware]', function (): void { + ['router' => $router, 'log' => $log] = createInnermostRouter( + [InnermostWrappingMiddleware::class], + [InnermostRouteMiddleware::class], + [InnermostWrappingMiddleware::class], + ); + + $response = $router->handle(innermostRequest()); + + expect($log->entries)->toBe(['route', 'controller']) + ->and($response->body())->toBe('controller body'); + }); +});