From f4b5a90ad3d3d1817d4e87d349d699d5ad272110 Mon Sep 17 00:00:00 2001
From: Roland Lammel <rl@neotel.at>
Date: Wed, 19 Feb 2020 16:10:57 +0100
Subject: [PATCH] Fix #1493 router loop for param routes (#1501)

* Add test to reproduce router loop for #1493

* Simplify and correct  router param tests

* Fix #1493 to avoid router loop for param nodes
---
 router.go      | 77 ++++++++++++++++++++++++++++----------------------
 router_test.go | 17 +++++++++++
 2 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/router.go b/router.go
index 9db9ea92..cb2cc16c 100644
--- a/router.go
+++ b/router.go
@@ -376,8 +376,8 @@ func (r *Router) Find(method, path string, c Context) {
 			continue
 		}
 
-		// Param node
 	Param:
+		// Param node
 		if child = cn.findChildByKind(pkind); child != nil {
 			// Issue #378
 			if len(pvalues) == n {
@@ -401,47 +401,58 @@ func (r *Router) Find(method, path string, c Context) {
 			continue
 		}
 
-		// Any node
 	Any:
-		if cn = cn.findChildByKind(akind); cn == nil {
-			if nn != nil {
-				// No next node to go down in routing (issue #954)
-				// Find nearest "any" route going up the routing tree
-				search = ns
-				np := nn.parent
-				// Consider param route one level up only
-				// if no slash is remaining in search string
-				if cn = nn.findChildByKind(pkind); cn != nil && strings.IndexByte(ns, '/') == -1 {
+		// Any node
+		if cn = cn.findChildByKind(akind); cn != nil {
+			// If any node is found, use remaining path for pvalues
+			pvalues[len(cn.pnames)-1] = search
+			break
+		}
+
+		// No node found, continue at stored next node
+		// or find nearest "any" route
+		if nn != nil {
+			// No next node to go down in routing (issue #954)
+			// Find nearest "any" route going up the routing tree
+			search = ns
+			np := nn.parent
+			// Consider param route one level up only
+			if cn = nn.findChildByKind(pkind); cn != nil {
+				pos := strings.IndexByte(ns, '/')
+				if pos == -1 {
+					// If no slash is remaining in search string set param value
 					pvalues[len(cn.pnames)-1] = search
 					break
-				} else if cn != nil && strings.IndexByte(ns, '/') != 1 {
-					// If slash is present, it means that this is a parameterized route.
-					cn = cn.parent
+				} else if pos > 0 {
+					// Otherwise continue route processing with restored next node
+					cn = nn
+					nn = nil
+					ns = ""
 					goto Param
 				}
-				for {
-					np = nn.parent
-					if cn = nn.findChildByKind(akind); cn != nil {
-						break
-					}
-					if np == nil {
-						break // no further parent nodes in tree, abort
-					}
-					var str strings.Builder
-					str.WriteString(nn.prefix)
-					str.WriteString(search)
-					search = str.String()
-					nn = np
-				}
-				if cn != nil { // use the found "any" route and update path
-					pvalues[len(cn.pnames)-1] = search
+			}
+			// No param route found, try to resolve nearest any route
+			for {
+				np = nn.parent
+				if cn = nn.findChildByKind(akind); cn != nil {
 					break
 				}
+				if np == nil {
+					break // no further parent nodes in tree, abort
+				}
+				var str strings.Builder
+				str.WriteString(nn.prefix)
+				str.WriteString(search)
+				search = str.String()
+				nn = np
+			}
+			if cn != nil { // use the found "any" route and update path
+				pvalues[len(cn.pnames)-1] = search
+				break
 			}
-			return // Not found
 		}
-		pvalues[len(cn.pnames)-1] = search
-		break
+		return // Not found
+
 	}
 
 	ctx.handler = cn.findHandler(method)
diff --git a/router_test.go b/router_test.go
index f47225d2..92db9d9d 100644
--- a/router_test.go
+++ b/router_test.go
@@ -1031,12 +1031,15 @@ func TestRouterParamBacktraceNotFound(t *testing.T) {
 	r.Find(http.MethodGet, "/a", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/foo", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/bar", c)
 	assert.Equal(t, "a", c.Param("param1"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/a/bar/b", c)
 	assert.Equal(t, "a", c.Param("param1"))
 	assert.Equal(t, "b", c.Param("param2"))
@@ -1157,31 +1160,45 @@ func TestRouterParam1466(t *testing.T) {
 	r.Find(http.MethodGet, "/users/ajitem", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/signup", c)
 	assert.Equal(t, "", c.Param("username"))
 	// Additional assertions for #1479
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/likes/projects/ids", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/likes/projects/ids", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/profile", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/profile", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/sharewithme/uploads/self", c)
 	assert.Equal(t, "sharewithme", c.Param("username"))
 	assert.Equal(t, "self", c.Param("type"))
 
+	c = e.NewContext(nil, nil).(*context)
 	r.Find(http.MethodGet, "/users/ajitem/uploads/self", c)
 	assert.Equal(t, "ajitem", c.Param("username"))
 	assert.Equal(t, "self", c.Param("type"))
+
+	// Issue #1493 - check for routing loop
+	c = e.NewContext(nil, nil).(*context)
+	r.Find(http.MethodGet, "/users/tree/free", c)
+	assert.Equal(t, "", c.Param("id"))
+	assert.Equal(t, 0, c.response.Status)
 }
 
 func benchmarkRouterRoutes(b *testing.B, routes []*Route) {