a new fix for #2820

This commit is contained in:
Zhu Xi 2021-10-08 16:56:42 +08:00
parent 1a2bc0e7cb
commit c639b3ef26
2 changed files with 113 additions and 37 deletions

View File

@ -521,6 +521,7 @@ 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+"/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")

149
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,20 @@ 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, unescape bool) (value nodeValue) {
var ( skippedNodes := make([]skippedNode, 0, countSections(path)) // Caching the latest nodes
skippedPath string var globalParamsCount int16 = 0
latestNode = n // Caching the latest node
)
walk: // Outer loop for walking the tree walk: // Outer loop for walking the tree
for { for {
@ -417,15 +427,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,
} }
} }
@ -435,9 +450,34 @@ 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 be equal to the latest matching node
matched := path != "/" && !n.wildChild /*
if matched { matched := path != "/" && !n.wildChild
n = latestNode if matched {
if len(skippedNodes) > 0 {
l := len(skippedNodes)
n = skippedNodes[l-1].node
path = skippedNodes[l-1].path
skippedNodes = skippedNodes[:l-1]
}
//n = latestNode
}
*/
if path != "/" && !n.wildChild {
for len(skippedNodes) > 0 {
l := len(skippedNodes)
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 +491,21 @@ 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 += 1
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 if matched {
// The saved path is used after the prefix route is intercepted by matching path = prefix + path
if n.indices == "/" { // The saved path is used after the prefix route is intercepted by matching
path = skippedPath[1:] if n.indices == "/" {
path = skippedPath[1:]
}
} }
} */
// Find param end (either '/' or path end) // Find param end (either '/' or path end)
end := 0 end := 0
@ -549,8 +592,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 be equal to the latest matching node
if latestNode.wildChild && n.handlers == nil && path != "/" { if n.handlers == nil && path != "/" {
n = latestNode.children[len(latestNode.children)-1] for len(skippedNodes) > 0 {
l := len(skippedNodes)
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,20 +638,38 @@ walk: // Outer loop for walking the tree
return return
} }
if path != "/" && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) { if path != "/" {
path = skippedPath for len(skippedNodes) > 0 {
// Reduce the number of cycles l := len(skippedNodes)
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
}
}
} }
/*
len(skippedNodes) > 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 // Nothing found. We can recommend to redirect to the same URL with an
// extra trailing slash if a leaf exists for that path // extra trailing slash if a leaf exists for that path