From 472d086af2acd924cb4b9d7be0525f7d790f69bc Mon Sep 17 00:00:00 2001 From: Varun Chawla <34209028+veeceey@users.noreply.github.com> Date: Fri, 27 Feb 2026 19:15:27 -0800 Subject: [PATCH] fix(tree): panic in findCaseInsensitivePathRec with RedirectFixedPath (#4535) * fix(tree): panic in findCaseInsensitivePathRec with RedirectFixedPath enabled When RedirectFixedPath is enabled and a request doesn't match any route, findCaseInsensitivePathRec panics with "invalid node type". This happens because it accesses children[0] to find the wildcard child, but addChild() keeps the wildcard child at the end of the array (not the beginning). When a node has both static and wildcard children, children[0] is a static node, so the switch on nType falls through to the default panic case. The fix mirrors what getValue() already does correctly (line 483): use children[len(children)-1] to access the wildcard child. Fixes #2959 * Address review feedback: improve test assertions and prefer static routes in case-insensitive lookup - Assert found=false for non-matching paths instead of only checking for panics - Fix expected casing for case-insensitive static route match (/PREFIX/XXX -> /prefix/xxx) - Update findCaseInsensitivePathRec to try static children before falling back to wildcard child, ensuring static routes win over param routes in case-insensitive matching --------- Co-authored-by: Bo-Yi Wu --- tree.go | 67 ++++++++++++++++++++++++++++++++++++- tree_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/tree.go b/tree.go index 3ac0a3b1..580abbaf 100644 --- a/tree.go +++ b/tree.go @@ -818,7 +818,72 @@ walk: // Outer loop for walking the tree return nil } - n = n.children[0] + // When wildChild is true, try static children first (via indices) + // before falling back to the wildcard child. This ensures that + // case-insensitive lookups prefer static routes over param routes + // (e.g., /PREFIX/XXX should resolve to /prefix/xxx, not match :id). + if len(n.indices) > 0 { + rb = shiftNRuneBytes(rb, npLen) + + if rb[0] != 0 { + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + } else { + var rv rune + var off int + for max_ := min(npLen, 3); off < max_; off++ { + if i := npLen - off; utf8.RuneStart(oldPath[i]) { + rv, _ = utf8.DecodeRuneInString(oldPath[i:]) + break + } + } + + lo := unicode.ToLower(rv) + utf8.EncodeRune(rb[:], lo) + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + + if up := unicode.ToUpper(rv); up != lo { + utf8.EncodeRune(rb[:], up) + rb = shiftNRuneBytes(rb, off) + + idxc := rb[0] + for i, c := range []byte(n.indices) { + if c == idxc { + if out := n.children[i].findCaseInsensitivePathRec( + path, ciPath, rb, fixTrailingSlash, + ); out != nil { + return out + } + break + } + } + } + } + } + + // Fall back to wildcard child, which is always at the end of the array + n = n.children[len(n.children)-1] switch n.nType { case param: // Find param end (either '/' or path end) diff --git a/tree_test.go b/tree_test.go index b580007d..23339af4 100644 --- a/tree_test.go +++ b/tree_test.go @@ -1018,3 +1018,96 @@ func TestWildcardInvalidSlash(t *testing.T) { } } } + +func TestTreeFindCaseInsensitivePathWithMultipleChildrenAndWildcard(t *testing.T) { + tree := &node{} + + // Setup routes that create a node with both static children and a wildcard child. + // This configuration previously caused a panic ("invalid node type") in + // findCaseInsensitivePathRec because it accessed children[0] instead of the + // wildcard child (which is always at the end of the children array). + // See: https://github.com/gin-gonic/gin/issues/2959 + routes := [...]string{ + "/aa/aa", + "/:bb/aa", + } + + for _, route := range routes { + recv := catchPanic(func() { + tree.addRoute(route, fakeHandler(route)) + }) + if recv != nil { + t.Fatalf("panic inserting route '%s': %v", route, recv) + } + } + + // These lookups previously panicked with "invalid node type" because + // findCaseInsensitivePathRec picked children[0] (a static node) instead + // of the wildcard child at the end of the array. + out, found := tree.findCaseInsensitivePath("/aa", true) + if found { + t.Errorf("Expected no match for '/aa', but got: %s", string(out)) + } + + out, found = tree.findCaseInsensitivePath("/aa/aa/aa/aa", true) + if found { + t.Errorf("Expected no match for '/aa/aa/aa/aa', but got: %s", string(out)) + } + + // Case-insensitive lookup should match the static route /aa/aa + out, found = tree.findCaseInsensitivePath("/AA/AA", true) + if !found { + t.Error("Route '/AA/AA' not found via case-insensitive lookup") + } else if string(out) != "/aa/aa" { + t.Errorf("Wrong result for '/AA/AA': expected '/aa/aa', got: %s", string(out)) + } +} + +func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) { + tree := &node{} + + // Another variant: param route + static route under same prefix + routes := [...]string{ + "/prefix/:id", + "/prefix/xxx", + } + + for _, route := range routes { + recv := catchPanic(func() { + tree.addRoute(route, fakeHandler(route)) + }) + if recv != nil { + t.Fatalf("panic inserting route '%s': %v", route, recv) + } + } + + // Should NOT panic even for paths that don't match any route + out, found := tree.findCaseInsensitivePath("/prefix/a/b/c", true) + if found { + t.Errorf("Expected no match for '/prefix/a/b/c', but got: %s", string(out)) + } + + // Exact match should still work + out, found = tree.findCaseInsensitivePath("/prefix/xxx", true) + if !found { + t.Error("Route '/prefix/xxx' not found") + } else if string(out) != "/prefix/xxx" { + t.Errorf("Wrong result for '/prefix/xxx': %s", string(out)) + } + + // Case-insensitive match should work + out, found = tree.findCaseInsensitivePath("/PREFIX/XXX", true) + if !found { + t.Error("Route '/PREFIX/XXX' not found via case-insensitive lookup") + } else if string(out) != "/prefix/xxx" { + t.Errorf("Wrong result for '/PREFIX/XXX': expected '/prefix/xxx', got: %s", string(out)) + } + + // Param route should still match + out, found = tree.findCaseInsensitivePath("/prefix/something", true) + if !found { + t.Error("Route '/prefix/something' not found via param match") + } else if string(out) != "/prefix/something" { + t.Errorf("Wrong result for '/prefix/something': %s", string(out)) + } +}