feat(engine): add SkipMethodNotAllowedMiddleware option

Middleware registered via Use() is prepended to the NoMethod handlers by
rebuild405Handlers, so a global middleware that aborts the request answers
before the NoMethod handler can reply. A middleware validating a header that
only some methods carry therefore turns every 405 into its own error
response.

Add an opt-in Engine.SkipMethodNotAllowedMiddleware flag that runs only the
NoMethod handlers for 405 responses. It is disabled by default, so existing
behaviour is unchanged.

The flag is resolved when the request is served rather than when the chain is
built, so it applies regardless of the order in which Use(), NoMethod() and
the flag are set. Requests handled by NoRoute() keep running the global
middleware.

Fixes #4189

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.

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

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()