From 3bda7d852b6b3d2602f14ab2f61cdeab9298f7d2 Mon Sep 17 00:00:00 2001 From: Kristian Svalland Date: Tue, 3 Jan 2023 10:37:25 +0100 Subject: [PATCH] tree: Fix bug where loop index is not decremented. Also add test to validate fix. fixes https://github.com/gin-gonic/gin/issues/3459 Signed-off-by: Kristian Svalland --- routes_test.go | 19 +++++++++++++++++++ tree.go | 6 +++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/routes_test.go b/routes_test.go index cd8cf14145..ada8e1e457 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) +} diff --git a/tree.go b/tree.go index 3f34b8ee82..eccb3e6a16 100644 --- a/tree.go +++ b/tree.go @@ -459,7 +459,7 @@ 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; { + for l := len(*skippedNodes); l > 0; l-- { skippedNode := (*skippedNodes)[l-1] *skippedNodes = (*skippedNodes)[:l-1] if strings.HasSuffix(skippedNode.path, path) { @@ -576,7 +576,7 @@ 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; { + for l := len(*skippedNodes); l > 0; l-- { skippedNode := (*skippedNodes)[l-1] *skippedNodes = (*skippedNodes)[:l-1] if strings.HasSuffix(skippedNode.path, path) { @@ -633,7 +633,7 @@ walk: // Outer loop for walking the tree // roll back to last valid skippedNode if !value.tsr && path != "/" { - for l := len(*skippedNodes); l > 0; { + for l := len(*skippedNodes); l > 0; l-- { skippedNode := (*skippedNodes)[l-1] *skippedNodes = (*skippedNodes)[:l-1] if strings.HasSuffix(skippedNode.path, path) {