Skip to content

Commit 472d086

Browse files
veeceeyappleboy
andauthored
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>
1 parent fb25834 commit 472d086

File tree

2 files changed

+159
-1
lines changed

2 files changed

+159
-1
lines changed

tree.go

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,72 @@ walk: // Outer loop for walking the tree
818818
return nil
819819
}
820820

821-
n = n.children[0]
821+
// When wildChild is true, try static children first (via indices)
822+
// before falling back to the wildcard child. This ensures that
823+
// case-insensitive lookups prefer static routes over param routes
824+
// (e.g., /PREFIX/XXX should resolve to /prefix/xxx, not match :id).
825+
if len(n.indices) > 0 {
826+
rb = shiftNRuneBytes(rb, npLen)
827+
828+
if rb[0] != 0 {
829+
idxc := rb[0]
830+
for i, c := range []byte(n.indices) {
831+
if c == idxc {
832+
if out := n.children[i].findCaseInsensitivePathRec(
833+
path, ciPath, rb, fixTrailingSlash,
834+
); out != nil {
835+
return out
836+
}
837+
break
838+
}
839+
}
840+
} else {
841+
var rv rune
842+
var off int
843+
for max_ := min(npLen, 3); off < max_; off++ {
844+
if i := npLen - off; utf8.RuneStart(oldPath[i]) {
845+
rv, _ = utf8.DecodeRuneInString(oldPath[i:])
846+
break
847+
}
848+
}
849+
850+
lo := unicode.ToLower(rv)
851+
utf8.EncodeRune(rb[:], lo)
852+
rb = shiftNRuneBytes(rb, off)
853+
854+
idxc := rb[0]
855+
for i, c := range []byte(n.indices) {
856+
if c == idxc {
857+
if out := n.children[i].findCaseInsensitivePathRec(
858+
path, ciPath, rb, fixTrailingSlash,
859+
); out != nil {
860+
return out
861+
}
862+
break
863+
}
864+
}
865+
866+
if up := unicode.ToUpper(rv); up != lo {
867+
utf8.EncodeRune(rb[:], up)
868+
rb = shiftNRuneBytes(rb, off)
869+
870+
idxc := rb[0]
871+
for i, c := range []byte(n.indices) {
872+
if c == idxc {
873+
if out := n.children[i].findCaseInsensitivePathRec(
874+
path, ciPath, rb, fixTrailingSlash,
875+
); out != nil {
876+
return out
877+
}
878+
break
879+
}
880+
}
881+
}
882+
}
883+
}
884+
885+
// Fall back to wildcard child, which is always at the end of the array
886+
n = n.children[len(n.children)-1]
822887
switch n.nType {
823888
case param:
824889
// Find param end (either '/' or path end)

tree_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,3 +1018,96 @@ func TestWildcardInvalidSlash(t *testing.T) {
10181018
}
10191019
}
10201020
}
1021+
1022+
func TestTreeFindCaseInsensitivePathWithMultipleChildrenAndWildcard(t *testing.T) {
1023+
tree := &node{}
1024+
1025+
// Setup routes that create a node with both static children and a wildcard child.
1026+
// This configuration previously caused a panic ("invalid node type") in
1027+
// findCaseInsensitivePathRec because it accessed children[0] instead of the
1028+
// wildcard child (which is always at the end of the children array).
1029+
// See: https://github.com/gin-gonic/gin/issues/2959
1030+
routes := [...]string{
1031+
"/aa/aa",
1032+
"/:bb/aa",
1033+
}
1034+
1035+
for _, route := range routes {
1036+
recv := catchPanic(func() {
1037+
tree.addRoute(route, fakeHandler(route))
1038+
})
1039+
if recv != nil {
1040+
t.Fatalf("panic inserting route '%s': %v", route, recv)
1041+
}
1042+
}
1043+
1044+
// These lookups previously panicked with "invalid node type" because
1045+
// findCaseInsensitivePathRec picked children[0] (a static node) instead
1046+
// of the wildcard child at the end of the array.
1047+
out, found := tree.findCaseInsensitivePath("/aa", true)
1048+
if found {
1049+
t.Errorf("Expected no match for '/aa', but got: %s", string(out))
1050+
}
1051+
1052+
out, found = tree.findCaseInsensitivePath("/aa/aa/aa/aa", true)
1053+
if found {
1054+
t.Errorf("Expected no match for '/aa/aa/aa/aa', but got: %s", string(out))
1055+
}
1056+
1057+
// Case-insensitive lookup should match the static route /aa/aa
1058+
out, found = tree.findCaseInsensitivePath("/AA/AA", true)
1059+
if !found {
1060+
t.Error("Route '/AA/AA' not found via case-insensitive lookup")
1061+
} else if string(out) != "/aa/aa" {
1062+
t.Errorf("Wrong result for '/AA/AA': expected '/aa/aa', got: %s", string(out))
1063+
}
1064+
}
1065+
1066+
func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) {
1067+
tree := &node{}
1068+
1069+
// Another variant: param route + static route under same prefix
1070+
routes := [...]string{
1071+
"/prefix/:id",
1072+
"/prefix/xxx",
1073+
}
1074+
1075+
for _, route := range routes {
1076+
recv := catchPanic(func() {
1077+
tree.addRoute(route, fakeHandler(route))
1078+
})
1079+
if recv != nil {
1080+
t.Fatalf("panic inserting route '%s': %v", route, recv)
1081+
}
1082+
}
1083+
1084+
// Should NOT panic even for paths that don't match any route
1085+
out, found := tree.findCaseInsensitivePath("/prefix/a/b/c", true)
1086+
if found {
1087+
t.Errorf("Expected no match for '/prefix/a/b/c', but got: %s", string(out))
1088+
}
1089+
1090+
// Exact match should still work
1091+
out, found = tree.findCaseInsensitivePath("/prefix/xxx", true)
1092+
if !found {
1093+
t.Error("Route '/prefix/xxx' not found")
1094+
} else if string(out) != "/prefix/xxx" {
1095+
t.Errorf("Wrong result for '/prefix/xxx': %s", string(out))
1096+
}
1097+
1098+
// Case-insensitive match should work
1099+
out, found = tree.findCaseInsensitivePath("/PREFIX/XXX", true)
1100+
if !found {
1101+
t.Error("Route '/PREFIX/XXX' not found via case-insensitive lookup")
1102+
} else if string(out) != "/prefix/xxx" {
1103+
t.Errorf("Wrong result for '/PREFIX/XXX': expected '/prefix/xxx', got: %s", string(out))
1104+
}
1105+
1106+
// Param route should still match
1107+
out, found = tree.findCaseInsensitivePath("/prefix/something", true)
1108+
if !found {
1109+
t.Error("Route '/prefix/something' not found via param match")
1110+
} else if string(out) != "/prefix/something" {
1111+
t.Errorf("Wrong result for '/prefix/something': %s", string(out))
1112+
}
1113+
}

0 commit comments

Comments
 (0)