Merge 641abb68dabce0f7cab2a19f9b768fedaafeaf65 into dcaa4296d111981ffb31ac3eba90bb63e1eb5ab9

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