public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Live: adopt latest fixes from gin route tree (#41141)

Browse Source
This commit is contained in:
Alexander Emelin
2021-11-01 20:02:03 +03:00
committed by GitHub
parent c91bfcc1d6
commit 9c160413f6
3 changed files with 156 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/live/pipeline/rule_cache_segmented.go
+1 -2
View File
@@ -75,8 +75,7 @@ func (s *CacheSegmentedTree) Get(orgID int64, channel string) (*LiveChannelRule,
if !ok {
return nil, false, nil
}
ps := make(tree.Params, 0, 20)
nodeValue := t.GetValue("/"+channel, &ps, true)
nodeValue := t.GetValue("/"+channel, true)
if nodeValue.Handler == nil {
return nil, false, nil
}
pkg/services/live/pipeline/tree/tree.go
+103 -57
View File
@@ -15,6 +15,7 @@ import (
var (
strColon = []byte(":")
strStar = []byte("*")
strSlash = []byte("/")
)
func min(a, b int) int {
@@ -51,6 +52,11 @@ func countParams(path string) uint16 {
return n
}
func countSections(path string) uint16 {
s := StringToBytes(path)
return uint16(bytes.Count(s, strSlash))
}
type nodeType uint8
const (
@@ -66,14 +72,22 @@ func New() *Node {
}
type Node struct {
path string
indices string
wildChild bool
nType nodeType
priority uint32
children []*Node // child nodes, at most 1 :param style Node at the end of the array
handler Handler
fullPath string
path string
indices string
wildChild bool
nType nodeType
priority uint32
children []*Node // child nodes, at most 1 :param style Node at the end of the array
handler Handler
fullPath string
maxParams uint16
maxSections uint16
}
type skippedNode struct {
path string
node *Node
paramsCount int16
}
// Increments priority of the given child and reorders if necessary
@@ -100,6 +114,12 @@ func (n *Node) incrementChildPrio(pos int) int {
}
func (n *Node) AddRoute(path string, handlers Handler) {
if paramsCount := countParams(path); paramsCount > n.maxParams {
n.maxParams = paramsCount
}
if sectionsCount := countSections(path); sectionsCount > n.maxSections {
n.maxSections = sectionsCount
}
n.addRoute(path, handlers)
}
@@ -356,8 +376,10 @@ type NodeValue struct {
FullPath string
}
func (n *Node) GetValue(path string, params *Params, unescape bool) (value NodeValue) {
return n.getValue(path, params, unescape)
func (n *Node) GetValue(path string, unescape bool) (value NodeValue) {
skippedNodes := make([]skippedNode, 0, n.maxSections)
ps := make(Params, 0, n.maxParams)
return n.getValue(path, &ps, &skippedNodes, unescape)
}
// Returns the handle registered with the given path (key). The values of
@@ -366,11 +388,8 @@ func (n *Node) GetValue(path string, params *Params, unescape bool) (value NodeV
// made if a handle exists with an extra (without the) trailing slash for the
// given path.
// nolint:gocyclo
func (n *Node) getValue(path string, params *Params, unescape bool) (value NodeValue) {
var (
skippedPath string
latestNode = n // Caching the latest Node
)
func (n *Node) getValue(path string, params *Params, skippedNodes *[]skippedNode, unescape bool) (value NodeValue) {
var globalParamsCount int16
walk: // Outer loop for walking the tree
for {
@@ -385,15 +404,20 @@ walk: // Outer loop for walking the tree
if c == idxc {
// strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild
if n.wildChild {
skippedPath = prefix + path
latestNode = &Node{
path: n.path,
wildChild: n.wildChild,
nType: n.nType,
priority: n.priority,
children: n.children,
handler: n.handler,
fullPath: n.fullPath,
index := len(*skippedNodes)
*skippedNodes = (*skippedNodes)[:index+1]
(*skippedNodes)[index] = skippedNode{
path: prefix + path,
node: &Node{
path: n.path,
wildChild: n.wildChild,
nType: n.nType,
priority: n.priority,
children: n.children,
handler: n.handler,
fullPath: n.fullPath,
},
paramsCount: globalParamsCount,
}
}
@@ -401,15 +425,26 @@ walk: // Outer loop for walking the tree
continue walk
}
}
// 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 be equal to the latest matching Node
matched := path != "/" && !n.wildChild
if matched {
n = latestNode
}
// If there is no wildcard pattern, recommend a redirection
if !n.wildChild {
// 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 vaild skippedNode
if path != "/" {
for l := len(*skippedNodes); l > 0; {
skippedNode := (*skippedNodes)[l-1]
*skippedNodes = (*skippedNodes)[:l-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
}
}
}
// Nothing found.
// We can recommend to redirect to the same URL without a
// trailing slash if a leaf exists for that path.
@@ -419,18 +454,12 @@ walk: // Outer loop for walking the tree
// Handle wildcard child, which is always at the end of the array
n = n.children[len(n.children)-1]
globalParamsCount++
switch n.nType {
case param:
// fix truncate the parameter
// tree_test.go line: 204
if matched {
path = prefix + path
// The saved path is used after the prefix route is intercepted by matching
if n.indices == "/" {
path = skippedPath[1:]
}
}
// Find param end (either '/' or path end)
end := 0
@@ -516,9 +545,22 @@ walk: // Outer loop for walking the tree
if path == prefix {
// 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 be equal to the latest matching Node
if latestNode.wildChild && n.handler == nil && path != "/" {
n = latestNode.children[len(latestNode.children)-1]
// the current node needs to roll back to last vaild skippedNode
if n.handler == nil && path != "/" {
for l := len(*skippedNodes); l > 0; {
skippedNode := (*skippedNodes)[l-1]
*skippedNodes = (*skippedNodes)[:l-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
}
}
// n = latestNode.children[len(latestNode.children)-1]
}
// We should have reached the Node containing the handle.
// Check if this Node has a handle registered.
@@ -549,25 +591,29 @@ walk: // Outer loop for walking the tree
return
}
if path != "/" && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) {
path = skippedPath
// Reduce the number of cycles
n, latestNode = latestNode, n
// skippedPath cannot execute
// example:
// * /:cc/cc
// call /a/cc expectations:match/200 Actual:match/200
// call /a/dd expectations:unmatch/404 Actual: panic
// call /addr/dd/aa expectations:unmatch/404 Actual: panic
// skippedPath: It can only be executed if the secondary route is not found
skippedPath = ""
continue walk
}
// Nothing found. We can recommend to redirect to the same URL with an
// extra trailing slash if a leaf exists for that path
value.Tsr = path == "/" ||
(len(prefix) == len(path)+1 && n.handler != nil)
(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
path == prefix[:len(prefix)-1] && n.handler != nil)
// roll back to last valid skippedNode
if !value.Tsr && path != "/" {
for l := len(*skippedNodes); l > 0; {
skippedNode := (*skippedNodes)[l-1]
*skippedNodes = (*skippedNodes)[:l-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
}
}
}
return
}
}
pkg/services/live/pipeline/tree/tree_test.go
+52 -33
View File
@@ -29,11 +29,6 @@ type testRequests []struct {
ps Params
}
func getParams() *Params {
ps := make(Params, 0, 20)
return &ps
}
func checkRequests(t *testing.T, tree *Node, requests testRequests, unescapes ...bool) {
unescape := false
if len(unescapes) >= 1 {
@@ -41,7 +36,7 @@ func checkRequests(t *testing.T, tree *Node, requests testRequests, unescapes ..
}
for _, request := range requests {
value := tree.getValue(request.path, getParams(), unescape)
value := tree.GetValue(request.path, unescape)
if value.Handler == nil {
if !request.nilHandler {
@@ -114,7 +109,7 @@ func TestTreeAddAndGet(t *testing.T) {
"/β",
}
for _, route := range routes {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
}
checkRequests(t, tree, testRequests{
@@ -161,6 +156,8 @@ func TestTreeWildcard(t *testing.T) {
"/aa/*xx",
"/ab/*xx",
"/:cc",
"/c1/:dd/e",
"/c1/:dd/e1",
"/:cc/cc",
"/:cc/:dd/ee",
"/:cc/:dd/:ee/ff",
@@ -191,7 +188,7 @@ func TestTreeWildcard(t *testing.T) {
"/get/abc/123abfff/:param",
}
for _, route := range routes {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
}
checkRequests(t, tree, testRequests{
@@ -242,6 +239,9 @@ func TestTreeWildcard(t *testing.T) {
{"/alldd", false, "/:cc", Params{Param{Key: "cc", Value: "alldd"}}},
{"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "all"}}},
{"/a/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "a"}}},
{"/c1/d/e", false, "/c1/:dd/e", Params{Param{Key: "dd", Value: "d"}}},
{"/c1/d/e1", false, "/c1/:dd/e1", Params{Param{Key: "dd", Value: "d"}}},
{"/c1/d/ee", false, "/:cc/:dd/ee", Params{Param{Key: "cc", Value: "c1"}, Param{Key: "dd", Value: "d"}}},
{"/cc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "cc"}}},
{"/ccc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ccc"}}},
{"/deedwjfs/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "deedwjfs"}}},
@@ -328,7 +328,7 @@ func TestUnescapeParameters(t *testing.T) {
"/info/:user",
}
for _, route := range routes {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
}
checkRequests(t, tree, testRequests{
@@ -369,7 +369,7 @@ func testRoutes(t *testing.T, routes []testRoute) {
for _, route := range routes {
recv := catchPanic(func() {
tree.addRoute(route.path, nil)
tree.AddRoute(route.path, nil)
})
if route.conflict {
@@ -452,7 +452,7 @@ func TestTreeDuplicatePath(t *testing.T) {
}
for _, route := range routes {
recv := catchPanic(func() {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
})
if recv != nil {
t.Fatalf("panic inserting route '%s': %v", route, recv)
@@ -460,7 +460,7 @@ func TestTreeDuplicatePath(t *testing.T) {
// Add again
recv = catchPanic(func() {
tree.addRoute(route, nil)
tree.AddRoute(route, nil)
})
if recv == nil {
t.Fatalf("no panic while inserting duplicate route '%s", route)
@@ -489,7 +489,7 @@ func TestEmptyWildcardName(t *testing.T) {
}
for _, route := range routes {
recv := catchPanic(func() {
tree.addRoute(route, nil)
tree.AddRoute(route, nil)
})
if recv == nil {
t.Fatalf("no panic while inserting route with empty wildcard name '%s", route)
@@ -519,7 +519,7 @@ func TestTreeCatchAllConflictRoot(t *testing.T) {
func TestTreeCatchMaxParams(t *testing.T) {
tree := &Node{}
var route = "/cmd/*filepath"
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
}
func TestTreeDoubleWildcard(t *testing.T) {
@@ -534,7 +534,7 @@ func TestTreeDoubleWildcard(t *testing.T) {
for _, route := range routes {
tree := &Node{}
recv := catchPanic(func() {
tree.addRoute(route, nil)
tree.AddRoute(route, nil)
})
if rs, ok := recv.(string); !ok || !strings.HasPrefix(rs, panicMsg) {
@@ -580,11 +580,18 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/doc/go1.html",
"/no/a",
"/no/b",
"/api/hello/:name",
"/api/:page/:name",
"/api/hello/:name/bar/",
"/api/bar/:name",
"/api/baz/foo",
"/api/baz/foo/bar",
"/blog/:p",
"/posts/:b/:c",
"/posts/b/:c/d/",
}
for _, route := range routes {
recv := catchPanic(func() {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
})
if recv != nil {
t.Fatalf("panic inserting route '%s': %v", route, recv)
@@ -606,9 +613,20 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/admin/config/",
"/admin/config/permissions/",
"/doc/",
"/admin/static/",
"/admin/cfg/",
"/admin/cfg/users/",
"/api/hello/x/bar",
"/api/baz/foo/",
"/api/baz/bax/",
"/api/bar/huh/",
"/api/baz/foo/bar/",
"/api/world/abc/",
"/blog/pp/",
"/posts/b/c/d",
}
for _, route := range tsrRoutes {
value := tree.getValue(route, nil, false)
value := tree.GetValue(route, false)
if value.Handler != nil {
t.Fatalf("non-nil handler for TSR route '%s", route)
} else if !value.Tsr {
@@ -622,10 +640,14 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/no/",
"/_",
"/_/",
"/api/world/abc",
"/api",
"/api/",
"/api/hello/x/foo",
"/api/baz/foo/bad",
"/foo/p/p",
}
for _, route := range noTsrRoutes {
value := tree.getValue(route, nil, false)
value := tree.GetValue(route, false)
if value.Handler != nil {
t.Fatalf("non-nil handler for No-TSR route '%s", route)
} else if value.Tsr {
@@ -638,13 +660,13 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) {
tree := &Node{}
recv := catchPanic(func() {
tree.addRoute("/:test", fakeHandler("/:test"))
tree.AddRoute("/:test", fakeHandler("/:test"))
})
if recv != nil {
t.Fatalf("panic inserting test route: %v", recv)
}
value := tree.getValue("/", nil, false)
value := tree.GetValue("/", false)
if value.Handler != nil {
t.Fatalf("non-nil handler")
} else if value.Tsr {
@@ -696,7 +718,7 @@ func TestTreeFindCaseInsensitivePath(t *testing.T) {
for _, route := range routes {
recv := catchPanic(func() {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
})
if recv != nil {
t.Fatalf("panic inserting route '%s': %v", route, recv)
@@ -816,15 +838,15 @@ func TestTreeInvalidNodeType(t *testing.T) {
const panicMsg = "invalid Node type"
tree := &Node{}
tree.addRoute("/", fakeHandler("/"))
tree.addRoute("/:page", fakeHandler("/:page"))
tree.AddRoute("/", fakeHandler("/"))
tree.AddRoute("/:page", fakeHandler("/:page"))
// set invalid Node type
tree.children[0].nType = 42
// normal lookup
recv := catchPanic(func() {
tree.getValue("/test", nil, false)
tree.GetValue("/test", false)
})
if rs, ok := recv.(string); !ok || rs != panicMsg {
t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv)
@@ -845,11 +867,8 @@ func TestTreeInvalidParamsType(t *testing.T) {
tree.children = append(tree.children, &Node{})
tree.children[0].nType = 2
// set invalid Params type
params := make(Params, 0)
// try to trigger slice bounds out of range with capacity 0
tree.getValue("/test", &params, false)
tree.GetValue("/test", false)
}
func TestTreeWildcardConflictEx(t *testing.T) {
@@ -868,7 +887,7 @@ func TestTreeWildcardConflictEx(t *testing.T) {
for _, conflict := range conflicts {
// I have to re-create a 'tree', because the 'tree' will be
// in an inconsistent state when the loop recovers from the
// panic which threw by 'addRoute' function.
// panic which threw by 'AddRoute' function.
tree := &Node{}
routes := [...]string{
"/con:tact",
@@ -877,11 +896,11 @@ func TestTreeWildcardConflictEx(t *testing.T) {
}
for _, route := range routes {
tree.addRoute(route, fakeHandler(route))
tree.AddRoute(route, fakeHandler(route))
}
recv := catchPanic(func() {
tree.addRoute(conflict.route, fakeHandler(conflict.route))
tree.AddRoute(conflict.route, fakeHandler(conflict.route))
})
if !regexp.MustCompile(fmt.Sprintf("'%s' in new path .* conflicts with existing wildcard '%s' in existing prefix '%s'", conflict.segPath, conflict.existSegPath, conflict.existPath)).MatchString(fmt.Sprint(recv)) {