Merge 2d0455aa3b988d16b105560ff1cd1207a8b92b97 into dcaa4296d111981ffb31ac3eba90bb63e1eb5ab9

gin.go

@@ -743,6 +743,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
 			if tree.method == httpMethod {
 				continue
 			}
+			*c.skippedNodes = (*c.skippedNodes)[:0]
 			if value := tree.root.getValue(rPath, nil, c.skippedNodes, unescape); value.handlers != nil {
 				allowed = append(allowed, tree.method)
 			}

routes_test.go

@@ -527,6 +527,23 @@ func TestRouteNotAllowedEnabled3(t *testing.T) {
 	assert.Contains(t, allowed, http.MethodPost)
 }
 
+func TestRouteNotAllowedDoesNotReuseSkippedNodes(t *testing.T) {
+	router := New()
+	router.HandleMethodNotAllowed = true
+	router.POST("/b", func(c *Context) {})
+	router.POST("/:p0", func(c *Context) {})
+	router.PUT("/a/:p1", func(c *Context) {})
+
+	// The POST probe for the Allow header matches static "/b" while
+	// recording the skipped wildcard ":p0". handleHTTPRequest must reset
+	// c.skippedNodes before probing the PUT tree, or the stale entry
+	// leaks the POST route into the PUT lookup and PUT is wrongly
+	// reported in the Allow header.
+	w := PerformRequest(router, http.MethodGet, "/b")
+	assert.Equal(t, http.StatusMethodNotAllowed, w.Code)
+	assert.Equal(t, http.MethodPost, w.Header().Get("Allow"))
+}
+
 func TestRouteNotAllowedDisabled(t *testing.T) {
 	router := New()
 	router.HandleMethodNotAllowed = false

tree.go

@@ -410,6 +410,36 @@ type skippedNode struct {
 	paramsCount int16
 }
 
+// rollbackToSkippedNode unwinds the skipped-wildcard stack after the walk hit
+// a dead end. It pops entries until it finds one whose saved remaining path
+// ends with the current remaining path, i.e. a branch point on the walk that
+// led here; entries from abandoned branches are discarded. If one is found,
+// the walk state saved there (remaining path, current node, captured params)
+// is restored and true is returned; the caller must then resume the walk
+// loop, which retries from the branch point with its wildcard child.
+func rollbackToSkippedNode(
+	skippedNodes *[]skippedNode,
+	path *string,
+	n **node,
+	params *Params,
+	globalParamsCount *int16,
+) bool {
+	for length := len(*skippedNodes); length > 0; length-- {
+		sn := (*skippedNodes)[length-1]
+		*skippedNodes = (*skippedNodes)[:length-1]
+		if strings.HasSuffix(sn.path, *path) {
+			*path = sn.path
+			*n = sn.node
+			if params != nil {
+				*params = (*params)[:sn.paramsCount]
+			}
+			*globalParamsCount = sn.paramsCount
+			return true
+		}
+	}
+	return false
+}
+
 // Returns the handle registered with the given path (key). The values of
 // wildcards are saved to a map.
 // If no handle can be found, a TSR (trailing slash redirect) recommendation is
@@ -457,18 +487,8 @@ 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 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
-								if value.params != nil {
-									*value.params = (*value.params)[:skippedNode.paramsCount]
-								}
-								globalParamsCount = skippedNode.paramsCount
-								continue walk
-							}
+						if rollbackToSkippedNode(skippedNodes, &path, &n, value.params, &globalParamsCount) {
+							continue walk
 						}
 					}
 
@@ -531,6 +551,11 @@ walk: // Outer loop for walking the tree
 
 						// ... but we can't
 						value.tsr = len(path) == end+1
+						if !value.tsr {
+							if rollbackToSkippedNode(skippedNodes, &path, &n, value.params, &globalParamsCount) {
+								continue walk
+							}
+						}
 						return value
 					}
 
@@ -544,6 +569,11 @@ walk: // Outer loop for walking the tree
 						n = n.children[0]
 						value.tsr = (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/")
 					}
+					if !value.tsr {
+						if rollbackToSkippedNode(skippedNodes, &path, &n, value.params, &globalParamsCount) {
+							continue walk
+						}
+					}
 					return value
 
 				case catchAll:
@@ -588,18 +618,8 @@ 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 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
-						if value.params != nil {
-							*value.params = (*value.params)[:skippedNode.paramsCount]
-						}
-						globalParamsCount = skippedNode.paramsCount
-						continue walk
-					}
+				if rollbackToSkippedNode(skippedNodes, &path, &n, value.params, &globalParamsCount) {
+					continue walk
 				}
 				//	n = latestNode.children[len(latestNode.children)-1]
 			}
@@ -645,18 +665,8 @@ walk: // Outer loop for walking the tree
 
 		// roll back to last valid skippedNode
 		if !value.tsr && path != "/" {
-			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
-					if value.params != nil {
-						*value.params = (*value.params)[:skippedNode.paramsCount]
-					}
-					globalParamsCount = skippedNode.paramsCount
-					continue walk
-				}
+			if rollbackToSkippedNode(skippedNodes, &path, &n, value.params, &globalParamsCount) {
+				continue walk
 			}
 		}
 

tree_test.go

@@ -1111,3 +1111,36 @@ func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) {
 		t.Errorf("Wrong result for '/prefix/something': %s", string(out))
 	}
 }
+
+func TestTreeParamFallbackAfterStaticDeadEnd(t *testing.T) {
+	tests := []struct {
+		name    string
+		routes  []string
+		request testRequests
+	}{
+		{
+			name:   "param has child but no handler",
+			routes: []string{"/:p0/:p1", "/bc/:p1/a"},
+			request: testRequests{
+				{"/bc/bc", false, "/:p0/:p1", Params{{Key: "p0", Value: "bc"}, {Key: "p1", Value: "bc"}}},
+			},
+		},
+		{
+			name:   "param has no child for remaining path",
+			routes: []string{"/:p0/:p1/:p2", "/bc/:p1"},
+			request: testRequests{
+				{"/bc/bc/bc", false, "/:p0/:p1/:p2", Params{{Key: "p0", Value: "bc"}, {Key: "p1", Value: "bc"}, {Key: "p2", Value: "bc"}}},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tree := &node{}
+			for _, route := range tt.routes {
+				tree.addRoute(route, fakeHandler(route))
+			}
+			checkRequests(t, tree, tt.request)
+		})
+	}
+}