Passing middleware configuration as list of strings

[boesing]

Feature Request

Q	A
New Feature	yes
RFC	yes
BC Break	yes

Summary

I am migrating from laminas-mvc middleware handling to this package and I've realized a difference I want to discuss.

Mezzio middleware definition and Laminas MVC middleware definition both support a list of multiple Middlewares (a so-called "pipe") in form of strings matching the FQCN of the middleware. This package does not. Instead, one has to implement some (imho) hard to understand configuration:

[
    'middleware' => new PipeSpec(
        ContentEncodingDecoderMiddleware::class,
        FromBackofficeController::class,
    ),
]

If one is familiar with Mezzio or the old configuration, it is hard to understand why this component requires to instantiate that pipe specification.

In order to prepare a project to be forward compatible to Mezzio so one can step-by-step migrate to Mezzio by converting existing routes to the middleware definition, I see a problem with this instantiation.

In our company, we are using MVC but want to migrate to Mezzio when we finished converting all controllers (which are around 500+ and thus wont be done in a single step). If we now have to specify new Pipespec for all middleware keys, we have to remove this again when migrated to mezzio.

Summary

Is there any reason why we should not support a list of strings as array definition? If not, I am fine with creating a PR to support that along with the new PipeSpec definition. This would also provide backwards compatibility to old laminas-mvc middlewares (atleast these users only have to change the interfaces of their implementation rather than converting a whole bunch of configuration files).

[froschdesign]

In our company, we are using MVC but want to migrate to Mezzio when we finished converting all controllers (which are around 500+ and thus wont be done in a single step). If we now have to specify new Pipespec for all middleware keys, we have to remove this again when migrated to mezzio.

I'm in the same situation.

There is a focus on interoperability in Mezzio and laminas-mvc to standards, but at the same time it is missing between Mezzio and laminas-mvc or the handling is different. I would therefore welcome a harmonization.

[rieschl]

When @Xerkus came up with the new PipeSpec in his PR, I had the same concerns as you and we had a lengthy discussion in Slack. The main reason for using the PipeSpec is to ensure that no config merging is happening in the middleware key and to stress that this middleware approach in mvc is no real middleware pipe as in Mezzio but only specific to one route ("controller action"). So it cannot really be standardized and a migration from mvc to Mezzio means reconfiguring the middlware pipe anyway (at least in our setup). Maybe @Xerkus has more insights on this.

As for being forward compatible, you're right that this is a bit cumbersome change (also having to add PipeSpec::class in the controller key), but essentially a search/replace.

If it's any help: in our project this was more or less a non-issue because we already abstracted away this verbose array-based route setup:

    /**
     * @param string|list<string> $middleware
     * @return array<string, mixed>
     */
    public static function segment(string $route, $middleware): array
    {
        return self::middlewareRoute(Segment::class, $route, $middleware);
    }

    /**
     * @param string|list<string> $middleware
     * @return array<string, mixed>
     */
    public static function middlewareRoute(string $type, string $route, $middleware): array
    {
        $middleware = is_string($middleware) ? [$middleware] : $middleware;
        return [
            'type' => $type,
            'options' => [
                'route' => $route,
                'defaults' => [
                    'controller' => PipeSpec::class,
                    'middleware' => new PipeSpec(...$middleware),
                ],
            ],
        ];
    }

in the module.config.php there's just a single line:

'pay' => RouteHelper::literal('/payment/pay', InitiatePaymentHandler::class),

we have some more helper functions for backend middleware routes and admin middlware, which prepend Login middleware and stuff in front of the $middleware params to keep the config files clean.

You could rewrite your routes now using the old syntax and then would just have to use the new call to PipeSpec at a single place.

[Xerkus]

object instead of array removes a dangerous uncertainty of config merging. middleware declared as arrays will be merged resulting in something unexpected that is virtually impossible to notice from just a code review. In Mezzio, preferred way to configure routes does not involve config merging, so it is fine. controller must be set to PipeSpec to avoid the problem where child route is not aware of middleware parameter and, because middleware dispatch have higher priority, results in inherited middleware parameter hijacking the dispatch.

[boesing]

So if we want to avoid dangerous config merging, why wont we add an RFC to mezzio router aswell?

Imho, synchronizing this in some way would help alot as I really dont get why we should make migrations such a PITA.

So having some kind of "PipeSpec" in the mezzio-router would at least help.

---

Another way could be a dedicated Option in the config which explicitly enables the array config "at own risk" and thus only gets merged when manually added.

I personally have to say, that as an advanced user, this kind of protection bothers me. I would say, there should be at least a way to have this achieved.

---

sure, I could add some kind of Route builder, as the structure might change in mezzio anyways due to switch from zend-router to fastroute. I'll give this a shot.

[froschdesign]

As a user, I want to map a route to a middleware, not to a controller. But I have to add PipeSpec as controller but, is not a controller – it's just confusing.

I think the technical problem is clear, but the question is if we are pushing an internal problem to the user?

[rieschl]

So if we want to avoid dangerous config merging, why wont we add an RFC to mezzio router aswell?

There is no merging of Routed Middlewar config, is there? So it's not an issue there.

Imho, synchronizing this in some way would help alot as I really dont get why we should make migrations such a PITA. So having some kind of "PipeSpec" in the mezzio-router would at least help.

How would you do such a migration? IMO when migrating from mvc to Mezzio there is no direct copying the route config from mvc to Mezzio. Besides different syntax, the whole pipe is different. Because there is no global middleware pipe in mvc, I add every middleware in all route PipeSpecs. In mezzio at least some middleware is in the global pipe. Eg. we have a middleware which determines the current locale of the user (based on headers, cookies, ...) and a middleware for backend login check which are added in every route. I wouldn't do that in Mezzio to put all middleware in the route config. So, when migrating a route to Mezzio you'll have to touch the spec anyways.

Honestly, I don't get why that PipeSpec bothers you so much. It's just a tiny implementation detail and I don't care if I write new PipeSpec(A::class, B::class) or [A::class, B::class].

[rieschl]

As a user, I want to map a route to a middleware, not to a controller. But I have to add PipeSpec as controller but, is not a controller – it's just confusing.

That's different issue. The explicit PipeSpec::class class-string is there to make middleware routes work in every stage of the route stack, no matter if it's in a parent route or some child route. So in addition to pass the middleware you want to dispatch in the middleware key, you have to put PipeSpec::class in the controller key. Otherwise it would not be possible to have a middleware route as parent route and a "classic" controller action as a child.

[froschdesign]

@rieschl I already wrote:

…technical problem is clear…

[rieschl]

I already wrote:

…technical problem is clear…

Yes I read that, but do to config merging there's no other way around, is there? Despite migrating controllers to middleware from bottom up.

[boesing]

@rieschl There is a config merge process.

https://github.com/mezzio/mezzio/blob/1925171d5647821853dabbf2c5e5975b68ae9cc6/src/Container/ApplicationConfigInjectionDelegator.php#L62

And regarding the route configuration: I've written a library for that in the past: https://github.com/boesing/zend-router-to-expressive-router This could help converting existing route configurations.

And sure, the global pipeline needs to be configured when migrating to mezzio. This is not needed yet, as we have global DispatchListeners for that.

But it helps to migrate step-by-step without the need to run mezzio in parallel. And thats what we want to achieve.

[Xerkus]

Initial reason to introduce PipeSpec was security focused: a route that would allow arbitrary 'middleware' parameter to be injected could lead to execution of any middleware or combination. Setting controller to PipeSpec::class allowed to bring back plain strings as value, since checking just for the presence of middleware is dangerous in case route author did not even consider possibility of dispatching middleware.

As for the plain array, it was not brought back because of how config merging works: arrays with numeric keys will append and result in unpredictable pipelines. Route definitions are getting overridden with shared modules.

I can suggest a listener on dispatch event, before middleware dispatch, that would wrap array in middleware parameter with PipeSpec.

[boesing]

Plenty of security additions recently made to this library. Glad to hear that. I dont think that this feature will land here and thus, closing it. Thanks anyways.