gin icon indicating copy to clipboard operation
gin copied to clipboard

Improve middleware for router groups

Open Murderlon opened this issue 1 year ago • 5 comments

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: https://github.com/gin-gonic/gin/issues/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.

Murderlon avatar Mar 21 '23 13:03 Murderlon

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

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

ShikharY10 avatar Apr 03 '23 21:04 ShikharY10

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.

Murderlon avatar Apr 04 '23 07:04 Murderlon

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

MysticalMount avatar Apr 06 '23 09:04 MysticalMount

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

pscheid92 avatar May 04 '23 18:05 pscheid92

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.

Ozoniuss avatar May 06 '23 15:05 Ozoniuss

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.

AshinZ avatar Jul 31 '23 09:07 AshinZ

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.

Murderlon avatar Aug 03 '23 14:08 Murderlon