Update the code logic for latestNode in tree.go (#2897)

This commit is contained in:
Zhu Xi 2021-10-23 11:58:57 +08:00 committed by GitHub
parent 34ae7777ca
commit 3fe928994b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 120 additions and 52 deletions

View File

@ -55,8 +55,9 @@ type Context struct {
index int8 index int8
fullPath string fullPath string
engine *Engine engine *Engine
params *Params params *Params
skippedNodes *[]skippedNode
// This mutex protect Keys map // This mutex protect Keys map
mu sync.RWMutex mu sync.RWMutex
@ -99,6 +100,7 @@ func (c *Context) reset() {
c.queryCache = nil c.queryCache = nil
c.formCache = nil c.formCache = nil
*c.params = (*c.params)[:0] *c.params = (*c.params)[:0]
*c.skippedNodes = (*c.skippedNodes)[:0]
} }
// Copy returns a copy of the current context that can be safely used outside the request's scope. // Copy returns a copy of the current context that can be safely used outside the request's scope.

12
gin.go
View File

@ -144,6 +144,7 @@ type Engine struct {
pool sync.Pool pool sync.Pool
trees methodTrees trees methodTrees
maxParams uint16 maxParams uint16
maxSections uint16
trustedProxies []string trustedProxies []string
trustedCIDRs []*net.IPNet trustedCIDRs []*net.IPNet
} }
@ -200,7 +201,8 @@ func Default() *Engine {
func (engine *Engine) allocateContext() *Context { func (engine *Engine) allocateContext() *Context {
v := make(Params, 0, engine.maxParams) v := make(Params, 0, engine.maxParams)
return &Context{engine: engine, params: &v} skippedNodes := make([]skippedNode, 0, engine.maxSections)
return &Context{engine: engine, params: &v, skippedNodes: &skippedNodes}
} }
// Delims sets template left and right delims and returns a Engine instance. // Delims sets template left and right delims and returns a Engine instance.
@ -306,6 +308,10 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) {
if paramsCount := countParams(path); paramsCount > engine.maxParams { if paramsCount := countParams(path); paramsCount > engine.maxParams {
engine.maxParams = paramsCount engine.maxParams = paramsCount
} }
if sectionsCount := countSections(path); sectionsCount > engine.maxSections {
engine.maxSections = sectionsCount
}
} }
// Routes returns a slice of registered routes, including some useful information, such as: // Routes returns a slice of registered routes, including some useful information, such as:
@ -539,7 +545,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
} }
root := t[i].root root := t[i].root
// Find route in tree // Find route in tree
value := root.getValue(rPath, c.params, unescape) value := root.getValue(rPath, c.params, c.skippedNodes, unescape)
if value.params != nil { if value.params != nil {
c.Params = *value.params c.Params = *value.params
} }
@ -567,7 +573,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) {
if tree.method == httpMethod { if tree.method == httpMethod {
continue continue
} }
if value := tree.root.getValue(rPath, nil, unescape); value.handlers != nil { if value := tree.root.getValue(rPath, nil, c.skippedNodes, unescape); value.handlers != nil {
c.handlers = engine.allNoMethod c.handlers = engine.allNoMethod
serveError(c, http.StatusMethodNotAllowed, default405Body) serveError(c, http.StatusMethodNotAllowed, default405Body)
return return

View File

@ -408,8 +408,13 @@ func TestTreeRunDynamicRouting(t *testing.T) {
router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") }) router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") })
router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") }) router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") })
router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") }) router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") })
router.GET("/c1/:dd/e", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/e") })
router.GET("/c1/:dd/e1", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/e1") })
router.GET("/c1/:dd/f1", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/f1") })
router.GET("/c1/:dd/f2", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/f2") })
router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") }) router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") })
router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") }) router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") })
router.GET("/:cc/:dd/f", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/f") })
router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") }) router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") })
router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") }) router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") })
router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") }) router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") })
@ -446,6 +451,10 @@ func TestTreeRunDynamicRouting(t *testing.T) {
testRequest(t, ts.URL+"/all", "", "/:cc") testRequest(t, ts.URL+"/all", "", "/:cc")
testRequest(t, ts.URL+"/all/cc", "", "/:cc/cc") testRequest(t, ts.URL+"/all/cc", "", "/:cc/cc")
testRequest(t, ts.URL+"/a/cc", "", "/:cc/cc") testRequest(t, ts.URL+"/a/cc", "", "/:cc/cc")
testRequest(t, ts.URL+"/c1/d/e", "", "/c1/:dd/e")
testRequest(t, ts.URL+"/c1/d/e1", "", "/c1/:dd/e1")
testRequest(t, ts.URL+"/c1/d/ee", "", "/:cc/:dd/ee")
testRequest(t, ts.URL+"/c1/d/f", "", "/:cc/:dd/f")
testRequest(t, ts.URL+"/c/d/ee", "", "/:cc/:dd/ee") testRequest(t, ts.URL+"/c/d/ee", "", "/:cc/:dd/ee")
testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff") testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff")
testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg") testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg")
@ -528,6 +537,12 @@ func TestTreeRunDynamicRouting(t *testing.T) {
testRequest(t, ts.URL+"/get/abc/123abf/testss", "", "/get/abc/123abf/:param") testRequest(t, ts.URL+"/get/abc/123abf/testss", "", "/get/abc/123abf/:param")
testRequest(t, ts.URL+"/get/abc/123abfff/te", "", "/get/abc/123abfff/:param") testRequest(t, ts.URL+"/get/abc/123abfff/te", "", "/get/abc/123abfff/:param")
// 404 not found // 404 not found
testRequest(t, ts.URL+"/c/d/e", "404 Not Found")
testRequest(t, ts.URL+"/c/d/e1", "404 Not Found")
testRequest(t, ts.URL+"/c/d/eee", "404 Not Found")
testRequest(t, ts.URL+"/c1/d/eee", "404 Not Found")
testRequest(t, ts.URL+"/c1/d/e2", "404 Not Found")
testRequest(t, ts.URL+"/cc/dd/ee/ff/gg/hh1", "404 Not Found")
testRequest(t, ts.URL+"/a/dd", "404 Not Found") testRequest(t, ts.URL+"/a/dd", "404 Not Found")
testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found") testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found")
testRequest(t, ts.URL+"/something/secondthing/121", "404 Not Found") testRequest(t, ts.URL+"/something/secondthing/121", "404 Not Found")

117
tree.go
View File

@ -17,6 +17,7 @@ import (
var ( var (
strColon = []byte(":") strColon = []byte(":")
strStar = []byte("*") strStar = []byte("*")
strSlash = []byte("/")
) )
// Param is a single URL parameter, consisting of a key and a value. // Param is a single URL parameter, consisting of a key and a value.
@ -98,6 +99,11 @@ func countParams(path string) uint16 {
return n return n
} }
func countSections(path string) uint16 {
s := bytesconv.StringToBytes(path)
return uint16(bytes.Count(s, strSlash))
}
type nodeType uint8 type nodeType uint8
const ( const (
@ -393,16 +399,19 @@ type nodeValue struct {
fullPath string fullPath string
} }
type skippedNode struct {
path string
node *node
paramsCount int16
}
// Returns the handle registered with the given path (key). The values of // Returns the handle registered with the given path (key). The values of
// wildcards are saved to a map. // wildcards are saved to a map.
// If no handle can be found, a TSR (trailing slash redirect) recommendation is // If no handle can be found, a TSR (trailing slash redirect) recommendation is
// made if a handle exists with an extra (without the) trailing slash for the // made if a handle exists with an extra (without the) trailing slash for the
// given path. // given path.
func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) { func (n *node) getValue(path string, params *Params, skippedNodes *[]skippedNode, unescape bool) (value nodeValue) {
var ( var globalParamsCount int16
skippedPath string
latestNode = n // Caching the latest node
)
walk: // Outer loop for walking the tree walk: // Outer loop for walking the tree
for { for {
@ -417,15 +426,20 @@ walk: // Outer loop for walking the tree
if c == idxc { if c == idxc {
// strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild // strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild
if n.wildChild { if n.wildChild {
skippedPath = prefix + path index := len(*skippedNodes)
latestNode = &node{ *skippedNodes = (*skippedNodes)[:index+1]
path: n.path, (*skippedNodes)[index] = skippedNode{
wildChild: n.wildChild, path: prefix + path,
nType: n.nType, node: &node{
priority: n.priority, path: n.path,
children: n.children, wildChild: n.wildChild,
handlers: n.handlers, nType: n.nType,
fullPath: n.fullPath, priority: n.priority,
children: n.children,
handlers: n.handlers,
fullPath: n.fullPath,
},
paramsCount: globalParamsCount,
} }
} }
@ -434,10 +448,22 @@ walk: // Outer loop for walking the tree
} }
} }
// If the path at the end of the loop is not equal to '/' and the current node has no child nodes // 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 // the current node needs to roll back to last vaild skippedNode
matched := path != "/" && !n.wildChild
if matched { if path != "/" && !n.wildChild {
n = latestNode 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
}
}
} }
// If there is no wildcard pattern, recommend a redirection // If there is no wildcard pattern, recommend a redirection
@ -451,18 +477,12 @@ walk: // Outer loop for walking the tree
// Handle wildcard child, which is always at the end of the array // Handle 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]
globalParamsCount++
switch n.nType { switch n.nType {
case param: case param:
// fix truncate the parameter // fix truncate the parameter
// tree_test.go line: 204 // 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) // Find param end (either '/' or path end)
end := 0 end := 0
@ -548,9 +568,22 @@ walk: // Outer loop for walking the tree
if path == prefix { 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 // 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 // the current node needs to roll back to last vaild skippedNode
if latestNode.wildChild && n.handlers == nil && path != "/" { if n.handlers == nil && path != "/" {
n = latestNode.children[len(latestNode.children)-1] 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. // We should have reached the node containing the handle.
// Check if this node has a handle registered. // Check if this node has a handle registered.
@ -581,19 +614,21 @@ walk: // Outer loop for walking the tree
return return
} }
if path != "/" && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) { // roll back to last vaild skippedNode
path = skippedPath if path != "/" {
// Reduce the number of cycles for l := len(*skippedNodes); l > 0; {
n, latestNode = latestNode, n skippedNode := (*skippedNodes)[l-1]
// skippedPath cannot execute *skippedNodes = (*skippedNodes)[:l-1]
// example: if strings.HasSuffix(skippedNode.path, path) {
// * /:cc/cc path = skippedNode.path
// call /a/cc expectations:match/200 Actual:match/200 n = skippedNode.node
// call /a/dd expectations:unmatch/404 Actual: panic if value.params != nil {
// call /addr/dd/aa expectations:unmatch/404 Actual: panic *value.params = (*value.params)[:skippedNode.paramsCount]
// skippedPath: It can only be executed if the secondary route is not found }
skippedPath = "" globalParamsCount = skippedNode.paramsCount
continue walk continue walk
}
}
} }
// Nothing found. We can recommend to redirect to the same URL with an // Nothing found. We can recommend to redirect to the same URL with an

View File

@ -33,6 +33,11 @@ func getParams() *Params {
return &ps return &ps
} }
func getSkippedNodes() *[]skippedNode {
ps := make([]skippedNode, 0, 20)
return &ps
}
func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ...bool) { func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ...bool) {
unescape := false unescape := false
if len(unescapes) >= 1 { if len(unescapes) >= 1 {
@ -40,7 +45,7 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ..
} }
for _, request := range requests { for _, request := range requests {
value := tree.getValue(request.path, getParams(), unescape) value := tree.getValue(request.path, getParams(), getSkippedNodes(), unescape)
if value.handlers == nil { if value.handlers == nil {
if !request.nilHandler { if !request.nilHandler {
@ -157,6 +162,8 @@ func TestTreeWildcard(t *testing.T) {
"/aa/*xx", "/aa/*xx",
"/ab/*xx", "/ab/*xx",
"/:cc", "/:cc",
"/c1/:dd/e",
"/c1/:dd/e1",
"/:cc/cc", "/:cc/cc",
"/:cc/:dd/ee", "/:cc/:dd/ee",
"/:cc/:dd/:ee/ff", "/:cc/:dd/:ee/ff",
@ -238,6 +245,9 @@ func TestTreeWildcard(t *testing.T) {
{"/alldd", false, "/:cc", Params{Param{Key: "cc", Value: "alldd"}}}, {"/alldd", false, "/:cc", Params{Param{Key: "cc", Value: "alldd"}}},
{"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "all"}}}, {"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "all"}}},
{"/a/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "a"}}}, {"/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"}}}, {"/cc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "cc"}}},
{"/ccc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ccc"}}}, {"/ccc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ccc"}}},
{"/deedwjfs/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "deedwjfs"}}}, {"/deedwjfs/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "deedwjfs"}}},
@ -605,7 +615,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/doc/", "/doc/",
} }
for _, route := range tsrRoutes { for _, route := range tsrRoutes {
value := tree.getValue(route, nil, false) value := tree.getValue(route, nil, getSkippedNodes(), false)
if value.handlers != nil { if value.handlers != nil {
t.Fatalf("non-nil handler for TSR route '%s", route) t.Fatalf("non-nil handler for TSR route '%s", route)
} else if !value.tsr { } else if !value.tsr {
@ -622,7 +632,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
"/api/world/abc", "/api/world/abc",
} }
for _, route := range noTsrRoutes { for _, route := range noTsrRoutes {
value := tree.getValue(route, nil, false) value := tree.getValue(route, nil, getSkippedNodes(), false)
if value.handlers != nil { if value.handlers != nil {
t.Fatalf("non-nil handler for No-TSR route '%s", route) t.Fatalf("non-nil handler for No-TSR route '%s", route)
} else if value.tsr { } else if value.tsr {
@ -641,7 +651,7 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) {
t.Fatalf("panic inserting test route: %v", recv) t.Fatalf("panic inserting test route: %v", recv)
} }
value := tree.getValue("/", nil, false) value := tree.getValue("/", nil, getSkippedNodes(), false)
if value.handlers != nil { if value.handlers != nil {
t.Fatalf("non-nil handler") t.Fatalf("non-nil handler")
} else if value.tsr { } else if value.tsr {
@ -821,7 +831,7 @@ func TestTreeInvalidNodeType(t *testing.T) {
// normal lookup // normal lookup
recv := catchPanic(func() { recv := catchPanic(func() {
tree.getValue("/test", nil, false) tree.getValue("/test", nil, getSkippedNodes(), false)
}) })
if rs, ok := recv.(string); !ok || rs != panicMsg { if rs, ok := recv.(string); !ok || rs != panicMsg {
t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv) t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv)
@ -846,7 +856,7 @@ func TestTreeInvalidParamsType(t *testing.T) {
params := make(Params, 0) params := make(Params, 0)
// try to trigger slice bounds out of range with capacity 0 // try to trigger slice bounds out of range with capacity 0
tree.getValue("/test", &params, false) tree.getValue("/test", &params, getSkippedNodes(), false)
} }
func TestTreeWildcardConflictEx(t *testing.T) { func TestTreeWildcardConflictEx(t *testing.T) {