diff --git a/tree.go b/tree.go index b6530665..41861366 100644 --- a/tree.go +++ b/tree.go @@ -8,6 +8,7 @@ import ( "net/url" "strings" "unicode" + "unicode/utf8" ) // Param is a single URL parameter, consisting of a key and a value. @@ -186,16 +187,24 @@ func (n *node) addRoute(path string, handlers HandlersChain) { numParams-- // Check if the wildcard matches - if len(path) >= len(n.path) && n.path == path[:len(n.path)] { + if len(path) >= len(n.path) && n.path == path[:len(n.path)] && (len(n.path) >= len(path) || path[len(n.path)] == '/') { // check for longer wildcard, e.g. :name and :names - if len(n.path) >= len(path) || path[len(n.path)] == '/' { - continue walk + continue walk + } else { + // Wildcard conflict + var pathSeg string + if n.nType == catchAll { + pathSeg = path + } else { + pathSeg = strings.SplitN(path, "/", 2)[0] } + prefix := fullPath[:strings.Index(fullPath, pathSeg)] + n.path + panic("'" + pathSeg + + "' in new path '" + fullPath + + "' conflicts with existing wildcard '" + n.path + + "' in existing prefix '" + prefix + + "'") } - - panic("path segment '" + path + - "' conflicts with existing wildcard '" + n.path + - "' in path '" + fullPath + "'") } c := path[0] @@ -229,7 +238,6 @@ func (n *node) addRoute(path string, handlers HandlersChain) { } n.insertChild(numParams, path, fullPath, handlers) return - } else if i == len(path) { // Make node a (in-path) leaf if n.handlers != nil { panic("handlers are already registered for path '" + fullPath + "'") @@ -364,7 +372,7 @@ func (n *node) insertChild(numParams uint8, path string, fullPath string, handle // given path. func (n *node) getValue(path string, po Params, unescape bool) (handlers HandlersChain, p Params, tsr bool) { p = po -walk: // Outer loop for walking the tree +walk: // outer loop for walking the tree for { if len(path) > len(n.path) { if path[:len(n.path)] == n.path { @@ -504,34 +512,117 @@ walk: // Outer loop for walking the tree // It returns the case-corrected path and a bool indicating whether the lookup // was successful. func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPath []byte, found bool) { - ciPath = make([]byte, 0, len(path)+1) // preallocate enough memory + return n.findCaseInsensitivePathRec( + path, + strings.ToLower(path), + make([]byte, 0, len(path)+1), // preallocate enough memory for new path + [4]byte{}, // empty rune buffer + fixTrailingSlash, + ) +} - // Outer loop for walking the tree - for len(path) >= len(n.path) && strings.ToLower(path[:len(n.path)]) == strings.ToLower(n.path) { - path = path[len(n.path):] +// shift bytes in array by n bytes left +func shiftNRuneBytes(rb [4]byte, n int) [4]byte { + switch n { + case 0: + return rb + case 1: + return [4]byte{rb[1], rb[2], rb[3], 0} + case 2: + return [4]byte{rb[2], rb[3]} + case 3: + return [4]byte{rb[3]} + default: + return [4]byte{} + } +} + +func (n *node) findCaseInsensitivePathRec( + path, loPath string, ciPath []byte, rb [4]byte, fixTrailingSlash bool, +) ([]byte, bool) { + loNPath := strings.ToLower(n.path) + +walk: // outer loop for walking the tree + for len(loPath) >= len(loNPath) && (len(loNPath) == 0 || loPath[1:len(loNPath)] == loNPath[1:]) { + // add common path to result ciPath = append(ciPath, n.path...) - if len(path) > 0 { + if path = path[len(n.path):]; len(path) > 0 { + loOld := loPath + loPath = loPath[len(loNPath):] + // If this node does not have a wildcard (param or catchAll) child, // we can just look up the next child node and continue to walk down // the tree if !n.wildChild { - r := unicode.ToLower(rune(path[0])) - for i, index := range n.indices { - // must use recursive approach since both index and - // ToLower(index) could exist. We must check both. - if r == unicode.ToLower(index) { - out, found := n.children[i].findCaseInsensitivePath(path, fixTrailingSlash) - if found { - return append(ciPath, out...), true + // skip rune bytes already processed + rb = shiftNRuneBytes(rb, len(loNPath)) + + if rb[0] != 0 { + // old rune not finished + for i := 0; i < len(n.indices); i++ { + if n.indices[i] == rb[0] { + // continue with child node + n = n.children[i] + loNPath = strings.ToLower(n.path) + continue walk + } + } + } else { + // process a new rune + var rv rune + + // find rune start + // runes are up to 4 byte long, + // -4 would definitely be another rune + var off int + for max := min(len(loNPath), 3); off < max; off++ { + if i := len(loNPath) - off; utf8.RuneStart(loOld[i]) { + // read rune from cached lowercase path + rv, _ = utf8.DecodeRuneInString(loOld[i:]) + break + } + } + + // calculate lowercase bytes of current rune + utf8.EncodeRune(rb[:], rv) + // skip already processed bytes + rb = shiftNRuneBytes(rb, off) + + for i := 0; i < len(n.indices); i++ { + // lowercase matches + if n.indices[i] == rb[0] { + // must use recursive approach since both the uppercase byte + // and the lowercase byte might exist as an index + if out, found := n.children[i].findCaseInsensitivePathRec( + path, loPath, ciPath, rb, fixTrailingSlash, + ); found { + return out, true + } + break + } + } + + // same for uppercase rune, if it differs + if up := unicode.ToUpper(rv); up != rv { + utf8.EncodeRune(rb[:], up) + rb = shiftNRuneBytes(rb, off) + + for i := 0; i < len(n.indices); i++ { + // uppercase matches + if n.indices[i] == rb[0] { + // continue with child node + n = n.children[i] + loNPath = strings.ToLower(n.path) + continue walk + } } } } // Nothing found. We can recommend to redirect to the same URL // without a trailing slash if a leaf exists for that path - found = fixTrailingSlash && path == "/" && n.handlers != nil - return + return ciPath, (fixTrailingSlash && path == "/" && n.handlers != nil) } n = n.children[0] @@ -549,8 +640,11 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa // we need to go deeper! if k < len(path) { if len(n.children) > 0 { - path = path[k:] + // continue with child node n = n.children[0] + loNPath = strings.ToLower(n.path) + loPath = loPath[k:] + path = path[k:] continue } @@ -558,7 +652,7 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa if fixTrailingSlash && len(path) == k+1 { return ciPath, true } - return + return ciPath, false } if n.handlers != nil { @@ -571,7 +665,7 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa return append(ciPath, '/'), true } } - return + return ciPath, false case catchAll: return append(ciPath, path...), true @@ -596,11 +690,11 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa (n.nType == catchAll && n.children[0].handlers != nil) { return append(ciPath, '/'), true } - return + return ciPath, false } } } - return + return ciPath, false } } @@ -610,11 +704,11 @@ func (n *node) findCaseInsensitivePath(path string, fixTrailingSlash bool) (ciPa if path == "/" { return ciPath, true } - if len(path)+1 == len(n.path) && n.path[len(path)] == '/' && - strings.ToLower(path) == strings.ToLower(n.path[:len(path)]) && - n.handlers != nil { + if len(loPath)+1 == len(loNPath) && loNPath[len(loPath)] == '/' && + loPath[1:] == loNPath[1:len(loPath)] && n.handlers != nil { return append(ciPath, n.path...), true } } - return + + return ciPath, false } diff --git a/tree_test.go b/tree_test.go index 5bc27171..147762a5 100644 --- a/tree_test.go +++ b/tree_test.go @@ -5,7 +5,9 @@ package gin import ( + "fmt" "reflect" + "regexp" "strings" "testing" ) @@ -664,3 +666,42 @@ func TestTreeInvalidNodeType(t *testing.T) { t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv) } } + +func TestTreeWildcardConflictEx(t *testing.T) { + conflicts := [...]struct { + route string + segPath string + existPath string + existSegPath string + }{ + {"/who/are/foo", "/foo", `/who/are/\*you`, `/\*you`}, + {"/who/are/foo/", "/foo/", `/who/are/\*you`, `/\*you`}, + {"/who/are/foo/bar", "/foo/bar", `/who/are/\*you`, `/\*you`}, + {"/conxxx", "xxx", `/con:tact`, `:tact`}, + {"/conooo/xxx", "ooo", `/con:tact`, `:tact`}, + } + + 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. + tree := &node{} + routes := [...]string{ + "/con:tact", + "/who/are/*you", + "/who/foo/hello", + } + + for _, route := range routes { + tree.addRoute(route, fakeHandler(route)) + } + + recv := catchPanic(func() { + 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)) { + t.Fatalf("invalid wildcard conflict error (%v)", recv) + } + } +}