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

Improve middleware for router groups #3546

Open
Murderlon opened this issue Mar 21, 2023 · 7 comments
Open

Improve middleware for router groups #3546

Murderlon opened this issue Mar 21, 2023 · 7 comments

Comments

@Murderlon
Copy link

Murderlon commented Mar 21, 2023

Problem

  • When using router groups, .Use does not do anything as middleware needs to be defined before routes. That is fine for global middleware, but doesn't work for groups which is not intention revealing and blocks essential patterns, such as different CORS middleware per group.
  • gin.RouterGroup does not expose the native http.Handler in order to pass it to third-party middleware libraries.
  • Related: grouping routes and middleware not working #531

Consider this example:

group := router.Group("/group")
{
  group.Use(CORS()) // does not work. Chaining it to `Group` above also does not.
  group.POST("/", someHandler())
}

It gets worse when you'd like to use third-party middleware which expects a http.Handler. It would be great if this works:

group := router.Group("/group")
{
  group.Use(gin.wrapH(thirdparty.Middleware(group.Handler()))) // note: group.Handler() doesn't exist
  group.POST("/", someHandler())
}

Instead I have to extract the code from the library into my own project, and if I only want it to apply to a group, we get something like this:

group := router.Group("/group")
{
  group.OPTIONS("/*group", Middleware())
  group.POST("/", Middleware(), someHandler())
  group.HEAD("/:id", Middleware(), someHandler())
  group.PATCH("/:id", Middleware(), someHandler())
  group.DELETE("/:id", Middleware(), someHandler())
  group.GET("/:id", Middleware(), someHandler())
}

Solution

  • Expose http.Handler in gin.RouterGroup
  • Allow .Use to be used inside groups

I'm willing to contribute these changes if we agree on this and I get some tips on how to approach it.

@ShikharY10
Copy link

Can you please tell me what below line is doing. Means, what s.Router.Group is doing.

group := s.Router.Group("/group")

@Murderlon
Copy link
Author

Murderlon commented Apr 4, 2023

s was something from my codebase, but it's just this: https://gin-gonic.com/docs/examples/grouping-routes/

Edited the example above to be more clear.

@MysticalMount
Copy link

Suffering from this too, amazing we havnt encountered this up till now and the feature isnt already implemented

@pscheid92
Copy link

Hmm, I am confused. Please provide a reproducible example I could run locally. Maybe I misunderstand your expectations.

When I use .Use() on a group, it works as expected. (It runs the middleware code for all routes in this group but not for other routes in other groups.)

PTAL on my example here: https://go.dev/play/p/YX5Iy5sjuXC

@Ozoniuss
Copy link

Ozoniuss commented May 6, 2023

See my last paragraph for what I think could maybe be the problem.

I'm also confused. A similar example also works for me. Take this code for example:

func someMiddleware() gin.HandlerFunc {
	return func(c *gin.Context) {
		fmt.Println("some middleware called")
	}
}

func main() {
	r := gin.New()

	r1 := r.Group("/v1").Use(someMiddleware())
	{
		r1.GET("/hello", func(ctx *gin.Context) {
			fmt.Println("hello")
		})
	}

	r2 := r.Group("/v2")
	{
		r2.Use(someMiddleware())
		r2.GET("/hello", func(ctx *gin.Context) {
			fmt.Println("hello")
		})
	}
	r.Run(":6677")
}

I get the following output (on the server, client doesn't print anything):

$ curl localhost:6677/v1/hello
# server prints:
some middleware called
hello

$ curl localhost:6677/v2/hello
# server prints
some middleware called
hello

Changing the method to POST doesn't make a difference. In fact, when you call Use it even binds the middleware to all handlers of the group:

	r3 := r.Group("/v3")
	{
		// middleware is called
		r3.Use(someMiddleware()).GET("/hello", func(ctx *gin.Context) {
			fmt.Println("hello")
		})
		// middleware is also called
		r3.GET("/hello2", func(ctx *gin.Context) {
			fmt.Println("hello2")
		})
	}

What does not work is the following:

	// middleware is not called.
	r2 := r.Group("/v2")
	{
		r2.GET("/hello", func(ctx *gin.Context) {
			fmt.Println("hello")
		}).Use(someMiddleware())
	}

but you can get around this with the following approach, if you want it to be specific to an endpoint in the group (although I'm not sure if it's a good practice, will have to check):

	// middleware is called.
	r2 := r.Group("/v2")
	{
		r2.GET("/hello", someMiddleware(), func(ctx *gin.Context) {
			fmt.Println("hello")
		})
	}

With this approach you can even have separate "middleware" for every route:

	r2 := r.Group("/v2")
	{
		// someMiddleware is called.
		r2.GET("/hello", someMiddleware(), func(ctx *gin.Context) {
			fmt.Println("hello")
		})
		// someMiddleware2 is called.
		r2.GET("/hello2", someMiddleware2(), func(ctx *gin.Context) {
			fmt.Println("hello2")
		})
	}

How do you know that your examples are not working? Note that your middlewares might be doing some internal logic without you knowing about it. In my examples, the prints are on the server, not in the client responses. If your middlewares don't do anything to the gin Context, it's likely that they're getting called without you noticing the effect.

@AshinZ
Copy link

AshinZ commented Jul 31, 2023

The reason of CORS middleware not working in group router is that when a path in a group router, gin will try to do route match before executing middleware. It's not middleware not working but CORS middleware not working
@Murderlon In your first example, the CORS request send by browser will be aborted if you don't define router for OPTIONS method, the CORS middleware won't be called because the OPTION has been aborted. In this case, we will get 404 error because the OPTIONS request is not defined. So after you add group.OPTIONS("/*group", Middleware()), gin can accept OPTIONS request, and then the CORS middleware work.
So how to use CORS middleware in group router? I don't have a better method now. If we allow every OPTION request in global middleware, the CORS mechanism will be meaningless. If we don't do this and try to do this by middleware in group router, it runs into a dead lock.

@Murderlon
Copy link
Author

I think that's it yes! Thanks for walking us through it. I'm not sure what's best to do here. Perhaps some clarification in the docs could also close this.

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

No branches or pull requests

6 participants