mirror of
https://github.com/gin-gonic/gin.git
synced 2026-07-06 18:31:24 +08:00
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
This commit is contained in:
parent
8c24788d83
commit
d86f2eb722
66
tree.go
66
tree.go
@ -818,7 +818,71 @@ walk: // Outer loop for walking the tree
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Handle wildcard child, which is always at the end of the array
|
// 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]
|
n = n.children[len(n.children)-1]
|
||||||
switch n.nType {
|
switch n.nType {
|
||||||
case param:
|
case param:
|
||||||
|
|||||||
42
tree_test.go
42
tree_test.go
@ -1044,26 +1044,22 @@ func TestTreeFindCaseInsensitivePathWithMultipleChildrenAndWildcard(t *testing.T
|
|||||||
// These lookups previously panicked with "invalid node type" because
|
// These lookups previously panicked with "invalid node type" because
|
||||||
// findCaseInsensitivePathRec picked children[0] (a static node) instead
|
// findCaseInsensitivePathRec picked children[0] (a static node) instead
|
||||||
// of the wildcard child at the end of the array.
|
// of the wildcard child at the end of the array.
|
||||||
recv := catchPanic(func() {
|
out, found := tree.findCaseInsensitivePath("/aa", true)
|
||||||
tree.findCaseInsensitivePath("/aa", true)
|
if found {
|
||||||
})
|
t.Errorf("Expected no match for '/aa', but got: %s", string(out))
|
||||||
if recv != nil {
|
|
||||||
t.Fatalf("unexpected panic looking up '/aa': %v", recv)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
recv = catchPanic(func() {
|
out, found = tree.findCaseInsensitivePath("/aa/aa/aa/aa", true)
|
||||||
tree.findCaseInsensitivePath("/aa/aa/aa/aa", true)
|
if found {
|
||||||
})
|
t.Errorf("Expected no match for '/aa/aa/aa/aa', but got: %s", string(out))
|
||||||
if recv != nil {
|
|
||||||
t.Fatalf("unexpected panic looking up '/aa/aa/aa/aa': %v", recv)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Also test case-insensitive lookup (this was crashing too)
|
// Case-insensitive lookup should match the static route /aa/aa
|
||||||
recv = catchPanic(func() {
|
out, found = tree.findCaseInsensitivePath("/AA/AA", true)
|
||||||
tree.findCaseInsensitivePath("/AA/AA", true)
|
if !found {
|
||||||
})
|
t.Error("Route '/AA/AA' not found via case-insensitive lookup")
|
||||||
if recv != nil {
|
} else if string(out) != "/aa/aa" {
|
||||||
t.Fatalf("unexpected panic looking up '/AA/AA': %v", recv)
|
t.Errorf("Wrong result for '/AA/AA': expected '/aa/aa', got: %s", string(out))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1086,15 +1082,13 @@ func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Should NOT panic even for paths that don't match any route
|
// Should NOT panic even for paths that don't match any route
|
||||||
recv := catchPanic(func() {
|
out, found := tree.findCaseInsensitivePath("/prefix/a/b/c", true)
|
||||||
tree.findCaseInsensitivePath("/prefix/a/b/c", true)
|
if found {
|
||||||
})
|
t.Errorf("Expected no match for '/prefix/a/b/c', but got: %s", string(out))
|
||||||
if recv != nil {
|
|
||||||
t.Fatalf("unexpected panic looking up '/prefix/a/b/c': %v", recv)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Exact match should still work
|
// Exact match should still work
|
||||||
out, found := tree.findCaseInsensitivePath("/prefix/xxx", true)
|
out, found = tree.findCaseInsensitivePath("/prefix/xxx", true)
|
||||||
if !found {
|
if !found {
|
||||||
t.Error("Route '/prefix/xxx' not found")
|
t.Error("Route '/prefix/xxx' not found")
|
||||||
} else if string(out) != "/prefix/xxx" {
|
} else if string(out) != "/prefix/xxx" {
|
||||||
@ -1105,8 +1099,8 @@ func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) {
|
|||||||
out, found = tree.findCaseInsensitivePath("/PREFIX/XXX", true)
|
out, found = tree.findCaseInsensitivePath("/PREFIX/XXX", true)
|
||||||
if !found {
|
if !found {
|
||||||
t.Error("Route '/PREFIX/XXX' not found via case-insensitive lookup")
|
t.Error("Route '/PREFIX/XXX' not found via case-insensitive lookup")
|
||||||
} else if string(out) != "/prefix/XXX" {
|
} else if string(out) != "/prefix/xxx" {
|
||||||
t.Errorf("Wrong result for '/PREFIX/XXX': %s", string(out))
|
t.Errorf("Wrong result for '/PREFIX/XXX': expected '/prefix/xxx', got: %s", string(out))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Param route should still match
|
// Param route should still match
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user