Repository: gin-gonic/gin
Pull request #3460 (2023-01-03 09:44:56)

fix(router): tree bug where loop index is not decremented.
Also add test to validate fix.

fixes https://github.com/gin-gonic/gin/issues/3459



codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/3460?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) Report
> Merging [#3460](https://codecov.io/gh/gin-gonic/gin/pull/3460?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (f4dfe65) into [master](https://codecov.io/gh/gin-gonic/gin/commit/79a61b90324586d3c3a5859f8755cae2d1c46f2d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (79a61b9) will **decrease** coverage by `0.09%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #3460      +/-   ##
==========================================
- Coverage   98.63%   98.53%   -0.10%     
==========================================
  Files          42       42              
  Lines        3140     3143       +3     
==========================================
  Hits         3097     3097              
- Misses         29       31       +2     
- Partials       14       15       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
|  | `98.53% <100.00%> (-0.10%)` | :arrow_down: |
| go-1.17 | `?` | |
| go-1.18 | `98.53% <100.00%> (+<0.01%)` | :arrow_up: |
| go-1.19 | `?` | |
| macos-latest | `98.53% <100.00%> (-0.10%)` | :arrow_down: |
| ubuntu-latest | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/gin-gonic/gin/pull/3460?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | Coverage Δ | |
|---|---|---|
| [tree.go](https://codecov.io/gh/gin-gonic/gin/pull/3460/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-dHJlZS5nbw==) | `100.00% <100.00%> (ø)` | |
| [context.go](https://codecov.io/gh/gin-gonic/gin/pull/3460/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-Y29udGV4dC5nbw==) | `97.45% <0.00%> (-0.51%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)


Patch:
diff --git a/docs/doc.md b/docs/doc.md
index 008a91d..7cebab5 100644
--- a/docs/doc.md
+++ b/docs/doc.md
@@ -450,22 +450,22 @@ func main() {
 
 ```go
 func main() {
-    // Disable Console Color, you don't need console color when writing the logs to file.
-    gin.DisableConsoleColor()
+  // Disable Console Color, you don't need console color when writing the logs to file.
+  gin.DisableConsoleColor()
 
-    // Logging to a file.
-    f, _ := os.Create("gin.log")
-    gin.DefaultWriter = io.MultiWriter(f)
+  // Logging to a file.
+  f, _ := os.Create("gin.log")
+  gin.DefaultWriter = io.MultiWriter(f)
 
-    // Use the following code if you need to write the logs to file and console at the same time.
-    // gin.DefaultWriter = io.MultiWriter(f, os.Stdout)
+  // Use the following code if you need to write the logs to file and console at the same time.
+  // gin.DefaultWriter = io.MultiWriter(f, os.Stdout)
 
-    router := gin.Default()
-    router.GET("/ping", func(c *gin.Context) {
-        c.String(http.StatusOK, "pong")
-    })
+  router := gin.Default()
+  router.GET("/ping", func(c *gin.Context) {
+      c.String(http.StatusOK, "pong")
+  })
 
-    router.Run(":8080")
+   router.Run(":8080")
 }
 ```
 
@@ -516,18 +516,18 @@ Never colorize logs:
 
 ```go
 func main() {
-    // Disable log's color
-    gin.DisableConsoleColor()
+  // Disable log's color
+  gin.DisableConsoleColor()
 
-    // Creates a gin router with default middleware:
-    // logger and recovery (crash-free) middleware
-    router := gin.Default()
+  // Creates a gin router with default middleware:
+  // logger and recovery (crash-free) middleware
+  router := gin.Default()
 
-    router.GET("/ping", func(c *gin.Context) {
-        c.String(http.StatusOK, "pong")
-    })
+  router.GET("/ping", func(c *gin.Context) {
+      c.String(http.StatusOK, "pong")
+  })
 
-    router.Run(":8080")
+  router.Run(":8080")
 }
 ```
 
@@ -535,18 +535,18 @@ Always colorize logs:
 
 ```go
 func main() {
-    // Force log's color
-    gin.ForceConsoleColor()
+  // Force log's color
+  gin.ForceConsoleColor()
 
-    // Creates a gin router with default middleware:
-    // logger and recovery (crash-free) middleware
-    router := gin.Default()
+  // Creates a gin router with default middleware:
+  // logger and recovery (crash-free) middleware
+  router := gin.Default()
 
-    router.GET("/ping", func(c *gin.Context) {
-        c.String(http.StatusOK, "pong")
-    })
+  router.GET("/ping", func(c *gin.Context) {
+      c.String(http.StatusOK, "pong")
+  })
 
-    router.Run(":8080")
+  router.Run(":8080")
 }
 ```
 
@@ -786,11 +786,11 @@ import (
 )
 
 type Person struct {
-        Name       string    `form:"name"`
-        Address    string    `form:"address"`
-        Birthday   time.Time `form:"birthday" time_format:"2006-01-02" time_utc:"1"`
-        CreateTime time.Time `form:"createTime" time_format:"unixNano"`
-        UnixTime   time.Time `form:"unixTime" time_format:"unix"`
+  Name       string    `form:"name"`
+  Address    string    `form:"address"`
+  Birthday   time.Time `form:"birthday" time_format:"2006-01-02" time_utc:"1"`
+  CreateTime time.Time `form:"createTime" time_format:"unixNano"`
+  UnixTime   time.Time `form:"unixTime" time_format:"unix"`
 }
 
 func main() {
@@ -804,13 +804,13 @@ func startPage(c *gin.Context) {
   // If `GET`, only `Form` binding engine (`query`) used.
   // If `POST`, first checks the `content-type` for `JSON` or `XML`, then uses `Form` (`form-data`).
   // See more at https://github.com/gin-gonic/gin/blob/master/binding/binding.go#L88
-        if c.ShouldBind(&person) == nil {
-                log.Println(person.Name)
-                log.Println(person.Address)
-                log.Println(person.Birthday)
-                log.Println(person.CreateTime)
-                log.Println(person.UnixTime)
-        }
+  if c.ShouldBind(&person) == nil {
+    log.Println(person.Name)
+    log.Println(person.Address)
+    log.Println(person.Birthday)
+    log.Println(person.CreateTime)
+    log.Println(person.UnixTime)
+  }
 
   c.String(http.StatusOK, "Success")
 }
@@ -1311,34 +1311,34 @@ main.go
 
 ```go
 import (
-    "fmt"
-    "html/template"
-    "net/http"
-    "time"
+  "fmt"
+  "html/template"
+  "net/http"
+  "time"
 
-    "github.com/gin-gonic/gin"
+  "github.com/gin-gonic/gin"
 )
 
 func formatAsDate(t time.Time) string {
-    year, month, day := t.Date()
-    return fmt.Sprintf("%d/%02d/%02d", year, month, day)
+  year, month, day := t.Date()
+  return fmt.Sprintf("%d/%02d/%02d", year, month, day)
 }
 
 func main() {
-    router := gin.Default()
-    router.Delims("{[{", "}]}")
-    router.SetFuncMap(template.FuncMap{
-        "formatAsDate": formatAsDate,
-    })
-    router.LoadHTMLFiles("./testdata/template/raw.tmpl")
+  router := gin.Default()
+  router.Delims("{[{", "}]}")
+  router.SetFuncMap(template.FuncMap{
+      "formatAsDate": formatAsDate,
+  })
+  router.LoadHTMLFiles("./testdata/template/raw.tmpl")
 
-    router.GET("/raw", func(c *gin.Context) {
-        c.HTML(http.StatusOK, "raw.tmpl", gin.H{
-            "now": time.Date(2017, 07, 01, 0, 0, 0, 0, time.UTC),
-        })
-    })
+  router.GET("/raw", func(c *gin.Context) {
+      c.HTML(http.StatusOK, "raw.tmpl", gin.H{
+          "now": time.Date(2017, 07, 01, 0, 0, 0, 0, time.UTC),
+      })
+  })
 
-    router.Run(":8080")
+  router.Run(":8080")
 }
 
 ```
@@ -2099,28 +2099,27 @@ func main() {
 
 ```go
 import (
-    "fmt"
+  "fmt"
 
-    "github.com/gin-gonic/gin"
+  "github.com/gin-gonic/gin"
 )
 
 func main() {
+  router := gin.Default()
 
-    router := gin.Default()
-
-    router.GET("/cookie", func(c *gin.Context) {
+  router.GET("/cookie", func(c *gin.Context) {
 
-        cookie, err := c.Cookie("gin_cookie")
+      cookie, err := c.Cookie("gin_cookie")
 
-        if err != nil {
-            cookie = "NotSet"
-            c.SetCookie("gin_cookie", "test", 3600, "/", "localhost", false, true)
-        }
+      if err != nil {
+          cookie = "NotSet"
+          c.SetCookie("gin_cookie", "test", 3600, "/", "localhost", false, true)
+      }
 
-        fmt.Printf("Cookie value: %s \n", cookie)
-    })
+      fmt.Printf("Cookie value: %s \n", cookie)
+  })
 
-    router.Run()
+  router.Run()
 }
 ```
 
@@ -2149,7 +2148,6 @@ import (
 )
 
 func main() {
-
   router := gin.Default()
   router.SetTrustedProxies([]string{"192.168.1.2"})
 
@@ -2176,7 +2174,6 @@ import (
 )
 
 func main() {
-
   router := gin.Default()
   // Use predefined header gin.PlatformXXX
   router.TrustedPlatform = gin.PlatformGoogleAppEngine
diff --git a/tree.go b/tree.go
index 3f34b8e..dda8f4f 100644
--- a/tree.go
+++ b/tree.go
@@ -459,9 +459,9 @@ walk: // Outer loop for walking the tree
 					// If the path at the end of the loop is not equal to '/' and the current node has no child nodes
 					// the current node needs to roll back to last valid skippedNode
 					if path != "/" {
-						for l := len(*skippedNodes); l > 0; {
-							skippedNode := (*skippedNodes)[l-1]
-							*skippedNodes = (*skippedNodes)[:l-1]
+						for length := len(*skippedNodes); length > 0; length-- {
+							skippedNode := (*skippedNodes)[length-1]
+							*skippedNodes = (*skippedNodes)[:length-1]
 							if strings.HasSuffix(skippedNode.path, path) {
 								path = skippedNode.path
 								n = skippedNode.node
@@ -576,9 +576,9 @@ walk: // Outer loop for walking the tree
 			// If the current path does not equal '/' and the node does not have a registered handle and the most recently matched node has a child node
 			// the current node needs to roll back to last valid skippedNode
 			if n.handlers == nil && path != "/" {
-				for l := len(*skippedNodes); l > 0; {
-					skippedNode := (*skippedNodes)[l-1]
-					*skippedNodes = (*skippedNodes)[:l-1]
+				for length := len(*skippedNodes); length > 0; length-- {
+					skippedNode := (*skippedNodes)[length-1]
+					*skippedNodes = (*skippedNodes)[:length-1]
 					if strings.HasSuffix(skippedNode.path, path) {
 						path = skippedNode.path
 						n = skippedNode.node
@@ -633,9 +633,9 @@ walk: // Outer loop for walking the tree
 
 		// roll back to last valid skippedNode
 		if !value.tsr && path != "/" {
-			for l := len(*skippedNodes); l > 0; {
-				skippedNode := (*skippedNodes)[l-1]
-				*skippedNodes = (*skippedNodes)[:l-1]
+			for length := len(*skippedNodes); length > 0; length-- {
+				skippedNode := (*skippedNodes)[length-1]
+				*skippedNodes = (*skippedNodes)[:length-1]
 				if strings.HasSuffix(skippedNode.path, path) {
 					path = skippedNode.path
 					n = skippedNode.node


Test patch:
diff --git a/routes_test.go b/routes_test.go
index cd8cf14..ada8e1e 100644
--- a/routes_test.go
+++ b/routes_test.go
@@ -670,3 +670,22 @@ func TestRouteContextHoldsFullPath(t *testing.T) {
 	w := PerformRequest(router, http.MethodGet, "/not-found")
 	assert.Equal(t, http.StatusNotFound, w.Code)
 }
+
+func TestEngineHandleMethodNotAllowedCornerCase(t *testing.T) {
+	r := New()
+	r.HandleMethodNotAllowed = true
+
+	base := r.Group("base")
+	base.GET("/metrics", handlerTest1)
+
+	v1 := base.Group("v1")
+
+	v1.GET("/:id/devices", handlerTest1)
+	v1.GET("/user/:id/groups", handlerTest1)
+
+	v1.GET("/orgs/:id", handlerTest1)
+	v1.DELETE("/orgs/:id", handlerTest1)
+
+	w := PerformRequest(r, "GET", "/base/v1/user/groups")
+	assert.Equal(t, http.StatusNotFound, w.Code)
+}