diff --git a/docs/doc.md b/docs/doc.md index d1c33b87..35c4a20c 100644 --- a/docs/doc.md +++ b/docs/doc.md @@ -24,6 +24,7 @@ - [Custom Recovery behavior](#custom-recovery-behavior) - [Using BasicAuth() middleware](#using-basicauth-middleware) - [Goroutines inside a middleware](#goroutines-inside-a-middleware) + - [Skip middleware for 405 responses](#skip-middleware-for-405-responses) - [Logging](#logging) - [How to write log file](#how-to-write-log-file) - [Custom Log Format](#custom-log-format) @@ -605,6 +606,51 @@ func main() { } ``` +### Skip middleware for 405 responses + +Middleware registered with `Use()` runs for every request, including the `405 Method Not +Allowed` responses produced when `HandleMethodNotAllowed` is enabled. A middleware that +rejects the request therefore aborts the chain before the `NoMethod()` handler can reply, +and the client receives the middleware's response instead of a 405. + +Enable `SkipMethodNotAllowedMiddleware` to run only the `NoMethod()` handlers for those +responses: + +```go +func main() { + r := gin.New() + r.HandleMethodNotAllowed = true + + // Without SkipMethodNotAllowedMiddleware this middleware answers "wrong checksum" + // with 400 for a GET request, because a GET carries no X-Checksum header. + r.SkipMethodNotAllowedMiddleware = true + + r.Use(func(c *gin.Context) { + if c.GetHeader("X-Checksum") == "" { + c.String(http.StatusBadRequest, "wrong checksum") + c.Abort() + return + } + c.Next() + }) + + r.NoMethod(func(c *gin.Context) { + c.String(http.StatusMethodNotAllowed, "method not allowed") + }) + + r.POST("/ping", func(c *gin.Context) { + c.String(http.StatusOK, "pong") + }) + + // GET /ping now returns 405 "method not allowed" with an "Allow: POST" header. + r.Run(":8080") +} +``` + +The option is disabled by default, so the existing behaviour is unchanged. It only +applies to 405 responses; requests handled by `NoRoute()` still run the global +middleware. + ## Logging > Control log output, formatting, and filtering. diff --git a/gin.go b/gin.go index 2e033bf3..e772cc15 100644 --- a/gin.go +++ b/gin.go @@ -122,6 +122,14 @@ type Engine struct { // handler. HandleMethodNotAllowed bool + // SkipMethodNotAllowedMiddleware if enabled, global middleware registered via Use() + // is not executed when the router answers with 405 Method Not Allowed. Only the + // handlers registered with NoMethod run. This lets a NoMethod handler reply with + // 405 even when a global middleware (authentication, checksum validation, ...) + // would otherwise abort the request first. + // Requires HandleMethodNotAllowed to be enabled. + SkipMethodNotAllowedMiddleware bool + // ForwardedByClientIP if enabled, client IP will be parsed from the request's headers that // match those stored at `(*gin.Engine).RemoteIPHeaders`. If no IP was // fetched, it falls back to the IP obtained from @@ -337,6 +345,7 @@ func (engine *Engine) NoMethod(handlers ...HandlerFunc) { // Use attaches a global middleware to the router. i.e. the middleware attached through Use() will be // included in the handlers chain for every single request. Even 404, 405, static files... // For example, this is the right place for a logger or error management middleware. +// Set Engine.SkipMethodNotAllowedMiddleware to exclude this middleware from 405 responses. func (engine *Engine) Use(middleware ...HandlerFunc) IRoutes { engine.RouterGroup.Use(middleware...) engine.rebuild404Handlers() @@ -361,6 +370,18 @@ func (engine *Engine) rebuild405Handlers() { engine.allNoMethod = engine.combineHandlers(engine.noMethod) } +// noMethodHandlers returns the handlers chain used to answer 405 Method Not Allowed. +// When SkipMethodNotAllowedMiddleware is enabled the global middleware registered +// via Use() is left out, so only the NoMethod handlers run. +// The choice is made here rather than in rebuild405Handlers so that it holds +// regardless of the order in which Use(), NoMethod() and the flag are set. +func (engine *Engine) noMethodHandlers() HandlersChain { + if engine.SkipMethodNotAllowedMiddleware { + return engine.noMethod + } + return engine.allNoMethod +} + func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { assert1(path[0] == '/', "path must begin with '/'") assert1(method != "", "HTTP method can not be empty") @@ -748,7 +769,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) { } } if len(allowed) > 0 { - c.handlers = engine.allNoMethod + c.handlers = engine.noMethodHandlers() c.writermem.Header().Set("Allow", strings.Join(allowed, ", ")) serveError(c, http.StatusMethodNotAllowed, default405Body) return diff --git a/middleware_test.go b/middleware_test.go index 8dc7c3b3..7e9412ad 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -156,6 +156,134 @@ func TestMiddlewareNoMethodDisabled(t *testing.T) { assert.Equal(t, "AC X DB", signature) } +// Test the fix for https://github.com/gin-gonic/gin/issues/4189 +func TestMiddlewareNoMethodSkipped(t *testing.T) { + signature := "" + router := New() + router.HandleMethodNotAllowed = true + router.SkipMethodNotAllowedMiddleware = true + router.Use(func(c *Context) { + signature += "A" + c.Next() + signature += "B" + }) + router.Use(func(c *Context) { + signature += "C" + c.Next() + signature += "D" + }) + router.NoMethod(func(c *Context) { + signature += "E" + c.Next() + signature += "F" + }, func(c *Context) { + signature += "G" + c.Next() + signature += "H" + }) + router.NoRoute(func(c *Context) { + signature += " X " + }) + router.POST("/", func(c *Context) { + signature += " XX " + }) + + // RUN + w := PerformRequest(router, http.MethodGet, "/") + + // TEST + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, http.MethodPost, w.Header().Get("Allow")) + assert.Equal(t, "EGHF", signature) +} + +// The flag is read when the request is served, so it takes effect even when it +// is set after Use() and NoMethod() have already built the handlers chain. +func TestMiddlewareNoMethodSkippedSetAfterRegistration(t *testing.T) { + signature := "" + router := New() + router.HandleMethodNotAllowed = true + router.Use(func(c *Context) { + signature += "A" + c.Next() + signature += "B" + }) + router.NoMethod(func(c *Context) { + signature += "E" + c.Next() + signature += "F" + }) + router.POST("/", func(c *Context) { + signature += " XX " + }) + router.SkipMethodNotAllowedMiddleware = true + + // RUN + w := PerformRequest(router, http.MethodGet, "/") + + // TEST + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, "EF", signature) +} + +func TestMiddlewareNoMethodSkippedWithoutNoMethodHandlers(t *testing.T) { + signature := "" + router := New() + router.HandleMethodNotAllowed = true + router.SkipMethodNotAllowedMiddleware = true + router.Use(func(c *Context) { + signature += "A" + c.Next() + signature += "B" + }) + router.POST("/", func(c *Context) { + signature += " XX " + }) + + // RUN + w := PerformRequest(router, http.MethodGet, "/") + + // TEST + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) + assert.Equal(t, "405 method not allowed", w.Body.String()) + assert.Empty(t, signature) +} + +// The flag only covers 405 responses, requests falling through to NoRoute must +// keep running the global middleware. +func TestMiddlewareNoMethodSkippedDoesNotAffectNoRoute(t *testing.T) { + signature := "" + router := New() + router.HandleMethodNotAllowed = true + router.SkipMethodNotAllowedMiddleware = true + router.Use(func(c *Context) { + signature += "A" + c.Next() + signature += "B" + }) + router.Use(func(c *Context) { + signature += "C" + c.Next() + signature += "D" + }) + router.NoMethod(func(c *Context) { + signature += " E " + }) + router.NoRoute(func(c *Context) { + signature += " X " + }) + router.POST("/", func(c *Context) { + signature += " XX " + }) + + // RUN + w := PerformRequest(router, http.MethodGet, "/not-registered") + + // TEST + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Equal(t, "AC X DB", signature) +} + func TestMiddlewareAbort(t *testing.T) { signature := "" router := New()