From 3f3770a030965f7c8092444d70239f5c5c7001d1 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 13:27:49 -0700 Subject: [PATCH 01/10] fix(router): keep escaped colon and parameter routes reachable in v4 --- router.go | 42 +++++++++++++++++++++++++++--------------- router_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/router.go b/router.go index 912cfeac0..542ccdd70 100644 --- a/router.go +++ b/router.go @@ -66,8 +66,7 @@ const ( paramKind anyKind - paramLabel = byte(':') - anyLabel = byte('*') + anyLabel = byte('*') ) func (m *routeMethods) isHandler() bool { @@ -214,7 +213,10 @@ func (r *Router) Add(method, path string, h HandlerFunc) { func (r *Router) insert(method, path string, h HandlerFunc) { path = normalizePathSlash(path) pnames := []string{} // Param names - ppath := path // Pristine path + // Positions of parameter markers after names are removed. Literal colons + // remain ordinary path bytes, so no sentinel byte is reserved. + paramMarkers := make([]int, 0) + ppath := path // Pristine path if h == nil && r.echo.Logger != nil { // FIXME: in future we should return error @@ -231,31 +233,32 @@ func (r *Router) insert(method, path string, h HandlerFunc) { } j := i + 1 - r.insertNode(method, path[:i], staticKind, routeMethod{}) + r.insertNode(method, path[:i], staticKind, routeMethod{}, paramMarkers) for ; i < lcpIndex && path[i] != '/'; i++ { } pnames = append(pnames, path[j:i]) + paramMarkers = append(paramMarkers, j-1) path = path[:j] + path[i:] i, lcpIndex = j, len(path) if i == lcpIndex { // path node is last fragment of route path. ie. `/users/:id` - r.insertNode(method, path[:i], paramKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}) + r.insertNode(method, path[:i], paramKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) } else { - r.insertNode(method, path[:i], paramKind, routeMethod{}) + r.insertNode(method, path[:i], paramKind, routeMethod{}, paramMarkers) } } else if path[i] == '*' { - r.insertNode(method, path[:i], staticKind, routeMethod{}) + r.insertNode(method, path[:i], staticKind, routeMethod{}, paramMarkers) pnames = append(pnames, "*") - r.insertNode(method, path[:i+1], anyKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}) + r.insertNode(method, path[:i+1], anyKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) } } - r.insertNode(method, path, staticKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}) + r.insertNode(method, path, staticKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) } -func (r *Router) insertNode(method, path string, t kind, rm routeMethod) { +func (r *Router) insertNode(method, path string, t kind, rm routeMethod, paramMarkers []int) { // Adjust max param paramLen := len(rm.pnames) if *r.echo.maxParam < paramLen { @@ -267,6 +270,7 @@ func (r *Router) insertNode(method, path string, t kind, rm routeMethod) { panic("echo: invalid method") } search := path + searchOffset := 0 for { searchLen := len(search) @@ -360,8 +364,16 @@ func (r *Router) insertNode(method, path string, t kind, rm routeMethod) { } currentNode.isLeaf = currentNode.staticChildren == nil && currentNode.paramChild == nil && currentNode.anyChild == nil } else if lcpLen < searchLen { + searchOffset += lcpLen search = search[lcpLen:] - c := currentNode.findChildWithLabel(search[0]) + isParamMarker := false + for _, marker := range paramMarkers { + if marker == searchOffset { + isParamMarker = true + break + } + } + c := currentNode.findChildWithLabel(search[0], isParamMarker) if c != nil { // Go deeper currentNode = c @@ -437,13 +449,13 @@ func (n *node) findStaticChild(l byte) *node { return nil } -func (n *node) findChildWithLabel(l byte) *node { +func (n *node) findChildWithLabel(l byte, isParamMarker bool) *node { + if isParamMarker { + return n.paramChild + } if c := n.findStaticChild(l); c != nil { return c } - if l == paramLabel { - return n.paramChild - } if l == anyLabel { return n.anyChild } diff --git a/router_test.go b/router_test.go index 203d014ec..04c63d6fd 100644 --- a/router_test.go +++ b/router_test.go @@ -1449,6 +1449,47 @@ func TestRouterParamStaticConflict(t *testing.T) { } } +func TestRouterParamLiteralByteConflictServeHTTP(t *testing.T) { + tests := []struct { + name, literalRoute, literalRequest string + }{ + {"escaped colon", `/name\:verb/x`, "/name:verb/x"}, + {"encoded NUL", "/name\x00verb/x", "/name%00verb/x"}, + } + for _, tc := range tests { + for _, literalFirst := range []bool{true, false} { + name := tc.name + "/parameter-first" + routes := []string{"/name:id", tc.literalRoute} + if literalFirst { + name = tc.name + "/literal-first" + routes[0], routes[1] = routes[1], routes[0] + } + t.Run(name, func(t *testing.T) { + e := New() + for _, route := range routes { + e.GET(route, func(c Context) error { + return c.String(http.StatusOK, c.Path()) + }) + } + for _, request := range []struct{ path, want string }{ + {tc.literalRequest, tc.literalRoute}, + {"/name1", "/name:id"}, + } { + t.Run(request.path, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, request.path, nil) + if !assert.NotPanics(t, func() { e.ServeHTTP(rec, req) }) { + return + } + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, request.want, rec.Body.String()) + }) + } + }) + } + } +} + func TestRouterParam_escapeColon(t *testing.T) { // to allow Google cloud API like route paths with colon in them // i.e. https://service.name/v1/some/resource/name:customVerb <- that `:customVerb` is not path param. It is just a string From 7d355b881d1af005f7e038b9b561a6613b935d05 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 13:47:53 -0700 Subject: [PATCH 02/10] fix(router): parse inline verbs consistently in v4 --- route_path.go | 55 +++++++++++++++++++++ route_syntax_test.go | 86 ++++++++++++++++++++++++++++++++ router.go | 115 +++++++++++++++++++++++++------------------ 3 files changed, 207 insertions(+), 49 deletions(-) create mode 100644 route_path.go create mode 100644 route_syntax_test.go diff --git a/route_path.go b/route_path.go new file mode 100644 index 000000000..a01304527 --- /dev/null +++ b/route_path.go @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package echo + +import "strings" + +// routePathPart is one parsed piece of a route pattern. A backslash before a +// colon makes the colon static, including when it follows a parameter name. +type routePathPart struct { + kind kind + value string +} + +func parseRoutePath(path string) []routePathPart { + var parts []routePathPart + var literal strings.Builder + flushLiteral := func() { + if literal.Len() > 0 { + parts = append(parts, routePathPart{kind: staticKind, value: literal.String()}) + literal.Reset() + } + } + + for i := 0; i < len(path); { + switch { + case path[i] == '\\' && i+1 < len(path) && path[i+1] == ':': + literal.WriteByte(':') + i += 2 + case path[i] == ':': + flushLiteral() + start := i + 1 + i = start + for i < len(path) && path[i] != '/' { + if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + break + } + i++ + } + parts = append(parts, routePathPart{kind: paramKind, value: path[start:i]}) + case path[i] == '*': + flushLiteral() + start := i + for i < len(path) && path[i] != '/' { + i++ + } + parts = append(parts, routePathPart{kind: anyKind, value: path[start:i]}) + default: + literal.WriteByte(path[i]) + i++ + } + } + flushLiteral() + return parts +} diff --git a/route_syntax_test.go b/route_syntax_test.go new file mode 100644 index 000000000..ae91fef5f --- /dev/null +++ b/route_syntax_test.go @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package echo + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func assertInlineVerbResponse(t *testing.T, e *Echo, path, want string) { + t.Helper() + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, path, nil) + if !assert.NotPanics(t, func() { e.ServeHTTP(rec, req) }) { + return + } + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, want, rec.Body.String()) +} + +func TestRouterInlineVerbRoutes(t *testing.T) { + for _, order := range [][]string{{"cancel", "get"}, {"get", "cancel"}} { + e := New() + for _, verb := range order { + verb := verb + e.GET("/r/:name\\:"+verb, func(c Context) error { + return c.String(http.StatusOK, verb+":"+c.Param("name")) + }) + } + assertInlineVerbResponse(t, e, "/r/foo:cancel", "cancel:foo") + assertInlineVerbResponse(t, e, "/r/foo:get", "get:foo") + assertInlineVerbResponse(t, e, "/r/foo:bar:cancel", "cancel:foo:bar") + } +} + +func TestRouterInlineVerbLongestSuffix(t *testing.T) { + e := New() + e.GET(`/r/:name\:foo\:bar`, func(c Context) error { + return c.String(http.StatusOK, "long:"+c.Param("name")) + }) + e.GET(`/r/:name\:bar`, func(c Context) error { + return c.String(http.StatusOK, "short:"+c.Param("name")) + }) + assertInlineVerbResponse(t, e, "/r/a:foo:bar", "long:a") + assertInlineVerbResponse(t, e, "/r/a:bar", "short:a") +} + +func TestRouterInlineVerbWithFollowingParam(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel/:action`, func(c Context) error { + return c.String(http.StatusOK, c.Param("name")+":"+c.Param("action")) + }) + assertInlineVerbResponse(t, e, "/r/foo:cancel/bar", "foo:bar") +} + +func TestRouterInlineVerbWithWildcard(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel/*`, func(c Context) error { + return c.String(http.StatusOK, c.Param("name")+":"+c.Param("*")) + }) + assertInlineVerbResponse(t, e, "/r/foo:cancel/bar", "foo:bar") + assertInlineVerbResponse(t, e, "/r/foo:cancel/", "foo:") +} + +func TestRouterInlineVerbAndGenericParam(t *testing.T) { + e := New() + e.GET("/r/:name", func(c Context) error { + return c.String(http.StatusOK, "generic:"+c.Param("name")) + }) + e.GET(`/r/:name\:cancel`, func(c Context) error { + return c.String(http.StatusOK, "cancel:"+c.Param("name")) + }) + assertInlineVerbResponse(t, e, "/r/foo:cancel", "cancel:foo") + assertInlineVerbResponse(t, e, "/r/foo:other", "generic:foo:other") +} + +func TestRouterReverseInlineVerb(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { return nil }).Name = "inline-verb" + assert.Equal(t, "/r/foo:cancel", e.Reverse("inline-verb", "foo")) + assert.Equal(t, "/r/:name:cancel", e.Reverse("inline-verb")) +} diff --git a/router.go b/router.go index 542ccdd70..08c9576f0 100644 --- a/router.go +++ b/router.go @@ -7,6 +7,7 @@ import ( "bytes" "fmt" "net/http" + "strings" ) // Router is the registry of all registered routes for an `Echo` instance for @@ -157,24 +158,20 @@ func (r *Router) Routes() []*Route { // Reverse generates a URL from route name and provided parameters. func (r *Router) Reverse(name string, params ...interface{}) string { uri := new(bytes.Buffer) - ln := len(params) - n := 0 for _, route := range r.routes { if route.Name == name { - for i, l := 0, len(route.Path); i < l; i++ { - hasBackslash := route.Path[i] == '\\' - if hasBackslash && i+1 < l && route.Path[i+1] == ':' { - i++ // backslash before colon escapes that colon. in that case skip backslash - } - if n < ln && (route.Path[i] == '*' || (!hasBackslash && route.Path[i] == ':')) { - // in case of `*` wildcard or `:` (unescaped colon) param we replace everything till next slash or end of path - for ; i < l && route.Path[i] != '/'; i++ { - } + n := 0 + for _, part := range parseRoutePath(route.Path) { + if part.kind == staticKind { + uri.WriteString(part.value) + } else if n < len(params) { uri.WriteString(fmt.Sprintf("%v", params[n])) n++ - } - if i < l { - uri.WriteByte(route.Path[i]) + } else if part.kind == paramKind { + uri.WriteByte(':') + uri.WriteString(part.value) + } else { + uri.WriteString(part.value) } } break @@ -212,50 +209,46 @@ func (r *Router) Add(method, path string, h HandlerFunc) { func (r *Router) insert(method, path string, h HandlerFunc) { path = normalizePathSlash(path) - pnames := []string{} // Param names - // Positions of parameter markers after names are removed. Literal colons - // remain ordinary path bytes, so no sentinel byte is reserved. - paramMarkers := make([]int, 0) - ppath := path // Pristine path - if h == nil && r.echo.Logger != nil { // FIXME: in future we should return error r.echo.Logger.Errorf("Adding route without handler function: %v:%v", method, path) } - - for i, lcpIndex := 0, len(path); i < lcpIndex; i++ { - if path[i] == ':' { - if i > 0 && path[i-1] == '\\' { - path = path[:i-1] + path[i:] - i-- - lcpIndex-- - continue - } - j := i + 1 - - r.insertNode(method, path[:i], staticKind, routeMethod{}, paramMarkers) - for ; i < lcpIndex && path[i] != '/'; i++ { + parts := parseRoutePath(path) + var pnames []string + for _, part := range parts { + if part.kind == paramKind { + pnames = append(pnames, part.value) + } else if part.kind == anyKind { + pnames = append(pnames, "*") + break + } + } + rm := routeMethod{ppath: path, pnames: pnames, handler: h} + var treePath string + var paramMarkers []int + for i, part := range parts { + switch part.kind { + case staticKind: + treePath += part.value + if i == len(parts)-1 { + r.insertNode(method, treePath, staticKind, rm, paramMarkers) } - - pnames = append(pnames, path[j:i]) - paramMarkers = append(paramMarkers, j-1) - path = path[:j] + path[i:] - i, lcpIndex = j, len(path) - - if i == lcpIndex { - // path node is last fragment of route path. ie. `/users/:id` - r.insertNode(method, path[:i], paramKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) + case paramKind: + r.insertNode(method, treePath, staticKind, routeMethod{}, paramMarkers) + paramMarkers = append(paramMarkers, len(treePath)) + treePath += ":" + if i == len(parts)-1 { + r.insertNode(method, treePath, paramKind, rm, paramMarkers) } else { - r.insertNode(method, path[:i], paramKind, routeMethod{}, paramMarkers) + r.insertNode(method, treePath, paramKind, routeMethod{}, paramMarkers) } - } else if path[i] == '*' { - r.insertNode(method, path[:i], staticKind, routeMethod{}, paramMarkers) - pnames = append(pnames, "*") - r.insertNode(method, path[:i+1], anyKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) + case anyKind: + r.insertNode(method, treePath, staticKind, routeMethod{}, paramMarkers) + treePath += "*" + r.insertNode(method, treePath, anyKind, rm, paramMarkers) + return } } - - r.insertNode(method, path, staticKind, routeMethod{ppath: ppath, pnames: pnames, handler: h}, paramMarkers) } func (r *Router) insertNode(method, path string, t kind, rm routeMethod, paramMarkers []int) { @@ -462,6 +455,22 @@ func (n *node) findChildWithLabel(l byte, isParamMarker bool) *node { return nil } +// canMatchStaticSuffix checks the static path following an inline parameter +// delimiter. Ordinary parameter routes keep their slash-based fast path. +func (n *node) canMatchStaticSuffix(path string) bool { + if !strings.HasPrefix(path, n.prefix) { + return false + } + path = path[len(n.prefix):] + if path == "" { + return n.isHandler || n.notFoundHandler != nil || n.anyChild != nil + } + if child := n.findStaticChild(path[0]); child != nil && child.canMatchStaticSuffix(path) { + return true + } + return n.paramChild != nil || n.anyChild != nil +} + func (n *node) addMethod(method string, h *routeMethod) { switch method { case http.MethodConnect: @@ -686,6 +695,14 @@ func (r *Router) Find(method, path string, c Context) { } else { for ; i < l && search[i] != '/'; i++ { } + if suffix := currentNode.findStaticChild(':'); suffix != nil { + for split := 0; split < i; split++ { + if search[split] == ':' && suffix.canMatchStaticSuffix(search[split:]) { + i = split + break + } + } + } } paramValues[paramIndex] = search[:i] From 358c2e8c9efd52ed18e0a7fc91332e2f3fcb9b05 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 14:41:21 -0700 Subject: [PATCH 03/10] fix(router): backtrack inline verb splits in v4 --- route_path.go | 60 ++++++++++++------- route_syntax_test.go | 61 +++++++++++++++++++ router.go | 137 +++++++++++++++++++++++++++---------------- 3 files changed, 188 insertions(+), 70 deletions(-) diff --git a/route_path.go b/route_path.go index a01304527..2ac197963 100644 --- a/route_path.go +++ b/route_path.go @@ -14,21 +14,18 @@ type routePathPart struct { func parseRoutePath(path string) []routePathPart { var parts []routePathPart - var literal strings.Builder - flushLiteral := func() { - if literal.Len() > 0 { - parts = append(parts, routePathPart{kind: staticKind, value: literal.String()}) - literal.Reset() - } - } + walkRoutePath(path, func(part routePathPart) { parts = append(parts, part) }) + return parts +} +// walkRoutePath is the common syntax scanner. Reverse uses it directly to +// avoid allocating a parts slice for each URL it builds. +func walkRoutePath(path string, emit func(routePathPart)) { for i := 0; i < len(path); { - switch { - case path[i] == '\\' && i+1 < len(path) && path[i+1] == ':': - literal.WriteByte(':') + if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + emit(routePathPart{kind: staticKind, value: ":"}) i += 2 - case path[i] == ':': - flushLiteral() + } else if path[i] == ':' { start := i + 1 i = start for i < len(path) && path[i] != '/' { @@ -37,19 +34,40 @@ func parseRoutePath(path string) []routePathPart { } i++ } - parts = append(parts, routePathPart{kind: paramKind, value: path[start:i]}) - case path[i] == '*': - flushLiteral() + emit(routePathPart{kind: paramKind, value: path[start:i]}) + } else if path[i] == '*' { start := i for i < len(path) && path[i] != '/' { i++ } - parts = append(parts, routePathPart{kind: anyKind, value: path[start:i]}) - default: - literal.WriteByte(path[i]) - i++ + emit(routePathPart{kind: anyKind, value: path[start:i]}) + } else { + start := i + for i < len(path) && path[i] != ':' && path[i] != '*' { + if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + break + } + i++ + } + emit(routePathPart{kind: staticKind, value: path[start:i]}) } } - flushLiteral() - return parts +} + +func routeTreePath(parts []routePathPart) (string, []int) { + var path strings.Builder + var paramMarkers []int + for _, part := range parts { + switch part.kind { + case staticKind: + path.WriteString(part.value) + case paramKind: + paramMarkers = append(paramMarkers, path.Len()) + path.WriteByte(':') + case anyKind: + path.WriteByte(anyLabel) + return path.String(), paramMarkers + } + } + return path.String(), paramMarkers } diff --git a/route_syntax_test.go b/route_syntax_test.go index ae91fef5f..d2aa1f5ac 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -84,3 +84,64 @@ func TestRouterReverseInlineVerb(t *testing.T) { assert.Equal(t, "/r/foo:cancel", e.Reverse("inline-verb", "foo")) assert.Equal(t, "/r/:name:cancel", e.Reverse("inline-verb")) } + +func TestRouterInlineVerbBacktracksToGenericRoute(t *testing.T) { + e := New() + e.GET(`/r/:name\:v:id/end`, func(c Context) error { return c.String(http.StatusOK, "verb") }) + e.GET(`/r/:name/other`, func(c Context) error { return c.String(http.StatusOK, c.Param("name")) }) + assertInlineVerbResponse(t, e, "/r/a:vq/other", "a:vq") +} + +func TestRouterInlineVerbMethodFallback(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { return c.String(http.StatusOK, "verb") }) + e.POST(`/r/:name`, func(c Context) error { return c.String(http.StatusOK, c.Param("name")) }) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/r/foo:cancel", nil)) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "foo:cancel", rec.Body.String()) +} + +func TestRouterInlineVerbRequiresNonemptyParameter(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { return c.String(http.StatusOK, c.Param("name")) }) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/:cancel", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) +} + +func TestRouterInlineVerbKeepsStaticSiblingPriority(t *testing.T) { + e := New() + e.GET(`/r/:name\:x:id`, func(c Context) error { return c.String(http.StatusOK, "verb") }) + e.GET(`/r/:name/q`, func(c Context) error { return c.String(http.StatusOK, "static:"+c.Param("name")) }) + assertInlineVerbResponse(t, e, "/r/a:x/q", "static:a:x") +} + +func TestRouterStaticParamNamesRemainEmptySlice(t *testing.T) { + e := New() + e.GET("/static", func(c Context) error { + assert.NotNil(t, c.ParamNames()) + assert.Empty(t, c.ParamNames()) + return c.NoContent(http.StatusOK) + }) + assertInlineVerbResponse(t, e, "/static", "") +} + +func TestRouterInlineVerbEncodedColonUsesGenericRoute(t *testing.T) { + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { + return c.String(http.StatusOK, "verb") + }) + e.GET(`/r/:name`, func(c Context) error { + return c.String(http.StatusOK, "generic:"+c.Param("name")) + }) + assertInlineVerbResponse(t, e, "/r/foo%3Acancel", "generic:foo%3Acancel") +} + +func TestRouterInlineVerbMethodNotAllowedWithoutFallback(t *testing.T) { + e := New() + e.POST(`/r/:name\:cancel`, func(c Context) error { return c.NoContent(http.StatusOK) }) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/foo:cancel", nil)) + assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) +} diff --git a/router.go b/router.go index 08c9576f0..596f34e61 100644 --- a/router.go +++ b/router.go @@ -7,7 +7,7 @@ import ( "bytes" "fmt" "net/http" - "strings" + "slices" ) // Router is the registry of all registered routes for an `Echo` instance for @@ -34,7 +34,8 @@ type node struct { // 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 + isHandler bool + hasColonChild bool } type kind uint8 @@ -161,11 +162,11 @@ func (r *Router) Reverse(name string, params ...interface{}) string { for _, route := range r.routes { if route.Name == name { n := 0 - for _, part := range parseRoutePath(route.Path) { + walkRoutePath(route.Path, func(part routePathPart) { if part.kind == staticKind { uri.WriteString(part.value) } else if n < len(params) { - uri.WriteString(fmt.Sprintf("%v", params[n])) + fmt.Fprint(uri, params[n]) n++ } else if part.kind == paramKind { uri.WriteByte(':') @@ -173,7 +174,7 @@ func (r *Router) Reverse(name string, params ...interface{}) string { } else { uri.WriteString(part.value) } - } + }) break } } @@ -214,7 +215,7 @@ func (r *Router) insert(method, path string, h HandlerFunc) { r.echo.Logger.Errorf("Adding route without handler function: %v:%v", method, path) } parts := parseRoutePath(path) - var pnames []string + pnames := []string{} for _, part := range parts { if part.kind == paramKind { pnames = append(pnames, part.value) @@ -224,28 +225,27 @@ func (r *Router) insert(method, path string, h HandlerFunc) { } } rm := routeMethod{ppath: path, pnames: pnames, handler: h} - var treePath string - var paramMarkers []int + treePath, paramMarkers := routeTreePath(parts) + pathEnd := 0 for i, part := range parts { switch part.kind { case staticKind: - treePath += part.value + pathEnd += len(part.value) if i == len(parts)-1 { - r.insertNode(method, treePath, staticKind, rm, paramMarkers) + r.insertNode(method, treePath[:pathEnd], staticKind, rm, paramMarkers) } case paramKind: - r.insertNode(method, treePath, staticKind, routeMethod{}, paramMarkers) - paramMarkers = append(paramMarkers, len(treePath)) - treePath += ":" + r.insertNode(method, treePath[:pathEnd], staticKind, routeMethod{}, paramMarkers) + pathEnd++ if i == len(parts)-1 { - r.insertNode(method, treePath, paramKind, rm, paramMarkers) + r.insertNode(method, treePath[:pathEnd], paramKind, rm, paramMarkers) } else { - r.insertNode(method, treePath, paramKind, routeMethod{}, paramMarkers) + r.insertNode(method, treePath[:pathEnd], paramKind, routeMethod{}, paramMarkers) } case anyKind: - r.insertNode(method, treePath, staticKind, routeMethod{}, paramMarkers) - treePath += "*" - r.insertNode(method, treePath, anyKind, rm, paramMarkers) + r.insertNode(method, treePath[:pathEnd], staticKind, routeMethod{}, paramMarkers) + pathEnd++ + r.insertNode(method, treePath[:pathEnd], anyKind, rm, paramMarkers) return } } @@ -324,6 +324,7 @@ func (r *Router) insertNode(method, path string, t kind, rm routeMethod, paramMa currentNode.label = currentNode.prefix[0] currentNode.prefix = currentNode.prefix[:lcpLen] currentNode.staticChildren = nil + currentNode.hasColonChild = false currentNode.originalPath = "" currentNode.methods = new(routeMethods) currentNode.paramsCount = 0 @@ -359,13 +360,7 @@ func (r *Router) insertNode(method, path string, t kind, rm routeMethod, paramMa } else if lcpLen < searchLen { searchOffset += lcpLen search = search[lcpLen:] - isParamMarker := false - for _, marker := range paramMarkers { - if marker == searchOffset { - isParamMarker = true - break - } - } + isParamMarker := slices.Contains(paramMarkers, searchOffset) c := currentNode.findChildWithLabel(search[0], isParamMarker) if c != nil { // Go deeper @@ -412,7 +407,7 @@ func newNode( anyChildren *node, notFoundHandler *routeMethod, ) *node { - return &node{ + n := &node{ kind: t, label: pre[0], prefix: pre, @@ -427,10 +422,20 @@ func newNode( isHandler: methods.isHandler(), notFoundHandler: notFoundHandler, } + for _, child := range sc { + if child.label == ':' { + n.hasColonChild = true + break + } + } + return n } func (n *node) addStaticChild(c *node) { n.staticChildren = append(n.staticChildren, c) + if c.label == ':' { + n.hasColonChild = true + } } func (n *node) findStaticChild(l byte) *node { @@ -455,22 +460,6 @@ func (n *node) findChildWithLabel(l byte, isParamMarker bool) *node { return nil } -// canMatchStaticSuffix checks the static path following an inline parameter -// delimiter. Ordinary parameter routes keep their slash-based fast path. -func (n *node) canMatchStaticSuffix(path string) bool { - if !strings.HasPrefix(path, n.prefix) { - return false - } - path = path[len(n.prefix):] - if path == "" { - return n.isHandler || n.notFoundHandler != nil || n.anyChild != nil - } - if child := n.findStaticChild(path[0]); child != nil && child.canMatchStaticSuffix(path) { - return true - } - return n.paramChild != nil || n.anyChild != nil -} - func (n *node) addMethod(method string, h *routeMethod) { switch method { case http.MethodConnect: @@ -611,6 +600,12 @@ func (r *Router) Find(method, path string, c Context) { return } + var splitPlan []int + var splitOptions []int + var splitUsed bool + var fallbackNode *node + var fallbackValues []string + // Router tree is implemented by longest common prefix array (LCP array) https://en.wikipedia.org/wiki/LCP_array // Tree search is implemented as for loop where one loop iteration is divided into 3 separate blocks // Each of these blocks checks specific kind of node (static/param/any). Order of blocks reflex their priority in routing. @@ -618,6 +613,9 @@ func (r *Router) Find(method, path string, c Context) { // // Note: backtracking in tree is implemented by replacing/switching currentNode to previous node // and hoping to (goto statement) next block by priority to check if it is the match. +searchRoute: + splitOptions = splitOptions[:0] + splitUsed = false for { prefixLen := 0 // Prefix length lcpLen := 0 // LCP (longest common prefix) length @@ -639,7 +637,7 @@ func (r *Router) Find(method, path string, c Context) { // No matching prefix, let's backtrack to the first possible alternative node of the decision path nk, ok := backtrackToNextNodeKind(staticKind) if !ok { - return // No other possibilities on the decision path, handler will be whatever context is reset to. + break // No other possibilities on the decision path. } else if nk == paramKind { goto Param // NOTE: this case (backtracking from static node to previous any node) can not happen by current any matching logic. Any node is end of search currently @@ -688,18 +686,37 @@ func (r *Router) Find(method, path string, c Context) { currentNode = child i := 0 l := len(search) - if currentNode.isLeaf { + if currentNode.isLeaf && !splitUsed { // when param node does not have any children (path param is last piece of route path) then param node should // act similarly to any node - consider all remaining search as match i = l } else { for ; i < l && search[i] != '/'; i++ { } - if suffix := currentNode.findStaticChild(':'); suffix != nil { - for split := 0; split < i; split++ { - if search[split] == ':' && suffix.canMatchStaticSuffix(search[split:]) { - i = split - break + // Try each literal-colon split, then the whole segment if the + // split route cannot handle the request. + if currentNode.hasColonChild { + choice := 0 + if len(splitPlan) > len(splitOptions) { + choice = splitPlan[len(splitOptions)] + } + count, chosen := 0, -1 + for split := 1; split < i; split++ { + if search[split] == ':' { + if count == choice { + chosen = split + } + count++ + } + } + if count > 0 { + if len(splitPlan) == len(splitOptions) { + splitPlan = append(splitPlan, 0) + } + splitOptions = append(splitOptions, count+1) + if chosen >= 0 { + i = chosen + splitUsed = true } } } @@ -751,6 +768,28 @@ func (r *Router) Find(method, path string, c Context) { break } } + if matchedRouteMethod == nil && len(splitOptions) > 0 { + if fallbackNode == nil && previousBestMatchNode != nil { + fallbackNode = previousBestMatchNode + fallbackValues = append(fallbackValues, paramValues[:fallbackNode.paramsCount]...) + } + for i := len(splitOptions) - 1; i >= 0; i-- { + if splitPlan[i]+1 < splitOptions[i] { + splitPlan[i]++ + splitPlan = splitPlan[:i+1] + currentNode = r.tree + previousBestMatchNode = nil + search, searchIndex, paramIndex = path, 0, 0 + clear(paramValues) + goto searchRoute + } + } + } + if matchedRouteMethod == nil && fallbackNode != nil { + currentNode = fallbackNode + previousBestMatchNode = fallbackNode + copy(paramValues, fallbackValues) + } if currentNode == nil && previousBestMatchNode == nil { return // nothing matched at all From 9e3f23dd0a161018c9b1f142322975640222c9de Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 14:48:47 -0700 Subject: [PATCH 04/10] perf(router): preserve v4 fast path without inline verbs --- route_path.go | 9 ++ router.go | 20 ++--- router_plain.go | 235 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 252 insertions(+), 12 deletions(-) create mode 100644 router_plain.go diff --git a/route_path.go b/route_path.go index 2ac197963..eef732115 100644 --- a/route_path.go +++ b/route_path.go @@ -12,6 +12,15 @@ type routePathPart struct { value string } +func hasInlineVerbPart(parts []routePathPart) bool { + for i := 1; i < len(parts); i++ { + if parts[i-1].kind == paramKind && parts[i].kind == staticKind && parts[i].value == ":" { + return true + } + } + return false +} + func parseRoutePath(path string) []routePathPart { var parts []routePathPart walkRoutePath(path, func(part routePathPart) { parts = append(parts, part) }) diff --git a/router.go b/router.go index 596f34e61..ee6df3f3f 100644 --- a/router.go +++ b/router.go @@ -13,9 +13,10 @@ import ( // Router is the registry of all registered routes for an `Echo` instance for // request matching and URL path parameter parsing. type Router struct { - tree *node - routes map[string]*Route - echo *Echo + tree *node + routes map[string]*Route + echo *Echo + hasInlineVerb bool } type node struct { @@ -215,6 +216,7 @@ func (r *Router) insert(method, path string, h HandlerFunc) { r.echo.Logger.Errorf("Adding route without handler function: %v:%v", method, path) } parts := parseRoutePath(path) + r.hasInlineVerb = r.hasInlineVerb || hasInlineVerbPart(parts) pnames := []string{} for _, part := range parts { if part.kind == paramKind { @@ -541,15 +543,9 @@ func optionsMethodHandler(allowMethods string) func(c Context) error { } } -// Find lookup a handler registered for method and path. It also parses URL for path -// parameters and load them into context. -// -// For performance: -// -// - Get context from `Echo#AcquireContext()` -// - Reset it `Context#Reset()` -// - Return it `Echo#ReleaseContext()`. -func (r *Router) Find(method, path string, c Context) { +// findInline handles requests that may need to retry a literal-colon split. +// The ordinary Find path stays separate for routers without inline verbs. +func (r *Router) findInline(method, path string, c Context) { ctx := c.(*context) currentNode := r.tree // Current node as root diff --git a/router_plain.go b/router_plain.go new file mode 100644 index 000000000..8a2d1682b --- /dev/null +++ b/router_plain.go @@ -0,0 +1,235 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package echo + +import ( + "net/http" + "strings" +) + +// Find looks up a handler and path parameters. It uses the established fast +// path unless the router has an inline verb and the request contains a colon. +func (r *Router) Find(method, path string, c Context) { + if r.hasInlineVerb && strings.IndexByte(path, ':') >= 0 { + r.findInline(method, path, c) + return + } + ctx := c.(*context) + currentNode := r.tree // Current node as root + + var ( + previousBestMatchNode *node + 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 + searchIndex = 0 + paramIndex int // Param counter + paramValues = ctx.pvalues // Use the internal slice so the interface can keep the illusion of a dynamic slice + ) + + // Backtracking is needed when a dead end (leaf node) is reached in the router tree. + // To backtrack the current node will be changed to the parent node and the next kind for the + // router logic will be returned based on fromKind or kind of the dead end node (static > param > any). + // For example if there is no static node match we should check parent next sibling by kind (param). + // Backtracking itself does not check if there is a next sibling, this is done by the router logic. + backtrackToNextNodeKind := func(fromKind kind) (nextNodeKind kind, valid bool) { + previous := currentNode + currentNode = previous.parent + valid = currentNode != nil + + // Next node type by priority + if previous.kind == anyKind { + nextNodeKind = staticKind + } else { + nextNodeKind = previous.kind + 1 + } + + if fromKind == staticKind { + // when backtracking is done from static kind block we did not change search so nothing to restore + return + } + + // restore search to value it was before we move to current node we are backtracking from. + if previous.kind == staticKind { + searchIndex -= len(previous.prefix) + } else { + paramIndex-- + // for param/any node.prefix value is always `:` so we can not deduce searchIndex from that and must use pValue + // for that index as it would also contain part of path we cut off before moving into node we are backtracking from + searchIndex -= len(paramValues[paramIndex]) + paramValues[paramIndex] = "" + } + search = path[searchIndex:] + return + } + + // Router tree is implemented by longest common prefix array (LCP array) https://en.wikipedia.org/wiki/LCP_array + // Tree search is implemented as for loop where one loop iteration is divided into 3 separate blocks + // Each of these blocks checks specific kind of node (static/param/any). Order of blocks reflex their priority in routing. + // Search order/priority is: static > param > any. + // + // Note: backtracking in tree is implemented by replacing/switching currentNode to previous node + // and hoping to (goto statement) next block by priority to check if it is the match. + for { + prefixLen := 0 // Prefix length + lcpLen := 0 // LCP (longest common prefix) length + + if currentNode.kind == staticKind { + searchLen := len(search) + prefixLen = len(currentNode.prefix) + + // LCP - Longest Common Prefix (https://en.wikipedia.org/wiki/LCP_array) + max := prefixLen + if searchLen < max { + max = searchLen + } + for ; lcpLen < max && search[lcpLen] == currentNode.prefix[lcpLen]; lcpLen++ { + } + } + + if lcpLen != prefixLen { + // No matching prefix, let's backtrack to the first possible alternative node of the decision path + nk, ok := backtrackToNextNodeKind(staticKind) + if !ok { + return // No other possibilities on the decision path, handler will be whatever context is reset to. + } else if nk == paramKind { + goto Param + // NOTE: this case (backtracking from static node to previous any node) can not happen by current any matching logic. Any node is end of search currently + //} else if nk == anyKind { + // goto Any + } else { + // Not found (this should never be possible for static node we are looking currently) + break + } + } + + // The full prefix has matched, remove the prefix from the remaining search + search = search[lcpLen:] + searchIndex = searchIndex + lcpLen + + // Finish routing if is no request path remaining to search + if search == "" { + // in case of node that is handler we have exact method type match or something for 405 to use + if currentNode.isHandler { + // check if current node has handler registered for http method we are looking for. we store currentNode as + // best matching in case we do no find no more routes matching this path+method + if previousBestMatchNode == nil { + previousBestMatchNode = currentNode + } + if h := currentNode.findMethod(method); h != nil { + matchedRouteMethod = h + break + } + } else if currentNode.notFoundHandler != nil { + matchedRouteMethod = currentNode.notFoundHandler + break + } + } + + // Static node + if search != "" { + if child := currentNode.findStaticChild(search[0]); child != nil { + currentNode = child + continue + } + } + + Param: + // Param node + if child := currentNode.paramChild; search != "" && child != nil { + currentNode = child + i := 0 + l := len(search) + if currentNode.isLeaf { + // when param node does not have any children (path param is last piece of route path) then param node should + // act similarly to any node - consider all remaining search as match + i = l + } else { + for ; i < l && search[i] != '/'; i++ { + } + } + + paramValues[paramIndex] = search[:i] + paramIndex++ + search = search[i:] + searchIndex = searchIndex + i + continue + } + + Any: + // Any node + if child := currentNode.anyChild; child != nil { + // If any node is found, use remaining path for paramValues + currentNode = child + paramValues[currentNode.paramsCount-1] = search + + // update indexes/search in case we need to backtrack when no handler match is found + paramIndex++ + searchIndex += len(search) + search = "" + + if h := currentNode.findMethod(method); h != nil { + matchedRouteMethod = h + break + } + // we store currentNode as best matching in case we do not find more routes matching this path+method. Needed for 405 + if previousBestMatchNode == nil { + previousBestMatchNode = currentNode + } + if currentNode.notFoundHandler != nil { + matchedRouteMethod = currentNode.notFoundHandler + break + } + } + + // Let's backtrack to the first possible alternative node of the decision path + nk, ok := backtrackToNextNodeKind(anyKind) + if !ok { + break // No other possibilities on the decision path + } else if nk == paramKind { + goto Param + } else if nk == anyKind { + goto Any + } else { + // Not found + break + } + } + + if currentNode == nil && previousBestMatchNode == nil { + return // nothing matched at all + } + + // matchedHandler could be method+path handler that we matched or notFoundHandler from node with matching path + // user provided not found (404) handler has priority over generic method not found (405) handler or global 404 handler + var rPath string + var rPNames []string + if matchedRouteMethod != nil { + rPath = matchedRouteMethod.ppath + rPNames = matchedRouteMethod.pnames + ctx.handler = matchedRouteMethod.handler + } 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 + + rPath = currentNode.originalPath + rPNames = nil // no params here + ctx.handler = NotFoundHandler + if currentNode.notFoundHandler != nil { + rPath = currentNode.notFoundHandler.ppath + rPNames = currentNode.notFoundHandler.pnames + ctx.handler = currentNode.notFoundHandler.handler + } else if currentNode.isHandler { + ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader) + ctx.handler = MethodNotAllowedHandler + if method == http.MethodOptions { + ctx.handler = optionsMethodHandler(currentNode.methods.allowHeader) + } + } + } + ctx.path = rPath + ctx.pnames = rPNames +} From b6f374666ed5fe4284ed3fad7175002a32e580a5 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 15:52:25 -0700 Subject: [PATCH 05/10] fix(router): retry inline verb splits on the param node in v4 Backport of the v5 change. Replace the whole-search retry plan with a backtrack point on the param node: when a candidate split fails, the same param node is retried with the next literal-colon split and finally the whole path segment before routing backtracks to its parent. Static > param > any priority of ancestors, the leaf rule of later params, group middleware and catch-all routes are no longer affected by a failed split. An escaped colon after a parameter starts an inline verb only when the rest of that path segment is static (`/:name\:cancel`, optionally followed by `/...`). Split candidates are then scanned once per segment, so a request with many colons is routed in linear time. Other escaped colons after a parameter keep their older meaning as part of the parameter name. The duplicated fast-path Find and the router-level inline verb flag are removed. The param scan uses strings.IndexByte, which keeps router benchmarks within about 0.3% of v4 (geomean). --- route_path.go | 32 ++++-- route_syntax_test.go | 69 ++++++++++++- router.go | 122 +++++++++------------- router_plain.go | 235 ------------------------------------------- 4 files changed, 136 insertions(+), 322 deletions(-) delete mode 100644 router_plain.go diff --git a/route_path.go b/route_path.go index eef732115..3f3575e70 100644 --- a/route_path.go +++ b/route_path.go @@ -6,21 +6,13 @@ package echo import "strings" // routePathPart is one parsed piece of a route pattern. A backslash before a -// colon makes the colon static, including when it follows a parameter name. +// colon makes the colon static. After a parameter name it starts an inline verb +// (`/:name\:cancel`) when the rest of that path segment is static. type routePathPart struct { kind kind value string } -func hasInlineVerbPart(parts []routePathPart) bool { - for i := 1; i < len(parts); i++ { - if parts[i-1].kind == paramKind && parts[i].kind == staticKind && parts[i].value == ":" { - return true - } - } - return false -} - func parseRoutePath(path string) []routePathPart { var parts []routePathPart walkRoutePath(path, func(part routePathPart) { parts = append(parts, part) }) @@ -38,7 +30,7 @@ func walkRoutePath(path string, emit func(routePathPart)) { start := i + 1 i = start for i < len(path) && path[i] != '/' { - if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' && isInlineVerb(path[i+2:]) { break } i++ @@ -63,6 +55,24 @@ func walkRoutePath(path string, emit func(routePathPart)) { } } +// isInlineVerb reports whether the route text after an escaped colon stays +// static up to the end of its path segment. Only then can the router find where +// the parameter value ends by trying the colons in the requested segment. Other +// escaped colons keep the older meaning and remain part of the parameter name. +func isInlineVerb(rest string) bool { + for i := 0; i < len(rest) && rest[i] != '/'; i++ { + switch rest[i] { + case '*': + return false + case ':': + if i == 0 || rest[i-1] != '\\' { + return false + } + } + } + return true +} + func routeTreePath(parts []routePathPart) (string, []int) { var path strings.Builder var paramMarkers []int diff --git a/route_syntax_test.go b/route_syntax_test.go index d2aa1f5ac..e54968817 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -6,6 +6,7 @@ package echo import ( "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -87,9 +88,11 @@ func TestRouterReverseInlineVerb(t *testing.T) { func TestRouterInlineVerbBacktracksToGenericRoute(t *testing.T) { e := New() - e.GET(`/r/:name\:v:id/end`, func(c Context) error { return c.String(http.StatusOK, "verb") }) + e.GET(`/r/:name\:v/:id/end`, func(c Context) error { return c.String(http.StatusOK, "verb") }) e.GET(`/r/:name/other`, func(c Context) error { return c.String(http.StatusOK, c.Param("name")) }) assertInlineVerbResponse(t, e, "/r/a:vq/other", "a:vq") + assertInlineVerbResponse(t, e, "/r/a:v/other", "a:v") + assertInlineVerbResponse(t, e, "/r/a:v/q/end", "verb") } func TestRouterInlineVerbMethodFallback(t *testing.T) { @@ -110,11 +113,69 @@ func TestRouterInlineVerbRequiresNonemptyParameter(t *testing.T) { assert.Equal(t, http.StatusNotFound, rec.Code) } -func TestRouterInlineVerbKeepsStaticSiblingPriority(t *testing.T) { +func TestRouterInlineVerbAndStaticSibling(t *testing.T) { e := New() - e.GET(`/r/:name\:x:id`, func(c Context) error { return c.String(http.StatusOK, "verb") }) + e.GET(`/r/:name\:x/:id`, func(c Context) error { return c.String(http.StatusOK, "verb:"+c.Param("name")+":"+c.Param("id")) }) e.GET(`/r/:name/q`, func(c Context) error { return c.String(http.StatusOK, "static:"+c.Param("name")) }) - assertInlineVerbResponse(t, e, "/r/a:x/q", "static:a:x") + assertInlineVerbResponse(t, e, "/r/a:x/q", "verb:a:q") + assertInlineVerbResponse(t, e, "/r/a:y/q", "static:a:y") +} + +func TestRouterInlineVerbMustEndPathSegment(t *testing.T) { + // An escaped colon that is followed by a param or wildcard in the same segment keeps its older meaning: it is part + // of the param name. Trying every colon in a request segment for such routes could not be bounded. + e := New() + e.GET(`/r/:name\:x:id`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }) + e.GET(`/s/:name\:x*`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }) + assertInlineVerbResponse(t, e, "/r/foo", `name\:x:id`) + assertInlineVerbResponse(t, e, "/s/foo", `name\:x*`) +} + +func TestRouterInlineVerbLeafParamAfterVerb(t *testing.T) { + e := New() + e.GET(`/r/:name\:x/:rest`, func(c Context) error { return c.String(http.StatusOK, c.Param("name")+"|"+c.Param("rest")) }) + assertInlineVerbResponse(t, e, "/r/a:x/b/c", "a|b/c") +} + +func TestRouterInlineVerbWithGroupMiddlewareAndCatchAll(t *testing.T) { + e := New() + g := e.Group("/r", func(next HandlerFunc) HandlerFunc { return next }) + g.GET("/:name", func(c Context) error { return c.String(http.StatusOK, "generic:"+c.Param("name")) }) + g.GET(`/:name\:cancel`, func(c Context) error { return c.String(http.StatusOK, "cancel:"+c.Param("name")) }) + assertInlineVerbResponse(t, e, "/r/foo:other", "generic:foo:other") + assertInlineVerbResponse(t, e, "/r/foo:cancel", "cancel:foo") + + e = New() + e.GET("/r/:name", func(c Context) error { return c.String(http.StatusOK, "generic:"+c.Param("name")) }) + e.GET(`/r/:name\:cancel`, func(c Context) error { return c.String(http.StatusOK, "cancel:"+c.Param("name")) }) + e.GET("/*", func(c Context) error { return c.String(http.StatusOK, "any") }) + assertInlineVerbResponse(t, e, "/r/foo:other", "generic:foo:other") + assertInlineVerbResponse(t, e, "/r/foo:cancel", "cancel:foo") +} + +func TestRouterInlineVerbManyColons(t *testing.T) { + // Every colon in the segment is a possible split. Each is tried at most once, so a long run of colons is routed + // in linear time. + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { return c.String(http.StatusOK, "cancel:"+c.Param("name")) }) + e.GET(`/r/:name\:c`, func(c Context) error { return c.String(http.StatusOK, "c:"+c.Param("name")) }) + e.GET(`/r/:name\:x/:a\:y/z`, func(c Context) error { return c.String(http.StatusOK, "nested") }) + colons := strings.Repeat(":", 1<<16) + + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/a"+colons+"b", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) + + rec = httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/a"+colons+"x/b"+colons+"y/nope", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) + + // every ":c" enters the shared ":c" verb node before failing, so each split is retried + rec = httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/a"+strings.Repeat(":c", 1<<15)+"b", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) + + assertInlineVerbResponse(t, e, "/r/a"+colons+"cancel", "cancel:a"+colons[1:]) } func TestRouterStaticParamNamesRemainEmptySlice(t *testing.T) { diff --git a/router.go b/router.go index ee6df3f3f..8634df34d 100644 --- a/router.go +++ b/router.go @@ -8,15 +8,15 @@ import ( "fmt" "net/http" "slices" + "strings" ) // Router is the registry of all registered routes for an `Echo` instance for // request matching and URL path parameter parsing. type Router struct { - tree *node - routes map[string]*Route - echo *Echo - hasInlineVerb bool + tree *node + routes map[string]*Route + echo *Echo } type node struct { @@ -216,7 +216,6 @@ func (r *Router) insert(method, path string, h HandlerFunc) { r.echo.Logger.Errorf("Adding route without handler function: %v:%v", method, path) } parts := parseRoutePath(path) - r.hasInlineVerb = r.hasInlineVerb || hasInlineVerbPart(parts) pnames := []string{} for _, part := range parts { if part.kind == paramKind { @@ -425,7 +424,7 @@ func newNode( notFoundHandler: notFoundHandler, } for _, child := range sc { - if child.label == ':' { + if t == paramKind && child.label == ':' { n.hasColonChild = true break } @@ -435,11 +434,29 @@ func newNode( func (n *node) addStaticChild(c *node) { n.staticChildren = append(n.staticChildren, c) - if c.label == ':' { + if n.kind == paramKind && c.label == ':' { n.hasColonChild = true } } +// inlineVerbSplit returns where a param value in search ends: at the first literal colon at or after from where this +// node's inline verb child could match, otherwise at the end of the path segment. A split value is never empty. The +// scan stops at the next slash, so trying every split of a segment in turn is linear in its length. +func (n *node) inlineVerbSplit(search string, from int) int { + verbs := n.findStaticChild(':') + for i := from; i < len(search); i++ { + switch search[i] { + case '/': + return i + case ':': + if i > 0 && verbs != nil && strings.HasPrefix(search[i:], verbs.prefix) { + return i + } + } + } + return len(search) +} + func (n *node) findStaticChild(l byte) *node { for _, c := range n.staticChildren { if c.label == l { @@ -543,9 +560,15 @@ func optionsMethodHandler(allowMethods string) func(c Context) error { } } -// findInline handles requests that may need to retry a literal-colon split. -// The ordinary Find path stays separate for routers without inline verbs. -func (r *Router) findInline(method, path string, c Context) { +// Find lookup a handler registered for method and path. It also parses URL for path +// parameters and load them into context. +// +// For performance: +// +// - Get context from `Echo#AcquireContext()` +// - Reset it `Context#Reset()` +// - Return it `Echo#ReleaseContext()`. +func (r *Router) Find(method, path string, c Context) { ctx := c.(*context) currentNode := r.tree // Current node as root @@ -596,12 +619,6 @@ func (r *Router) findInline(method, path string, c Context) { return } - var splitPlan []int - var splitOptions []int - var splitUsed bool - var fallbackNode *node - var fallbackValues []string - // Router tree is implemented by longest common prefix array (LCP array) https://en.wikipedia.org/wiki/LCP_array // Tree search is implemented as for loop where one loop iteration is divided into 3 separate blocks // Each of these blocks checks specific kind of node (static/param/any). Order of blocks reflex their priority in routing. @@ -609,9 +626,6 @@ func (r *Router) findInline(method, path string, c Context) { // // Note: backtracking in tree is implemented by replacing/switching currentNode to previous node // and hoping to (goto statement) next block by priority to check if it is the match. -searchRoute: - splitOptions = splitOptions[:0] - splitUsed = false for { prefixLen := 0 // Prefix length lcpLen := 0 // LCP (longest common prefix) length @@ -682,40 +696,16 @@ searchRoute: currentNode = child i := 0 l := len(search) - if currentNode.isLeaf && !splitUsed { + if currentNode.isLeaf { // when param node does not have any children (path param is last piece of route path) then param node should // act similarly to any node - consider all remaining search as match i = l - } else { - for ; i < l && search[i] != '/'; i++ { - } - // Try each literal-colon split, then the whole segment if the - // split route cannot handle the request. - if currentNode.hasColonChild { - choice := 0 - if len(splitPlan) > len(splitOptions) { - choice = splitPlan[len(splitOptions)] - } - count, chosen := 0, -1 - for split := 1; split < i; split++ { - if search[split] == ':' { - if count == choice { - chosen = split - } - count++ - } - } - if count > 0 { - if len(splitPlan) == len(splitOptions) { - splitPlan = append(splitPlan, 0) - } - splitOptions = append(splitOptions, count+1) - if chosen >= 0 { - i = chosen - splitUsed = true - } - } - } + } else if currentNode.hasColonChild { + // an inline verb (`/:name\:verb`) can end the param value at a literal colon. Start with the first + // possible split, the param node is retried with the next one before backtracking (see below). + i = currentNode.inlineVerbSplit(search, 0) + } else if i = strings.IndexByte(search, '/'); i < 0 { + i = l } paramValues[paramIndex] = search[:i] @@ -751,6 +741,16 @@ searchRoute: } } + // A param value that ended at an inline verb split is a decision point of the param node itself. Retry the + // node with the next split, and finally with the whole path segment, before backtracking to its parent. + if currentNode.hasColonChild && search != "" && search[0] == ':' { + start := searchIndex - len(paramValues[paramIndex-1]) + searchIndex = start + currentNode.inlineVerbSplit(path[start:], len(paramValues[paramIndex-1])+1) + paramValues[paramIndex-1] = path[start:searchIndex] + search = path[searchIndex:] + continue + } + // Let's backtrack to the first possible alternative node of the decision path nk, ok := backtrackToNextNodeKind(anyKind) if !ok { @@ -764,28 +764,6 @@ searchRoute: break } } - if matchedRouteMethod == nil && len(splitOptions) > 0 { - if fallbackNode == nil && previousBestMatchNode != nil { - fallbackNode = previousBestMatchNode - fallbackValues = append(fallbackValues, paramValues[:fallbackNode.paramsCount]...) - } - for i := len(splitOptions) - 1; i >= 0; i-- { - if splitPlan[i]+1 < splitOptions[i] { - splitPlan[i]++ - splitPlan = splitPlan[:i+1] - currentNode = r.tree - previousBestMatchNode = nil - search, searchIndex, paramIndex = path, 0, 0 - clear(paramValues) - goto searchRoute - } - } - } - if matchedRouteMethod == nil && fallbackNode != nil { - currentNode = fallbackNode - previousBestMatchNode = fallbackNode - copy(paramValues, fallbackValues) - } if currentNode == nil && previousBestMatchNode == nil { return // nothing matched at all diff --git a/router_plain.go b/router_plain.go deleted file mode 100644 index 8a2d1682b..000000000 --- a/router_plain.go +++ /dev/null @@ -1,235 +0,0 @@ -// SPDX-License-Identifier: MIT -// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors - -package echo - -import ( - "net/http" - "strings" -) - -// Find looks up a handler and path parameters. It uses the established fast -// path unless the router has an inline verb and the request contains a colon. -func (r *Router) Find(method, path string, c Context) { - if r.hasInlineVerb && strings.IndexByte(path, ':') >= 0 { - r.findInline(method, path, c) - return - } - ctx := c.(*context) - currentNode := r.tree // Current node as root - - var ( - previousBestMatchNode *node - 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 - searchIndex = 0 - paramIndex int // Param counter - paramValues = ctx.pvalues // Use the internal slice so the interface can keep the illusion of a dynamic slice - ) - - // Backtracking is needed when a dead end (leaf node) is reached in the router tree. - // To backtrack the current node will be changed to the parent node and the next kind for the - // router logic will be returned based on fromKind or kind of the dead end node (static > param > any). - // For example if there is no static node match we should check parent next sibling by kind (param). - // Backtracking itself does not check if there is a next sibling, this is done by the router logic. - backtrackToNextNodeKind := func(fromKind kind) (nextNodeKind kind, valid bool) { - previous := currentNode - currentNode = previous.parent - valid = currentNode != nil - - // Next node type by priority - if previous.kind == anyKind { - nextNodeKind = staticKind - } else { - nextNodeKind = previous.kind + 1 - } - - if fromKind == staticKind { - // when backtracking is done from static kind block we did not change search so nothing to restore - return - } - - // restore search to value it was before we move to current node we are backtracking from. - if previous.kind == staticKind { - searchIndex -= len(previous.prefix) - } else { - paramIndex-- - // for param/any node.prefix value is always `:` so we can not deduce searchIndex from that and must use pValue - // for that index as it would also contain part of path we cut off before moving into node we are backtracking from - searchIndex -= len(paramValues[paramIndex]) - paramValues[paramIndex] = "" - } - search = path[searchIndex:] - return - } - - // Router tree is implemented by longest common prefix array (LCP array) https://en.wikipedia.org/wiki/LCP_array - // Tree search is implemented as for loop where one loop iteration is divided into 3 separate blocks - // Each of these blocks checks specific kind of node (static/param/any). Order of blocks reflex their priority in routing. - // Search order/priority is: static > param > any. - // - // Note: backtracking in tree is implemented by replacing/switching currentNode to previous node - // and hoping to (goto statement) next block by priority to check if it is the match. - for { - prefixLen := 0 // Prefix length - lcpLen := 0 // LCP (longest common prefix) length - - if currentNode.kind == staticKind { - searchLen := len(search) - prefixLen = len(currentNode.prefix) - - // LCP - Longest Common Prefix (https://en.wikipedia.org/wiki/LCP_array) - max := prefixLen - if searchLen < max { - max = searchLen - } - for ; lcpLen < max && search[lcpLen] == currentNode.prefix[lcpLen]; lcpLen++ { - } - } - - if lcpLen != prefixLen { - // No matching prefix, let's backtrack to the first possible alternative node of the decision path - nk, ok := backtrackToNextNodeKind(staticKind) - if !ok { - return // No other possibilities on the decision path, handler will be whatever context is reset to. - } else if nk == paramKind { - goto Param - // NOTE: this case (backtracking from static node to previous any node) can not happen by current any matching logic. Any node is end of search currently - //} else if nk == anyKind { - // goto Any - } else { - // Not found (this should never be possible for static node we are looking currently) - break - } - } - - // The full prefix has matched, remove the prefix from the remaining search - search = search[lcpLen:] - searchIndex = searchIndex + lcpLen - - // Finish routing if is no request path remaining to search - if search == "" { - // in case of node that is handler we have exact method type match or something for 405 to use - if currentNode.isHandler { - // check if current node has handler registered for http method we are looking for. we store currentNode as - // best matching in case we do no find no more routes matching this path+method - if previousBestMatchNode == nil { - previousBestMatchNode = currentNode - } - if h := currentNode.findMethod(method); h != nil { - matchedRouteMethod = h - break - } - } else if currentNode.notFoundHandler != nil { - matchedRouteMethod = currentNode.notFoundHandler - break - } - } - - // Static node - if search != "" { - if child := currentNode.findStaticChild(search[0]); child != nil { - currentNode = child - continue - } - } - - Param: - // Param node - if child := currentNode.paramChild; search != "" && child != nil { - currentNode = child - i := 0 - l := len(search) - if currentNode.isLeaf { - // when param node does not have any children (path param is last piece of route path) then param node should - // act similarly to any node - consider all remaining search as match - i = l - } else { - for ; i < l && search[i] != '/'; i++ { - } - } - - paramValues[paramIndex] = search[:i] - paramIndex++ - search = search[i:] - searchIndex = searchIndex + i - continue - } - - Any: - // Any node - if child := currentNode.anyChild; child != nil { - // If any node is found, use remaining path for paramValues - currentNode = child - paramValues[currentNode.paramsCount-1] = search - - // update indexes/search in case we need to backtrack when no handler match is found - paramIndex++ - searchIndex += len(search) - search = "" - - if h := currentNode.findMethod(method); h != nil { - matchedRouteMethod = h - break - } - // we store currentNode as best matching in case we do not find more routes matching this path+method. Needed for 405 - if previousBestMatchNode == nil { - previousBestMatchNode = currentNode - } - if currentNode.notFoundHandler != nil { - matchedRouteMethod = currentNode.notFoundHandler - break - } - } - - // Let's backtrack to the first possible alternative node of the decision path - nk, ok := backtrackToNextNodeKind(anyKind) - if !ok { - break // No other possibilities on the decision path - } else if nk == paramKind { - goto Param - } else if nk == anyKind { - goto Any - } else { - // Not found - break - } - } - - if currentNode == nil && previousBestMatchNode == nil { - return // nothing matched at all - } - - // matchedHandler could be method+path handler that we matched or notFoundHandler from node with matching path - // user provided not found (404) handler has priority over generic method not found (405) handler or global 404 handler - var rPath string - var rPNames []string - if matchedRouteMethod != nil { - rPath = matchedRouteMethod.ppath - rPNames = matchedRouteMethod.pnames - ctx.handler = matchedRouteMethod.handler - } 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 - - rPath = currentNode.originalPath - rPNames = nil // no params here - ctx.handler = NotFoundHandler - if currentNode.notFoundHandler != nil { - rPath = currentNode.notFoundHandler.ppath - rPNames = currentNode.notFoundHandler.pnames - ctx.handler = currentNode.notFoundHandler.handler - } else if currentNode.isHandler { - ctx.Set(ContextKeyHeaderAllow, currentNode.methods.allowHeader) - ctx.handler = MethodNotAllowedHandler - if method == http.MethodOptions { - ctx.handler = optionsMethodHandler(currentNode.methods.allowHeader) - } - } - } - ctx.path = rPath - ctx.pnames = rPNames -} From 89c1ce19eabf95fa476cb43ce5bca687a9309b14 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 16:20:31 -0700 Subject: [PATCH 06/10] fix(router): retry inline verb splits after a wildcard fails A wildcard ends the route search, but a param value above it that ended at an inline verb split is now retried with the next split and the whole segment, so a verb route with a wildcard cannot shadow a generic route for other methods. The retry now runs only when routing backtracks from the inline verb child into its param node, instead of on every dead end. An escaped colon after a parameter keeps its older meaning (part of the parameter name) unless the first one in the segment starts an inline verb, so a later `\:` cannot turn such a legacy route into a verb route. Reverse writes placeholders for such names without the backslash, as before. Dead hasColonChild bookkeeping for split nodes is removed. --- route_path.go | 9 ++++++- route_syntax_test.go | 60 +++++++++++++++++++++++++++++++++++++++++ router.go | 63 +++++++++++++++++++++++++++++--------------- 3 files changed, 110 insertions(+), 22 deletions(-) diff --git a/route_path.go b/route_path.go index 3f3575e70..2e347f017 100644 --- a/route_path.go +++ b/route_path.go @@ -30,7 +30,14 @@ func walkRoutePath(path string, emit func(routePathPart)) { start := i + 1 i = start for i < len(path) && path[i] != '/' { - if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' && isInlineVerb(path[i+2:]) { + if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + if isInlineVerb(path[i+2:]) { + break + } + // not an inline verb: the rest of the segment is the param name, as before inline verbs + for i < len(path) && path[i] != '/' { + i++ + } break } i++ diff --git a/route_syntax_test.go b/route_syntax_test.go index e54968817..ce3a5fc23 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -129,6 +130,19 @@ func TestRouterInlineVerbMustEndPathSegment(t *testing.T) { e.GET(`/s/:name\:x*`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }) assertInlineVerbResponse(t, e, "/r/foo", `name\:x:id`) assertInlineVerbResponse(t, e, "/s/foo", `name\:x*`) + // the first escaped colon decides, so a later one that is followed only by static text does not start a verb + e.GET(`/t/:a\:x:y\:z`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }).Name = "legacy" + assertInlineVerbResponse(t, e, "/t/q:z", `a\:x:y\:z`) + assert.Equal(t, "/t/:a:x:y:z", e.Reverse("legacy")) +} + +func TestRouterInlineVerbBeforeWholeSegment(t *testing.T) { + // a matching inline verb split is tried before the whole segment, also when a wildcard follows the verb + e := New() + e.GET(`/r/:name\:x/*`, func(c Context) error { return c.String(http.StatusOK, "verb:"+c.Param("name")+"|"+c.Param("*")) }) + e.GET(`/r/:id/info`, func(c Context) error { return c.String(http.StatusOK, "info:"+c.Param("id")) }) + assertInlineVerbResponse(t, e, "/r/a:x/info", "verb:a|info") + assertInlineVerbResponse(t, e, "/r/a:y/info", "info:a:y") } func TestRouterInlineVerbLeafParamAfterVerb(t *testing.T) { @@ -161,6 +175,11 @@ func TestRouterInlineVerbManyColons(t *testing.T) { e.GET(`/r/:name\:c`, func(c Context) error { return c.String(http.StatusOK, "c:"+c.Param("name")) }) e.GET(`/r/:name\:x/:a\:y/z`, func(c Context) error { return c.String(http.StatusOK, "nested") }) colons := strings.Repeat(":", 1<<16) + start := time.Now() + defer func() { + // linear routing takes milliseconds here; trying splits quadratically would take minutes + assert.Less(t, time.Since(start), 10*time.Second) + }() rec := httptest.NewRecorder() e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/a"+colons+"b", nil)) @@ -206,3 +225,44 @@ func TestRouterInlineVerbMethodNotAllowedWithoutFallback(t *testing.T) { e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/foo:cancel", nil)) assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) } + +func TestRouterInlineVerbRetriedAfterWildcard(t *testing.T) { + // a wildcard ends the search, but the split above it is still retried with the next split and the whole segment + e := New() + e.POST(`/r/:n\:v/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:n/*`, func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("n")+"|"+c.Param("*")) }) + assertInlineVerbResponse(t, e, "/r/a:v/q", "get:a:v|q") + + e = New() + e.POST(`/r/:n\:a\:b/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:n\:b/x`, func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("n")) }) + assertInlineVerbResponse(t, e, "/r/q:a:b/x", "get:q:a") + + e = New() + e.POST(`/r/:n\:v/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/a:v/q", nil)) + assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) +} + +func TestRouterInlineVerbChangesEscapedColonAfterParam(t *testing.T) { + // Before inline verbs, `/:name\:cancel` was a single param named `name\:cancel` that matched any segment. + e := New() + e.GET(`/r/:name\:cancel`, func(c Context) error { + return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")+"="+c.Param("name")) + }) + assertInlineVerbResponse(t, e, "/r/foo:cancel", "name=foo") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/r/foo", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) +} + +func TestRouterReverseEscapedColonPlaceholder(t *testing.T) { + e := New() + e.GET(`/r/:n\:x:id`, func(c Context) error { return nil }).Name = "legacy" + e.GET(`/r/:name\:cancel`, func(c Context) error { return nil }).Name = "verb" + assert.Equal(t, "/r/:n:x:id", e.Reverse("legacy")) + assert.Equal(t, "/r/foo", e.Reverse("legacy", "foo")) + assert.Equal(t, "/r/:name:cancel", e.Reverse("verb")) + assert.Equal(t, "/r/foo:cancel", e.Reverse("verb", "foo")) +} diff --git a/router.go b/router.go index 8634df34d..c3085977c 100644 --- a/router.go +++ b/router.go @@ -169,11 +169,12 @@ func (r *Router) Reverse(name string, params ...interface{}) string { } else if n < len(params) { fmt.Fprint(uri, params[n]) n++ - } else if part.kind == paramKind { - uri.WriteByte(':') - uri.WriteString(part.value) } else { - uri.WriteString(part.value) + // placeholder for a missing value. An escaped colon in a param name is written without its backslash. + if part.kind == paramKind { + uri.WriteByte(':') + } + uri.WriteString(strings.ReplaceAll(part.value, `\:`, ":")) } }) break @@ -325,7 +326,6 @@ func (r *Router) insertNode(method, path string, t kind, rm routeMethod, paramMa currentNode.label = currentNode.prefix[0] currentNode.prefix = currentNode.prefix[:lcpLen] currentNode.staticChildren = nil - currentNode.hasColonChild = false currentNode.originalPath = "" currentNode.methods = new(routeMethods) currentNode.paramsCount = 0 @@ -423,22 +423,34 @@ func newNode( isHandler: methods.isHandler(), notFoundHandler: notFoundHandler, } - for _, child := range sc { - if t == paramKind && child.label == ':' { - n.hasColonChild = true - break - } - } return n } func (n *node) addStaticChild(c *node) { n.staticChildren = append(n.staticChildren, c) + // param nodes are never split (their prefix is a single byte), so this is where their inline verb child is set if n.kind == paramKind && c.label == ':' { n.hasColonChild = true } } +// pendingInlineVerbSplit returns the nearest param node, from n up to the root, whose value in paramValues ended at an +// inline verb split and so can still be retried. searchIndex and paramIndex are the routing state at n. +func pendingInlineVerbSplit(n *node, path string, searchIndex, paramIndex int, paramValues []string) *node { + for ; n != nil; n = n.parent { + if n.hasColonChild && searchIndex < len(path) && path[searchIndex] == ':' { + return n + } + if n.kind == staticKind { + searchIndex -= len(n.prefix) + } else { + paramIndex-- + searchIndex -= len(paramValues[paramIndex]) + } + } + return nil +} + // inlineVerbSplit returns where a param value in search ends: at the first literal colon at or after from where this // node's inline verb child could match, otherwise at the end of the path segment. A split value is never empty. The // scan stops at the next slash, so trying every split of a segment in turn is linear in its length. @@ -741,28 +753,37 @@ func (r *Router) Find(method, path string, c Context) { } } - // A param value that ended at an inline verb split is a decision point of the param node itself. Retry the - // node with the next split, and finally with the whole path segment, before backtracking to its parent. - if currentNode.hasColonChild && search != "" && search[0] == ':' { - start := searchIndex - len(paramValues[paramIndex-1]) - searchIndex = start + currentNode.inlineVerbSplit(path[start:], len(paramValues[paramIndex-1])+1) - paramValues[paramIndex-1] = path[start:searchIndex] - search = path[searchIndex:] - continue - } - // Let's backtrack to the first possible alternative node of the decision path nk, ok := backtrackToNextNodeKind(anyKind) if !ok { break // No other possibilities on the decision path } else if nk == paramKind { + if currentNode.hasColonChild && search != "" && search[0] == ':' { + goto InlineVerbSplit + } goto Param } else if nk == anyKind { goto Any + } else if n := pendingInlineVerbSplit(currentNode, path, searchIndex, paramIndex, paramValues); n != nil { + // A wildcard ends the search, but a param value above it that ended at an inline verb split is still retried. + for currentNode != n { + backtrackToNextNodeKind(anyKind) + } + goto InlineVerbSplit } else { // Not found break } + + InlineVerbSplit: + // A param value that ended at an inline verb split is a decision point of the param node itself. When its + // inline verb child fails, retry the node with the next split, and finally with the whole path segment, + // before backtracking to its parent. + start := searchIndex - len(paramValues[paramIndex-1]) + searchIndex = start + currentNode.inlineVerbSplit(path[start:], len(paramValues[paramIndex-1])+1) + paramValues[paramIndex-1] = path[start:searchIndex] + search = path[searchIndex:] + continue } if currentNode == nil && previousBestMatchNode == nil { From 0641c51a00f8c5c704562e61844939375d3b4199 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 16:34:36 -0700 Subject: [PATCH 07/10] fix(router): keep backtracking below an inline verb split When a wildcard fails below a param value that ended at an inline verb split, keep backtracking one node at a time instead of jumping to that param node, so the other routes below the split are tried before the next split. Without a pending split a failed wildcard still ends the search as before, and the check does not change the routing state. Adds tests for nested splits, routes below a split and a RouteNotFound wildcard below a split. --- route_syntax_test.go | 30 ++++++++++++++++++++++++++++++ router.go | 30 +++++++++++++++++------------- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/route_syntax_test.go b/route_syntax_test.go index ce3a5fc23..131b3f1a7 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -266,3 +266,33 @@ func TestRouterReverseEscapedColonPlaceholder(t *testing.T) { assert.Equal(t, "/r/:name:cancel", e.Reverse("verb")) assert.Equal(t, "/r/foo:cancel", e.Reverse("verb", "foo")) } + +func TestRouterInlineVerbWildcardBacktracksBelowSplit(t *testing.T) { + // after a wildcard below a split fails, the other routes below that split are tried before the next split + e := New() + e.POST(`/r/:n\:v/a/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:n\:v/:p/b`, func(c Context) error { return c.String(http.StatusOK, "verb:"+c.Param("n")+"|"+c.Param("p")) }) + e.GET(`/r/:n/a/b`, func(c Context) error { return c.String(http.StatusOK, "generic:"+c.Param("n")) }) + assertInlineVerbResponse(t, e, "/r/q:v/a/b", "verb:q|a") + + // nested splits: the nearest pending split is retried first, then the outer one + e = New() + e.POST(`/r/:a\:x/:b\:y/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:a/:b\:y/*`, func(c Context) error { + return c.String(http.StatusOK, c.Param("a")+"|"+c.Param("b")+"|"+c.Param("*")) + }) + assertInlineVerbResponse(t, e, "/r/p:x/q:y/z", "p:x|q|z") + + e = New() + e.POST(`/r/:a\:x/:b\:y/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:a\:x/:b/*`, func(c Context) error { + return c.String(http.StatusOK, c.Param("a")+"|"+c.Param("b")+"|"+c.Param("*")) + }) + assertInlineVerbResponse(t, e, "/r/p:x/q:y/z", "p|q:y|z") + + // a RouteNotFound wildcard below a split handles the request like any other RouteNotFound route + e = New() + e.RouteNotFound(`/r/:a\:x/*`, func(c Context) error { return c.String(http.StatusOK, "not found:"+c.Param("a")) }) + e.GET(`/r/:a/k`, func(c Context) error { return c.String(http.StatusOK, "k") }) + assertInlineVerbResponse(t, e, "/r/p:x/k", "not found:p") +} diff --git a/router.go b/router.go index c3085977c..fd7b51b14 100644 --- a/router.go +++ b/router.go @@ -408,7 +408,7 @@ func newNode( anyChildren *node, notFoundHandler *routeMethod, ) *node { - n := &node{ + return &node{ kind: t, label: pre[0], prefix: pre, @@ -423,7 +423,6 @@ func newNode( isHandler: methods.isHandler(), notFoundHandler: notFoundHandler, } - return n } func (n *node) addStaticChild(c *node) { @@ -434,12 +433,13 @@ func (n *node) addStaticChild(c *node) { } } -// pendingInlineVerbSplit returns the nearest param node, from n up to the root, whose value in paramValues ended at an -// inline verb split and so can still be retried. searchIndex and paramIndex are the routing state at n. -func pendingInlineVerbSplit(n *node, path string, searchIndex, paramIndex int, paramValues []string) *node { +// hasPendingInlineVerbSplit reports whether a param node from n up to the root has a value in paramValues that ended at +// an inline verb split and so can still be retried. searchIndex and paramIndex are the routing state at n. It does +// not change that state, so a request that ends here keeps its param values. +func hasPendingInlineVerbSplit(n *node, path string, searchIndex, paramIndex int, paramValues []string) bool { for ; n != nil; n = n.parent { if n.hasColonChild && searchIndex < len(path) && path[searchIndex] == ':' { - return n + return true } if n.kind == staticKind { searchIndex -= len(n.prefix) @@ -448,12 +448,16 @@ func pendingInlineVerbSplit(n *node, path string, searchIndex, paramIndex int, p searchIndex -= len(paramValues[paramIndex]) } } - return nil + return false } // inlineVerbSplit returns where a param value in search ends: at the first literal colon at or after from where this // node's inline verb child could match, otherwise at the end of the path segment. A split value is never empty. The // scan stops at the next slash, so trying every split of a segment in turn is linear in its length. +// +// A split is only chosen when the whole prefix of the inline verb child matches. Routing therefore never backtracks +// into the param node from a prefix mismatch of that child, and the split only needs to be retried when backtracking +// from within the child's subtree. func (n *node) inlineVerbSplit(search string, from int) int { verbs := n.findStaticChild(':') for i := from; i < len(search); i++ { @@ -755,6 +759,7 @@ func (r *Router) Find(method, path string, c Context) { // Let's backtrack to the first possible alternative node of the decision path nk, ok := backtrackToNextNodeKind(anyKind) + Backtracked: if !ok { break // No other possibilities on the decision path } else if nk == paramKind { @@ -764,12 +769,11 @@ func (r *Router) Find(method, path string, c Context) { goto Param } else if nk == anyKind { goto Any - } else if n := pendingInlineVerbSplit(currentNode, path, searchIndex, paramIndex, paramValues); n != nil { - // A wildcard ends the search, but a param value above it that ended at an inline verb split is still retried. - for currentNode != n { - backtrackToNextNodeKind(anyKind) - } - goto InlineVerbSplit + } else if hasPendingInlineVerbSplit(currentNode, path, searchIndex, paramIndex, paramValues) { + // A wildcard ends the search, except below a param value that ended at an inline verb split: keep + // backtracking, so the other routes below that split and then the next split are still tried. + nk, ok = backtrackToNextNodeKind(anyKind) + goto Backtracked } else { // Not found break From 4b22adde7878a942945ca4c33949ab5ef72ceb1a Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 16:36:15 -0700 Subject: [PATCH 08/10] fix(router): keep escaped colons after complex param names in v4 An escaped colon after a param name that contains ':' or '*' keeps its older meaning as part of the name. The escaped colon check is shared in the route syntax scanner. Adds tests for RouteNotFound on the whole segment and Reverse placeholders. --- route_path.go | 19 ++++++++++++------- route_syntax_test.go | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/route_path.go b/route_path.go index 2e347f017..7d4aacec6 100644 --- a/route_path.go +++ b/route_path.go @@ -23,15 +23,16 @@ func parseRoutePath(path string) []routePathPart { // avoid allocating a parts slice for each URL it builds. func walkRoutePath(path string, emit func(routePathPart)) { for i := 0; i < len(path); { - if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { + if isEscapedColon(path, i) { emit(routePathPart{kind: staticKind, value: ":"}) i += 2 } else if path[i] == ':' { start := i + 1 i = start + plainName := true // an escaped colon only starts an inline verb after a name without ':' or '*' for i < len(path) && path[i] != '/' { - if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { - if isInlineVerb(path[i+2:]) { + if isEscapedColon(path, i) { + if plainName && isInlineVerb(path[i+2:]) { break } // not an inline verb: the rest of the segment is the param name, as before inline verbs @@ -40,6 +41,9 @@ func walkRoutePath(path string, emit func(routePathPart)) { } break } + if path[i] == ':' || path[i] == '*' { + plainName = false + } i++ } emit(routePathPart{kind: paramKind, value: path[start:i]}) @@ -51,10 +55,7 @@ func walkRoutePath(path string, emit func(routePathPart)) { emit(routePathPart{kind: anyKind, value: path[start:i]}) } else { start := i - for i < len(path) && path[i] != ':' && path[i] != '*' { - if path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' { - break - } + for i < len(path) && path[i] != ':' && path[i] != '*' && !isEscapedColon(path, i) { i++ } emit(routePathPart{kind: staticKind, value: path[start:i]}) @@ -62,6 +63,10 @@ func walkRoutePath(path string, emit func(routePathPart)) { } } +func isEscapedColon(path string, i int) bool { + return path[i] == '\\' && i+1 < len(path) && path[i+1] == ':' +} + // isInlineVerb reports whether the route text after an escaped colon stays // static up to the end of its path segment. Only then can the router find where // the parameter value ends by trying the colons in the requested segment. Other diff --git a/route_syntax_test.go b/route_syntax_test.go index 131b3f1a7..8e90d4b13 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -28,7 +28,6 @@ func TestRouterInlineVerbRoutes(t *testing.T) { for _, order := range [][]string{{"cancel", "get"}, {"get", "cancel"}} { e := New() for _, verb := range order { - verb := verb e.GET("/r/:name\\:"+verb, func(c Context) error { return c.String(http.StatusOK, verb+":"+c.Param("name")) }) @@ -296,3 +295,17 @@ func TestRouterInlineVerbWildcardBacktracksBelowSplit(t *testing.T) { e.GET(`/r/:a/k`, func(c Context) error { return c.String(http.StatusOK, "k") }) assertInlineVerbResponse(t, e, "/r/p:x/k", "not found:p") } + +func TestRouterInlineVerbMisc(t *testing.T) { + e := New() + e.POST(`/r/:n\:v/*`, func(c Context) error { return c.String(http.StatusOK, "post") }).Name = "verb" + e.RouteNotFound(`/r/:n/*`, func(c Context) error { return c.String(http.StatusOK, "not found:"+c.Param("n")) }) + // the whole segment reaches the RouteNotFound route, as a static sibling would + assertInlineVerbResponse(t, e, "/r/a:v/q", "not found:a:v") + assert.Equal(t, "/r/:n:v/*", e.Reverse("verb")) + assert.Equal(t, "/r/a:v/b/c", e.Reverse("verb", "a", "b/c")) + + // a param name with ':' keeps an escaped colon as part of the name + e.GET(`/s/:a:b\:v`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }) + assertInlineVerbResponse(t, e, "/s/x", `a:b\:v`) +} From 63cf6590e4ac9b04757e6d561d5cb2f1fae6a4f2 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 16:47:39 -0700 Subject: [PATCH 09/10] fix(router): keep leaf params matching the rest of the path Registering an inline verb route under a param gave the param node a static child, so a sibling route ending in that param (`/files/:path`) stopped matching values across slashes. When the inline verb child is the node's only child, a param value without a split now takes the rest of the path, as a leaf param does. --- route_syntax_test.go | 24 ++++++++++++++++++++++++ router.go | 10 ++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/route_syntax_test.go b/route_syntax_test.go index 8e90d4b13..8e425f9d3 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -309,3 +309,27 @@ func TestRouterInlineVerbMisc(t *testing.T) { e.GET(`/s/:a:b\:v`, func(c Context) error { return c.String(http.StatusOK, strings.Join(c.ParamNames(), ",")) }) assertInlineVerbResponse(t, e, "/s/x", `a:b\:v`) } + +func TestRouterInlineVerbKeepsLeafParam(t *testing.T) { + // a param with only an inline verb child still takes the rest of the path when no split matches + e := New() + e.GET("/files/:path", func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("path")) }) + e.POST(`/files/:name\:upload`, func(c Context) error { return c.String(http.StatusOK, "upload:"+c.Param("name")) }) + assertInlineVerbResponse(t, e, "/files/a/b", "get:a/b") + assertInlineVerbResponse(t, e, "/files/a:upload/b", "get:a:upload/b") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/files/a:upload", nil)) + assert.Equal(t, "upload:a", rec.Body.String()) + + // with another child the param stops at the slash, as before + e.GET("/files/:path/meta", func(c Context) error { return c.String(http.StatusOK, "meta:"+c.Param("path")) }) + assertInlineVerbResponse(t, e, "/files/a/meta", "meta:a") +} + +func TestRouterInlineVerbPendingAboveParam(t *testing.T) { + // the pending split is found above a param without a split + e := New() + e.POST(`/r/:a\:v/:b/*`, func(c Context) error { return c.String(http.StatusOK, "post") }) + e.GET(`/r/:a/:b/q`, func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("a")+"|"+c.Param("b")) }) + assertInlineVerbResponse(t, e, "/r/x:v/y/q", "get:x:v|y") +} diff --git a/router.go b/router.go index fd7b51b14..91ad6ff36 100644 --- a/router.go +++ b/router.go @@ -452,8 +452,9 @@ func hasPendingInlineVerbSplit(n *node, path string, searchIndex, paramIndex int } // inlineVerbSplit returns where a param value in search ends: at the first literal colon at or after from where this -// node's inline verb child could match, otherwise at the end of the path segment. A split value is never empty. The -// scan stops at the next slash, so trying every split of a segment in turn is linear in its length. +// node's inline verb child could match, otherwise at the end of the path segment (or of the path when that child is the +// node's only child). A split value is never empty. The scan stops at the next slash, so trying every split of a +// segment in turn is linear in its length. // // A split is only chosen when the whole prefix of the inline verb child matches. Routing therefore never backtracks // into the param node from a prefix mismatch of that child, and the split only needs to be retried when backtracking @@ -463,6 +464,11 @@ func (n *node) inlineVerbSplit(search string, from int) int { for i := from; i < len(search); i++ { switch search[i] { case '/': + if len(n.staticChildren) == 1 && n.paramChild == nil && n.anyChild == nil { + // the inline verb child is the only child: without a split the param takes the rest of the path, as a + // leaf param does + return len(search) + } return i case ':': if i > 0 && verbs != nil && strings.HasPrefix(search[i:], verbs.prefix) { From 499f5639f31c8e780bb58da1785beffcaf56b3e0 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Tue, 29 Sep 2026 16:55:37 -0700 Subject: [PATCH 10/10] test(router): cover leaf param fallbacks next to an inline verb Drop the param and any child checks that cannot fail for a param node, and pin 405, RouteNotFound and wildcard fallbacks for a param value that spans slashes next to an inline verb route. --- route_syntax_test.go | 20 ++++++++++++++++++++ router.go | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/route_syntax_test.go b/route_syntax_test.go index 8e425f9d3..e53ee91bf 100644 --- a/route_syntax_test.go +++ b/route_syntax_test.go @@ -333,3 +333,23 @@ func TestRouterInlineVerbPendingAboveParam(t *testing.T) { e.GET(`/r/:a/:b/q`, func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("a")+"|"+c.Param("b")) }) assertInlineVerbResponse(t, e, "/r/x:v/y/q", "get:x:v|y") } + +func TestRouterInlineVerbKeepsLeafParamFallbacks(t *testing.T) { + e := New() + e.GET("/files/:path", func(c Context) error { return c.String(http.StatusOK, "get:"+c.Param("path")) }) + e.POST(`/files/:name\:upload`, func(c Context) error { return c.String(http.StatusOK, "upload") }) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodPut, "/files/a/b", nil)) + assert.Equal(t, http.StatusMethodNotAllowed, rec.Code) + assert.Equal(t, "OPTIONS, GET", rec.Header().Get(HeaderAllow)) + + e = New() + e.RouteNotFound("/files/:path", func(c Context) error { return c.String(http.StatusOK, "not found:"+c.Param("path")) }) + e.POST(`/files/:name\:upload`, func(c Context) error { return c.String(http.StatusOK, "upload") }) + assertInlineVerbResponse(t, e, "/files/a/b", "not found:a/b") + + e = New() + e.POST(`/files/:name\:upload`, func(c Context) error { return c.String(http.StatusOK, "upload") }) + e.GET("/files/*", func(c Context) error { return c.String(http.StatusOK, "any:"+c.Param("*")) }) + assertInlineVerbResponse(t, e, "/files/a/b", "any:a/b") +} diff --git a/router.go b/router.go index 91ad6ff36..de0824d79 100644 --- a/router.go +++ b/router.go @@ -464,9 +464,9 @@ func (n *node) inlineVerbSplit(search string, from int) int { for i := from; i < len(search); i++ { switch search[i] { case '/': - if len(n.staticChildren) == 1 && n.paramChild == nil && n.anyChild == nil { - // the inline verb child is the only child: without a split the param takes the rest of the path, as a - // leaf param does + if len(n.staticChildren) == 1 { + // the inline verb child is the only child (a param node never has a param or any child): without a + // split the param takes the rest of the path, as a leaf param does return len(search) } return i