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 <appleboy.tw@gmail.com>
This commit is contained in:
Varun Chawla 2026-02-27 19:15:27 -08:00 committed by GitHub
parent fb2583442c
commit 472d086af2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 159 additions and 1 deletions

67
tree.go
View File

@ -818,7 +818,72 @@ walk: // Outer loop for walking the tree
return nil 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 { switch n.nType {
case param: case param:
// Find param end (either '/' or path end) // Find param end (either '/' or path end)

View File

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