fix(router): handle escaped colons and inline verbs in v4

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
+}

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"))
+}

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
@@ -66,8 +67,7 @@ const (
 	paramKind
 	anyKind
 
-	paramLabel = byte(':')
-	anyLabel   = byte('*')
+	anyLabel = byte('*')
 )
 
 func (m *routeMethods) isHandler() bool {
@@ -158,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
@@ -213,49 +209,49 @@ 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
-
 	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{})
-			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])
-			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})
+		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{})
+				r.insertNode(method, treePath, paramKind, routeMethod{}, paramMarkers)
 			}
-		} else if path[i] == '*' {
-			r.insertNode(method, path[:i], staticKind, routeMethod{})
-			pnames = append(pnames, "*")
-			r.insertNode(method, path[:i+1], anyKind, routeMethod{ppath: ppath, pnames: pnames, handler: h})
+		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})
 }
 
-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 +263,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 +357,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,19 +442,35 @@ 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
 	}
 	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:
@@ -674,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]