From 3e45d2a5b19fff4779bcf26d2e14d796457e5604 Mon Sep 17 00:00:00 2001 From: ortyomka Date: Wed, 29 Jun 2022 14:46:17 +0300 Subject: [PATCH 1/5] Change methodHandler element type to methodContext Signed-off-by: ortyomka --- router.go | 64 ++++++++++++++++++++++++++++++-------------------- router_test.go | 2 +- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/router.go b/router.go index b5e50d94f..d725b9144 100644 --- a/router.go +++ b/router.go @@ -13,6 +13,9 @@ type ( routes map[string]*Route echo *Echo } + methodContext struct { + handler HandlerFunc + } node struct { kind kind label byte @@ -32,17 +35,17 @@ type ( kind uint8 children []*node methodHandler struct { - connect HandlerFunc - delete HandlerFunc - get HandlerFunc - head HandlerFunc - options HandlerFunc - patch HandlerFunc - post HandlerFunc - propfind HandlerFunc - put HandlerFunc - trace HandlerFunc - report HandlerFunc + connect *methodContext + delete *methodContext + get *methodContext + head *methodContext + options *methodContext + patch *methodContext + post *methodContext + propfind *methodContext + put *methodContext + trace *methodContext + report *methodContext allowHeader string } ) @@ -209,7 +212,9 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode.prefix = search if h != nil { currentNode.kind = t - currentNode.addHandler(method, h) + currentNode.addHandler(method, &methodContext{ + handler: h, + }) currentNode.ppath = ppath currentNode.pnames = pnames } @@ -257,13 +262,17 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string if lcpLen == searchLen { // At parent node currentNode.kind = t - currentNode.addHandler(method, h) + currentNode.addHandler(method, &methodContext{ + handler: h, + }) currentNode.ppath = ppath currentNode.pnames = pnames } else { // Create child node n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), ppath, pnames, nil, nil) - n.addHandler(method, h) + n.addHandler(method, &methodContext{ + handler: h, + }) // Only Static children could reach here currentNode.addStaticChild(n) } @@ -278,7 +287,9 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string } // Create child node n := newNode(t, search, currentNode, nil, new(methodHandler), ppath, pnames, nil, nil) - n.addHandler(method, h) + n.addHandler(method, &methodContext{ + handler: h, + }) switch t { case staticKind: currentNode.addStaticChild(n) @@ -291,7 +302,9 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string } else { // Node already exists if h != nil { - currentNode.addHandler(method, h) + currentNode.addHandler(method, &methodContext{ + handler: h, + }) currentNode.ppath = ppath if len(currentNode.pnames) == 0 { // Issue #729 currentNode.pnames = pnames @@ -345,7 +358,12 @@ func (n *node) findChildWithLabel(l byte) *node { return nil } -func (n *node) addHandler(method string, h HandlerFunc) { +func (n *node) addHandler(method string, h *methodContext) { + if h.handler == nil { + n.isHandler = n.methodHandler.isHandler() + return + } + switch method { case http.MethodConnect: n.methodHandler.connect = h @@ -372,14 +390,10 @@ func (n *node) addHandler(method string, h HandlerFunc) { } n.methodHandler.updateAllowHeader() - if h != nil { - n.isHandler = true - } else { - n.isHandler = n.methodHandler.isHandler() - } + n.isHandler = true } -func (n *node) findHandler(method string) HandlerFunc { +func (n *node) findHandler(method string) *methodContext { switch method { case http.MethodConnect: return n.methodHandler.connect @@ -530,7 +544,7 @@ func (r *Router) Find(method, path string, c Context) { previousBestMatchNode = currentNode } if h := currentNode.findHandler(method); h != nil { - matchedHandler = h + matchedHandler = h.handler break } } @@ -581,7 +595,7 @@ func (r *Router) Find(method, path string, c Context) { previousBestMatchNode = currentNode } if h := currentNode.findHandler(method); h != nil { - matchedHandler = h + matchedHandler = h.handler break } } diff --git a/router_test.go b/router_test.go index 457566b90..9ecb1824e 100644 --- a/router_test.go +++ b/router_test.go @@ -2380,7 +2380,7 @@ func TestRouterHandleMethodOptions(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tc.expectStatus, rec.Code) } - assert.Equal(t, tc.expectAllowHeader, c.Response().Header().Get("Allow")) + assert.Equal(t, tc.expectAllowHeader, c.Response().Header().Get(HeaderAllow)) }) } } From d9b49f85e7ceee3308ac370961ec547978428d24 Mon Sep 17 00:00:00 2001 From: ortyomka Date: Wed, 29 Jun 2022 15:35:33 +0300 Subject: [PATCH 2/5] Allow different param names in the smae path with different methods Signed-off-by: ortyomka --- router.go | 46 ++++++++++++++++++++++++---------------------- router_test.go | 21 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/router.go b/router.go index d725b9144..34bbd83aa 100644 --- a/router.go +++ b/router.go @@ -14,6 +14,8 @@ type ( echo *Echo } methodContext struct { + ppath string + pnames []string handler HandlerFunc } node struct { @@ -22,8 +24,6 @@ type ( prefix string parent *node staticChildren children - ppath string - pnames []string methodHandler *methodHandler paramChild *node anyChild *node @@ -213,10 +213,10 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string if h != nil { currentNode.kind = t currentNode.addHandler(method, &methodContext{ + ppath: ppath, + pnames: pnames, handler: h, }) - currentNode.ppath = ppath - currentNode.pnames = pnames } currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil } else if lcpLen < prefixLen { @@ -227,8 +227,6 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode, currentNode.staticChildren, currentNode.methodHandler, - currentNode.ppath, - currentNode.pnames, currentNode.paramChild, currentNode.anyChild, ) @@ -249,8 +247,6 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode.prefix = currentNode.prefix[:lcpLen] currentNode.staticChildren = nil currentNode.methodHandler = new(methodHandler) - currentNode.ppath = "" - currentNode.pnames = nil currentNode.paramChild = nil currentNode.anyChild = nil currentNode.isLeaf = false @@ -263,14 +259,16 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string // At parent node currentNode.kind = t currentNode.addHandler(method, &methodContext{ + ppath: ppath, + pnames: pnames, handler: h, }) - currentNode.ppath = ppath - currentNode.pnames = pnames } else { // Create child node - n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), ppath, pnames, nil, nil) + n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), nil, nil) n.addHandler(method, &methodContext{ + ppath: ppath, + pnames: pnames, handler: h, }) // Only Static children could reach here @@ -286,8 +284,10 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string continue } // Create child node - n := newNode(t, search, currentNode, nil, new(methodHandler), ppath, pnames, nil, nil) + n := newNode(t, search, currentNode, nil, new(methodHandler), nil, nil) n.addHandler(method, &methodContext{ + ppath: ppath, + pnames: pnames, handler: h, }) switch t { @@ -303,27 +303,23 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string // Node already exists if h != nil { currentNode.addHandler(method, &methodContext{ + ppath: ppath, + pnames: pnames, handler: h, }) - currentNode.ppath = ppath - if len(currentNode.pnames) == 0 { // Issue #729 - currentNode.pnames = pnames - } } } return } } -func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, ppath string, pnames []string, paramChildren, anyChildren *node) *node { +func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, paramChildren, anyChildren *node) *node { return &node{ kind: t, label: pre[0], prefix: pre, parent: p, staticChildren: sc, - ppath: ppath, - pnames: pnames, methodHandler: mh, paramChild: paramChildren, anyChild: anyChildren, @@ -583,7 +579,11 @@ func (r *Router) Find(method, path string, c Context) { if child := currentNode.anyChild; child != nil { // If any node is found, use remaining path for paramValues currentNode = child - paramValues[len(currentNode.pnames)-1] = search + if m := currentNode.findHandler(method); m != nil { + paramValues[len(m.pnames)-1] = search + } else { + break + } // update indexes/search in case we need to backtrack when no handler match is found paramIndex++ searchIndex += +len(search) @@ -634,6 +634,8 @@ func (r *Router) Find(method, path string, c Context) { } } } - ctx.path = currentNode.ppath - ctx.pnames = currentNode.pnames + if m := currentNode.findHandler(method); m != nil { + ctx.path = m.ppath + ctx.pnames = m.pnames + } } diff --git a/router_test.go b/router_test.go index 9ecb1824e..8a1a5aefa 100644 --- a/router_test.go +++ b/router_test.go @@ -2318,6 +2318,27 @@ func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) { } } +// Issue #1726 +func TestRouterDifferentParamsInPath(t *testing.T) { + e := New() + r := e.router + r.Add(http.MethodPut, "/users/:vid/files/:gid", func(Context) error { + return nil + }) + r.Add(http.MethodGet, "/users/:uid/files/:fid", func(Context) error { + return nil + }) + c := e.NewContext(nil, nil).(*context) + + r.Find(http.MethodGet, "/users/1/files/2", c) + assert.Equal(t, "1", c.Param("uid")) + assert.Equal(t, "2", c.Param("fid")) + + r.Find(http.MethodPut, "/users/3/files/4", c) + assert.Equal(t, "3", c.Param("vid")) + assert.Equal(t, "4", c.Param("gid")) +} + func TestRouterHandleMethodOptions(t *testing.T) { e := New() r := e.router From 5ec1f7e02d43472f99ecd10e2b6db9ad54578ee9 Mon Sep 17 00:00:00 2001 From: ortyomka Date: Wed, 6 Jul 2022 11:43:40 +0300 Subject: [PATCH 3/5] Rename methodContext to routeMethod Add paramsCount in each node for perfomance Signed-off-by: ortyomka --- router.go | 211 +++++++++++++++++++++++++----------------------------- 1 file changed, 99 insertions(+), 112 deletions(-) diff --git a/router.go b/router.go index 34bbd83aa..252c60adf 100644 --- a/router.go +++ b/router.go @@ -13,39 +13,40 @@ type ( routes map[string]*Route echo *Echo } - methodContext struct { - ppath string - pnames []string - handler HandlerFunc - } node struct { kind kind label byte prefix string parent *node staticChildren children - methodHandler *methodHandler + methods *routeMethods paramChild *node anyChild *node + paramsCount int // isLeaf indicates that node does not have child routes isLeaf bool // isHandler indicates that node has at least one handler registered to it isHandler bool } - kind uint8 - children []*node - methodHandler struct { - connect *methodContext - delete *methodContext - get *methodContext - head *methodContext - options *methodContext - patch *methodContext - post *methodContext - propfind *methodContext - put *methodContext - trace *methodContext - report *methodContext + kind uint8 + children []*node + routeMethod struct { + ppath string + pnames []string + handler HandlerFunc + } + routeMethods struct { + connect *routeMethod + delete *routeMethod + get *routeMethod + head *routeMethod + options *routeMethod + patch *routeMethod + post *routeMethod + propfind *routeMethod + put *routeMethod + trace *routeMethod + report *routeMethod allowHeader string } ) @@ -59,7 +60,7 @@ const ( anyLabel = byte('*') ) -func (m *methodHandler) isHandler() bool { +func (m *routeMethods) isHandler() bool { return m.connect != nil || m.delete != nil || m.get != nil || @@ -73,7 +74,7 @@ func (m *methodHandler) isHandler() bool { m.report != nil } -func (m *methodHandler) updateAllowHeader() { +func (m *routeMethods) updateAllowHeader() { buf := new(bytes.Buffer) buf.WriteString(http.MethodOptions) @@ -122,7 +123,7 @@ func (m *methodHandler) updateAllowHeader() { func NewRouter(e *Echo) *Router { return &Router{ tree: &node{ - methodHandler: new(methodHandler), + methods: new(routeMethods), }, routes: map[string]*Route{}, echo: e, @@ -156,7 +157,7 @@ func (r *Router) Add(method, path string, h HandlerFunc) { } j := i + 1 - r.insert(method, path[:i], nil, staticKind, "", nil) + r.insert(method, path[:i], staticKind, routeMethod{}) for ; i < lcpIndex && path[i] != '/'; i++ { } @@ -166,23 +167,23 @@ func (r *Router) Add(method, path string, h HandlerFunc) { if i == lcpIndex { // path node is last fragment of route path. ie. `/users/:id` - r.insert(method, path[:i], h, paramKind, ppath, pnames) + r.insert(method, path[:i], paramKind, routeMethod{ppath, pnames, h}) } else { - r.insert(method, path[:i], nil, paramKind, "", nil) + r.insert(method, path[:i], paramKind, routeMethod{}) } } else if path[i] == '*' { - r.insert(method, path[:i], nil, staticKind, "", nil) + r.insert(method, path[:i], staticKind, routeMethod{}) pnames = append(pnames, "*") - r.insert(method, path[:i+1], h, anyKind, ppath, pnames) + r.insert(method, path[:i+1], anyKind, routeMethod{ppath, pnames, h}) } } - r.insert(method, path, h, staticKind, ppath, pnames) + r.insert(method, path, staticKind, routeMethod{ppath, pnames, h}) } -func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string, pnames []string) { +func (r *Router) insert(method, path string, t kind, rm routeMethod) { // Adjust max param - paramLen := len(pnames) + paramLen := len(rm.pnames) if *r.echo.maxParam < paramLen { *r.echo.maxParam = paramLen } @@ -210,13 +211,10 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string // At root node currentNode.label = search[0] currentNode.prefix = search - if h != nil { + if rm.handler != nil { currentNode.kind = t - currentNode.addHandler(method, &methodContext{ - ppath: ppath, - pnames: pnames, - handler: h, - }) + currentNode.addMethod(method, &rm) + currentNode.paramsCount = len(rm.pnames) } currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil } else if lcpLen < prefixLen { @@ -226,7 +224,8 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode.prefix[lcpLen:], currentNode, currentNode.staticChildren, - currentNode.methodHandler, + currentNode.methods, + currentNode.paramsCount, currentNode.paramChild, currentNode.anyChild, ) @@ -246,7 +245,8 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode.label = currentNode.prefix[0] currentNode.prefix = currentNode.prefix[:lcpLen] currentNode.staticChildren = nil - currentNode.methodHandler = new(methodHandler) + currentNode.methods = new(routeMethods) + currentNode.paramsCount = 0 currentNode.paramChild = nil currentNode.anyChild = nil currentNode.isLeaf = false @@ -258,19 +258,17 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string if lcpLen == searchLen { // At parent node currentNode.kind = t - currentNode.addHandler(method, &methodContext{ - ppath: ppath, - pnames: pnames, - handler: h, - }) + if rm.handler != nil { + currentNode.addMethod(method, &rm) + currentNode.paramsCount = len(rm.pnames) + } } else { // Create child node - n = newNode(t, search[lcpLen:], currentNode, nil, new(methodHandler), nil, nil) - n.addHandler(method, &methodContext{ - ppath: ppath, - pnames: pnames, - handler: h, - }) + n = newNode(t, search[lcpLen:], currentNode, nil, new(routeMethods), 0, nil, nil) + if rm.handler != nil { + n.addMethod(method, &rm) + n.paramsCount = len(rm.pnames) + } // Only Static children could reach here currentNode.addStaticChild(n) } @@ -284,12 +282,12 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string continue } // Create child node - n := newNode(t, search, currentNode, nil, new(methodHandler), nil, nil) - n.addHandler(method, &methodContext{ - ppath: ppath, - pnames: pnames, - handler: h, - }) + n := newNode(t, search, currentNode, nil, new(routeMethods), 0, nil, nil) + if rm.handler != nil { + n.addMethod(method, &rm) + n.paramsCount = len(rm.pnames) + } + switch t { case staticKind: currentNode.addStaticChild(n) @@ -301,26 +299,24 @@ func (r *Router) insert(method, path string, h HandlerFunc, t kind, ppath string currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil } else { // Node already exists - if h != nil { - currentNode.addHandler(method, &methodContext{ - ppath: ppath, - pnames: pnames, - handler: h, - }) + if rm.handler != nil { + currentNode.addMethod(method, &rm) + currentNode.paramsCount = len(rm.pnames) } } return } } -func newNode(t kind, pre string, p *node, sc children, mh *methodHandler, paramChildren, anyChildren *node) *node { +func newNode(t kind, pre string, p *node, sc children, mh *routeMethods, paramsCount int, paramChildren, anyChildren *node) *node { return &node{ kind: t, label: pre[0], prefix: pre, parent: p, staticChildren: sc, - methodHandler: mh, + methods: mh, + paramsCount: paramsCount, paramChild: paramChildren, anyChild: anyChildren, isLeaf: sc == nil && paramChildren == nil && anyChildren == nil, @@ -354,65 +350,60 @@ func (n *node) findChildWithLabel(l byte) *node { return nil } -func (n *node) addHandler(method string, h *methodContext) { - if h.handler == nil { - n.isHandler = n.methodHandler.isHandler() - return - } - +func (n *node) addMethod(method string, h *routeMethod) { switch method { case http.MethodConnect: - n.methodHandler.connect = h + n.methods.connect = h case http.MethodDelete: - n.methodHandler.delete = h + n.methods.delete = h case http.MethodGet: - n.methodHandler.get = h + n.methods.get = h case http.MethodHead: - n.methodHandler.head = h + n.methods.head = h case http.MethodOptions: - n.methodHandler.options = h + n.methods.options = h case http.MethodPatch: - n.methodHandler.patch = h + n.methods.patch = h case http.MethodPost: - n.methodHandler.post = h + n.methods.post = h case PROPFIND: - n.methodHandler.propfind = h + n.methods.propfind = h case http.MethodPut: - n.methodHandler.put = h + n.methods.put = h case http.MethodTrace: - n.methodHandler.trace = h + n.methods.trace = h case REPORT: - n.methodHandler.report = h + n.methods.report = h } - n.methodHandler.updateAllowHeader() + n.methods.updateAllowHeader() n.isHandler = true } -func (n *node) findHandler(method string) *methodContext { +func (n *node) findMethod(method string) *routeMethod { switch method { case http.MethodConnect: - return n.methodHandler.connect + return n.methods.connect case http.MethodDelete: - return n.methodHandler.delete + return n.methods.delete case http.MethodGet: - return n.methodHandler.get + return n.methods.get case http.MethodHead: - return n.methodHandler.head + return n.methods.head case http.MethodOptions: - return n.methodHandler.options + return n.methods.options case http.MethodPatch: - return n.methodHandler.patch + return n.methods.patch case http.MethodPost: - return n.methodHandler.post + return n.methods.post case PROPFIND: - return n.methodHandler.propfind + return n.methods.propfind case http.MethodPut: - return n.methodHandler.put + return n.methods.put case http.MethodTrace: - return n.methodHandler.trace + return n.methods.trace case REPORT: - return n.methodHandler.report + return n.methods.report default: return nil } @@ -443,7 +434,7 @@ func (r *Router) Find(method, path string, c Context) { var ( previousBestMatchNode *node - matchedHandler HandlerFunc + matchedRouteMethod *routeMethod // search stores the remaining path to check for match. By each iteration we move from start of path to end of the path // and search value gets shorter and shorter. search = path @@ -539,8 +530,8 @@ func (r *Router) Find(method, path string, c Context) { if previousBestMatchNode == nil { previousBestMatchNode = currentNode } - if h := currentNode.findHandler(method); h != nil { - matchedHandler = h.handler + if h := currentNode.findMethod(method); h != nil { + matchedRouteMethod = h break } } @@ -579,11 +570,8 @@ func (r *Router) Find(method, path string, c Context) { if child := currentNode.anyChild; child != nil { // If any node is found, use remaining path for paramValues currentNode = child - if m := currentNode.findHandler(method); m != nil { - paramValues[len(m.pnames)-1] = search - } else { - break - } + paramValues[currentNode.paramsCount-1] = search + // update indexes/search in case we need to backtrack when no handler match is found paramIndex++ searchIndex += +len(search) @@ -594,8 +582,8 @@ func (r *Router) Find(method, path string, c Context) { if previousBestMatchNode == nil { previousBestMatchNode = currentNode } - if h := currentNode.findHandler(method); h != nil { - matchedHandler = h.handler + if h := currentNode.findMethod(method); h != nil { + matchedRouteMethod = h break } } @@ -618,24 +606,23 @@ func (r *Router) Find(method, path string, c Context) { return // nothing matched at all } - if matchedHandler != nil { - ctx.handler = matchedHandler + if matchedRouteMethod != nil { + ctx.handler = matchedRouteMethod.handler + ctx.path = matchedRouteMethod.ppath + ctx.pnames = matchedRouteMethod.pnames } else { // use previous match as basis. although we have no matching handler we have path match. // so we can send http.StatusMethodNotAllowed (405) instead of http.StatusNotFound (404) currentNode = previousBestMatchNode + ctx.path = path ctx.handler = NotFoundHandler if currentNode.isHandler { - ctx.Set(ContextKeyHeaderAllow, currentNode.methodHandler.allowHeader) + ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader) ctx.handler = MethodNotAllowedHandler if method == http.MethodOptions { - ctx.handler = optionsMethodHandler(currentNode.methodHandler.allowHeader) + ctx.handler = optionsMethodHandler(currentNode.methods.allowHeader) } } } - if m := currentNode.findHandler(method); m != nil { - ctx.path = m.ppath - ctx.pnames = m.pnames - } } From 83a5accbb10d6de4ec2fd735c092115a1dc0c41c Mon Sep 17 00:00:00 2001 From: ortyomka Date: Fri, 8 Jul 2022 10:11:53 +0300 Subject: [PATCH 4/5] Add backtracking to nearest path Signed-off-by: ortyomka --- router.go | 25 +++++++++++++++++++------ router_test.go | 6 ++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/router.go b/router.go index 252c60adf..931b593fc 100644 --- a/router.go +++ b/router.go @@ -19,6 +19,7 @@ type ( prefix string parent *node staticChildren children + originalPath string methods *routeMethods paramChild *node anyChild *node @@ -215,6 +216,7 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { currentNode.kind = t currentNode.addMethod(method, &rm) currentNode.paramsCount = len(rm.pnames) + currentNode.originalPath = rm.ppath } currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil } else if lcpLen < prefixLen { @@ -224,6 +226,7 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { currentNode.prefix[lcpLen:], currentNode, currentNode.staticChildren, + currentNode.originalPath, currentNode.methods, currentNode.paramsCount, currentNode.paramChild, @@ -245,6 +248,7 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { currentNode.label = currentNode.prefix[0] currentNode.prefix = currentNode.prefix[:lcpLen] currentNode.staticChildren = nil + currentNode.originalPath = "" currentNode.methods = new(routeMethods) currentNode.paramsCount = 0 currentNode.paramChild = nil @@ -261,13 +265,15 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { if rm.handler != nil { currentNode.addMethod(method, &rm) currentNode.paramsCount = len(rm.pnames) + currentNode.originalPath = rm.ppath } } else { // Create child node - n = newNode(t, search[lcpLen:], currentNode, nil, new(routeMethods), 0, nil, nil) + n = newNode(t, search[lcpLen:], currentNode, nil, "", new(routeMethods), 0, nil, nil) if rm.handler != nil { n.addMethod(method, &rm) n.paramsCount = len(rm.pnames) + n.originalPath = rm.ppath } // Only Static children could reach here currentNode.addStaticChild(n) @@ -282,7 +288,7 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { continue } // Create child node - n := newNode(t, search, currentNode, nil, new(routeMethods), 0, nil, nil) + n := newNode(t, search, currentNode, nil, rm.ppath, new(routeMethods), 0, nil, nil) if rm.handler != nil { n.addMethod(method, &rm) n.paramsCount = len(rm.pnames) @@ -302,19 +308,21 @@ func (r *Router) insert(method, path string, t kind, rm routeMethod) { if rm.handler != nil { currentNode.addMethod(method, &rm) currentNode.paramsCount = len(rm.pnames) + currentNode.originalPath = rm.ppath } } return } } -func newNode(t kind, pre string, p *node, sc children, mh *routeMethods, paramsCount int, paramChildren, anyChildren *node) *node { +func newNode(t kind, pre string, p *node, sc children, originalPath string, mh *routeMethods, paramsCount int, paramChildren, anyChildren *node) *node { return &node{ kind: t, label: pre[0], prefix: pre, parent: p, staticChildren: sc, + originalPath: originalPath, methods: mh, paramsCount: paramsCount, paramChild: paramChildren, @@ -606,16 +614,19 @@ func (r *Router) Find(method, path string, c Context) { return // nothing matched at all } + var rPath string + var rPNames []string if matchedRouteMethod != nil { ctx.handler = matchedRouteMethod.handler - ctx.path = matchedRouteMethod.ppath - ctx.pnames = matchedRouteMethod.pnames + rPath = matchedRouteMethod.ppath + rPNames = matchedRouteMethod.pnames } else { // use previous match as basis. although we have no matching handler we have path match. // so we can send http.StatusMethodNotAllowed (405) instead of http.StatusNotFound (404) currentNode = previousBestMatchNode - ctx.path = path + rPath = currentNode.originalPath + rPNames = paramValues[:currentNode.paramsCount] ctx.handler = NotFoundHandler if currentNode.isHandler { ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader) @@ -625,4 +636,6 @@ func (r *Router) Find(method, path string, c Context) { } } } + ctx.path = rPath + ctx.pnames = rPNames } diff --git a/router_test.go b/router_test.go index 8a1a5aefa..8645a26c1 100644 --- a/router_test.go +++ b/router_test.go @@ -2322,6 +2322,9 @@ func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) { func TestRouterDifferentParamsInPath(t *testing.T) { e := New() r := e.router + r.Add(http.MethodPut, "/*", func(Context) error { + return nil + }) r.Add(http.MethodPut, "/users/:vid/files/:gid", func(Context) error { return nil }) @@ -2334,6 +2337,9 @@ func TestRouterDifferentParamsInPath(t *testing.T) { assert.Equal(t, "1", c.Param("uid")) assert.Equal(t, "2", c.Param("fid")) + r.Find(http.MethodGet, "/users/1/shouldBacktrackToFirstAnyRouteAnd405", c) + assert.Equal(t, "/*", c.Path()) + r.Find(http.MethodPut, "/users/3/files/4", c) assert.Equal(t, "3", c.Param("vid")) assert.Equal(t, "4", c.Param("gid")) From 627588ac19ebb9383ce0eda15b29021504f62127 Mon Sep 17 00:00:00 2001 From: ortyomka Date: Fri, 8 Jul 2022 10:19:23 +0300 Subject: [PATCH 5/5] Remove params from NotAllowed Signed-off-by: ortyomka --- router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router.go b/router.go index 931b593fc..6a8615d83 100644 --- a/router.go +++ b/router.go @@ -626,7 +626,7 @@ func (r *Router) Find(method, path string, c Context) { currentNode = previousBestMatchNode rPath = currentNode.originalPath - rPNames = paramValues[:currentNode.paramsCount] + rPNames = nil // no params here ctx.handler = NotFoundHandler if currentNode.isHandler { ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader)