Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Ensure middleware group exists #42161

Closed
wants to merge 1 commit into from
Closed

[9.x] Ensure middleware group exists #42161

wants to merge 1 commit into from

Conversation

rodrigopedra
Copy link
Contributor

Closes #42159

After PR #42004 the behavior of method Illuminate\Routing\Router@middlewareGroup was slightly changed, as before it would define a middleware group even if the group was empty, e.g. the group had an empty array of middleware.

After the aforementioned PR, a middleware group with an empty array does not actually add a corresponding middleware group to the stack, thus when running a route with that empty middleware group, the dispatcher would try to instantiate a middleware named as the group which will fail, as reported in #42159 .

This PR:

  • Adds a new protected method named ensureMiddlewareGroup, which will check if a middleware group is defined, and if not will add it.
  • Modify the methods: middlewareGroup, prependMiddlewareToGroup and pushMiddlewareToGroup to use the method above

To reproduce the error described in issue #42159 one could use the following steps:

  1. Create a new laravel app
  2. Ensure it uses Laravel version 9.10.0
  3. Add an empty middleware group to the app's HTTP kernel (for example, named public)
  4. Define a route that uses the middleware defined as above
  5. Access the defined route through the browser

After applying the changes proposed in this PR, the route will work.

@rodrigopedra rodrigopedra changed the title [9.x ]Ensure middleware group exists [9.x] Ensure middleware group exists Apr 28, 2022
@toyi
Copy link

toyi commented Apr 28, 2022

I'm afraid it doesn't solves #42159 because a middleware group is still not entirely replaced by using middlewareGroup, which was the case before #42004:

public function middlewareGroup($name, array $middleware)
    {
        $this->middlewareGroups[$name] = $middleware;

        return $this;
    }

Notice that $middleware is an array even if it's singular, that typo was fixed in #42004
Allowed a package to litterally replace, let's say, the api group, by another set of middlewares.

@taylorotwell
Copy link
Member

Reverting the original PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change in middlewareGroup function behaviour introduces a breaking change in v9.10.0
3 participants