blackteay/gin
3
0
Fork 0
mirror of https://github.com/gin-gonic/gin.git synced 2026-09-04 22:53:34 +08:00
Code Issues Projects Releases Wiki Activity

fix(tree): roll back to skipped wildcards at param route dead ends

Browse Source 
getValue records skipped wildcard siblings whenever it prefers a static
branch, but two dead ends inside the param case returned "not found"
without consulting that stack:

- more path remains but the param node has no child to descend into
- the param node consumed the whole path but has no handler registered

Example: with routes /:p0/:p1 and /bc/:p1/a registered, a request for
/bc/bc returned 404 instead of matching /:p0/:p1.

Both dead ends now roll back to the last valid skippedNode, consistent
with the existing fallback at the other dead ends. The rollback loop,
previously duplicated inline at three call sites, is extracted into
rollbackToSkippedNode and reused at all five.
This commit is contained in:
Gates Wang 2026-08-02 19:10:04 -04:00
parent 34dac209ff
commit 1e39af4f58
2 changed files with 79 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
tree.go
View File

@ -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
}
}

33
tree_test.go
View File

@ -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)
})
}
}