Fix named route matching when static route with trailing slash is registered

gin_test.go

@@ -1195,3 +1195,78 @@ func TestRouterQueryMethod(t *testing.T) {
 	router.ServeHTTP(w2, req2)
 	assert.Equal(t, http.StatusNotFound, w2.Code)
 }
+
+func TestRouterParamRouteWithTrailingSlashSibling(t *testing.T) {
+	for _, redirectTrailingSlash := range []bool{false, true} {
+		// Case 1: Route /b/ and /:name, request /b
+		router1 := New()
+		router1.RedirectTrailingSlash = redirectTrailingSlash
+
+		var matchedRoute string
+		var matchedParams Params
+
+		router1.GET("/b/", func(c *Context) {
+			matchedRoute = "/b/"
+			matchedParams = c.Params
+			c.String(http.StatusOK, "b_slash")
+		})
+		router1.GET("/:name", func(c *Context) {
+			matchedRoute = "/:name"
+			matchedParams = c.Params
+			c.String(http.StatusOK, "name")
+		})
+
+		// GET /b -> should match /:name with name=b
+		w := httptest.NewRecorder()
+		req, _ := http.NewRequest(http.MethodGet, "/b", nil)
+		router1.ServeHTTP(w, req)
+		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, "/:name", matchedRoute)
+		assert.Equal(t, Params{Param{Key: "name", Value: "b"}}, matchedParams)
+
+		// GET /b/ -> should match /b/
+		w = httptest.NewRecorder()
+		req, _ = http.NewRequest(http.MethodGet, "/b/", nil)
+		router1.ServeHTTP(w, req)
+		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, "/b/", matchedRoute)
+
+		// GET /c -> should match /:name with name=c
+		w = httptest.NewRecorder()
+		req, _ = http.NewRequest(http.MethodGet, "/c", nil)
+		router1.ServeHTTP(w, req)
+		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, "/:name", matchedRoute)
+		assert.Equal(t, Params{Param{Key: "name", Value: "c"}}, matchedParams)
+
+		// Case 2: Mirror case: /a/b and /:p/b/, request /a/b/
+		router2 := New()
+		router2.RedirectTrailingSlash = redirectTrailingSlash
+
+		router2.GET("/a/b", func(c *Context) {
+			matchedRoute = "/a/b"
+			matchedParams = c.Params
+			c.String(http.StatusOK, "ab")
+		})
+		router2.GET("/:p/b/", func(c *Context) {
+			matchedRoute = "/:p/b/"
+			matchedParams = c.Params
+			c.String(http.StatusOK, "pb_slash")
+		})
+
+		// GET /a/b/ -> should match /:p/b/ with p=a
+		w = httptest.NewRecorder()
+		req, _ = http.NewRequest(http.MethodGet, "/a/b/", nil)
+		router2.ServeHTTP(w, req)
+		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, "/:p/b/", matchedRoute)
+		assert.Equal(t, Params{Param{Key: "p", Value: "a"}}, matchedParams)
+
+		// GET /a/b -> should match /a/b
+		w = httptest.NewRecorder()
+		req, _ = http.NewRequest(http.MethodGet, "/a/b", nil)
+		router2.ServeHTTP(w, req)
+		assert.Equal(t, http.StatusOK, w.Code)
+		assert.Equal(t, "/a/b", matchedRoute)
+	}
+}

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/mattn/go-isatty v0.0.20
 	github.com/modern-go/reflect2 v1.0.2
 	github.com/pelletier/go-toml/v2 v2.2.4
-	github.com/quic-go/quic-go v0.60.0
+	github.com/quic-go/quic-go v0.63.0
 	github.com/stretchr/testify v1.12.1
 	github.com/ugorji/go/codec v1.3.1
 	go.mongodb.org/mongo-driver/v2 v2.5.0

go.sum

@@ -50,6 +50,8 @@ github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8=
 github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII=
 github.com/quic-go/quic-go v0.60.0 h1:xcQioE8OM66UQLeUMHltK1CCcOu3JbVB4JAQdDQSB+0=
 github.com/quic-go/quic-go v0.60.0/go.mod h1:wpKpjmPpftl30sL6pFh7REVpjbcCVy4zt2vDyK1TuJk=
+github.com/quic-go/quic-go v0.63.0 h1:LIFGHI4PFUhhw2dDD1ARHdCff143ffMHwZtbnbuJ78A=
+github.com/quic-go/quic-go v0.63.0/go.mod h1:RAro2j2yN9a9EiPACLHT9IB2NXCvGQmmo/alT0yYI0w=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=

tree.go

@@ -461,28 +461,28 @@ walk: // Outer loop for walking the tree
 				}
 
 				if !n.wildChild {
-					// If the path at the end of the loop is not equal to '/' and the current node has no child nodes
-					// the current node needs to roll back to last valid skippedNode
-					if path != "/" {
-						for length := len(*skippedNodes); length > 0; length-- {
-							skippedNode := (*skippedNodes)[length-1]
-							*skippedNodes = (*skippedNodes)[:length-1]
-							if strings.HasSuffix(skippedNode.path, path) {
-								path = skippedNode.path
-								n = skippedNode.node
-								if value.params != nil {
-									*value.params = (*value.params)[:skippedNode.paramsCount]
-								}
-								globalParamsCount = skippedNode.paramsCount
-								continue walk
+					// If the current node has no child nodes,
+					// roll back to last valid skippedNode
+					cTsr := path == "/" && n.handlers != nil
+					for length := len(*skippedNodes); length > 0; length-- {
+						skippedNode := (*skippedNodes)[length-1]
+						*skippedNodes = (*skippedNodes)[:length-1]
+						if strings.HasSuffix(skippedNode.path, path) {
+							path = skippedNode.path
+							n = skippedNode.node
+							if value.params != nil {
+								*value.params = (*value.params)[:skippedNode.paramsCount]
 							}
+							globalParamsCount = skippedNode.paramsCount
+							value.tsr = value.tsr || cTsr
+							continue walk
 						}
 					}
 
 					// Nothing found.
 					// We can recommend to redirect to the same URL without a
 					// trailing slash if a leaf exists for that path.
-					value.tsr = path == "/" && n.handlers != nil
+					value.tsr = value.tsr || cTsr
 					return value
 				}
 
@@ -537,20 +537,52 @@ walk: // Outer loop for walking the tree
 						}
 
 						// ... but we can't
-						value.tsr = len(path) == end+1
+						cTsr := len(path) == end+1
+						for length := len(*skippedNodes); length > 0; length-- {
+							skippedNode := (*skippedNodes)[length-1]
+							*skippedNodes = (*skippedNodes)[:length-1]
+							if strings.HasSuffix(skippedNode.path, path) {
+								path = skippedNode.path
+								n = skippedNode.node
+								if value.params != nil {
+									*value.params = (*value.params)[:skippedNode.paramsCount]
+								}
+								globalParamsCount = skippedNode.paramsCount
+								value.tsr = value.tsr || cTsr
+								continue walk
+							}
+						}
+						value.tsr = value.tsr || cTsr
 						return value
 					}
 
 					if value.handlers = n.handlers; value.handlers != nil {
 						value.fullPath = n.fullPath
+						value.tsr = false
 						return value
 					}
+					cTsr := false
 					if len(n.children) == 1 {
 						// No handle found. Check if a handle for this path + a
 						// trailing slash exists for TSR recommendation
 						n = n.children[0]
-						value.tsr = (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/")
+						cTsr = (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/")
 					}
+					for length := len(*skippedNodes); length > 0; length-- {
+						skippedNode := (*skippedNodes)[length-1]
+						*skippedNodes = (*skippedNodes)[:length-1]
+						if strings.HasSuffix(skippedNode.path, path) {
+							path = skippedNode.path
+							n = skippedNode.node
+							if value.params != nil {
+								*value.params = (*value.params)[:skippedNode.paramsCount]
+							}
+							globalParamsCount = skippedNode.paramsCount
+							value.tsr = value.tsr || cTsr
+							continue walk
+						}
+					}
+					value.tsr = value.tsr || cTsr
 					return value
 
 				case catchAll:
@@ -583,6 +615,7 @@ walk: // Outer loop for walking the tree
 
 					value.handlers = n.handlers
 					value.fullPath = n.fullPath
+					value.tsr = false
 					return value
 
 				default:
@@ -592,9 +625,9 @@ walk: // Outer loop for walking the tree
 		}
 
 		if path == prefix {
-			// If the current path does not equal '/' and the node does not have a registered handle and the most recently matched node has a child node
+			// If the current node does not have a registered handle,
 			// the current node needs to roll back to last valid skippedNode
-			if n.handlers == nil && path != "/" {
+			if n.handlers == nil {
 				for length := len(*skippedNodes); length > 0; length-- {
 					skippedNode := (*skippedNodes)[length-1]
 					*skippedNodes = (*skippedNodes)[:length-1]
@@ -608,50 +641,32 @@ walk: // Outer loop for walking the tree
 						continue walk
 					}
 				}
-				//	n = latestNode.children[len(latestNode.children)-1]
 			}
 			// We should have reached the node containing the handle.
 			// Check if this node has a handle registered.
 			if value.handlers = n.handlers; value.handlers != nil {
 				value.fullPath = n.fullPath
+				value.tsr = false
 				return value
 			}
 
 			// If there is no handle for this route, but this route has a
 			// wildcard child, there must be a handle for this path with an
 			// additional trailing slash
-			if path == "/" && n.wildChild && n.nType != root {
-				value.tsr = true
-				return value
-			}
-
-			if path == "/" && n.nType == static {
-				value.tsr = true
-				return value
-			}
-
-			// No handle found. Check if a handle for this path + a
-			// trailing slash exists for trailing slash recommendation
-			for i, c := range []byte(n.indices) {
-				if c == '/' {
-					n = n.children[i]
-					value.tsr = (len(n.path) == 1 && n.handlers != nil) ||
-						(n.nType == catchAll && n.children[0].handlers != nil)
-					return value
+			cTsr := (path == "/" && n.wildChild && n.nType != root) || (path == "/" && n.nType == static)
+			if !cTsr {
+				// No handle found. Check if a handle for this path + a
+				// trailing slash exists for trailing slash recommendation
+				for i, c := range []byte(n.indices) {
+					if c == '/' {
+						n = n.children[i]
+						cTsr = (len(n.path) == 1 && n.handlers != nil) ||
+							(n.nType == catchAll && n.children[0].handlers != nil)
+						break
+					}
 				}
 			}
 
-			return value
-		}
-
-		// Nothing found. We can recommend to redirect to the same URL with an
-		// extra trailing slash if a leaf exists for that path
-		value.tsr = path == "/" ||
-			(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
-				path == prefix[:len(prefix)-1] && n.handlers != nil)
-
-		// roll back to last valid skippedNode
-		if !value.tsr && path != "/" {
 			for length := len(*skippedNodes); length > 0; length-- {
 				skippedNode := (*skippedNodes)[length-1]
 				*skippedNodes = (*skippedNodes)[:length-1]
@@ -662,11 +677,38 @@ walk: // Outer loop for walking the tree
 						*value.params = (*value.params)[:skippedNode.paramsCount]
 					}
 					globalParamsCount = skippedNode.paramsCount
+					value.tsr = value.tsr || cTsr
 					continue walk
 				}
 			}
+
+			value.tsr = value.tsr || cTsr
+			return value
+		}
+
+		// Nothing found. We can recommend to redirect to the same URL with an
+		// extra trailing slash if a leaf exists for that path
+		cTsr := path == "/" ||
+			(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
+				path == prefix[:len(prefix)-1] && n.handlers != nil)
+
+		// roll back to last valid skippedNode
+		for length := len(*skippedNodes); length > 0; length-- {
+			skippedNode := (*skippedNodes)[length-1]
+			*skippedNodes = (*skippedNodes)[:length-1]
+			if strings.HasSuffix(skippedNode.path, path) {
+				path = skippedNode.path
+				n = skippedNode.node
+				if value.params != nil {
+					*value.params = (*value.params)[:skippedNode.paramsCount]
+				}
+				globalParamsCount = skippedNode.paramsCount
+				value.tsr = value.tsr || cTsr
+				continue walk
+			}
 		}
 
+		value.tsr = value.tsr || cTsr
 		return value
 	}
 }

tree_test.go

@@ -1111,3 +1111,77 @@ func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) {
 		t.Errorf("Wrong result for '/prefix/something': %s", string(out))
 	}
 }
+
+func TestTreeTrailingSlashAndParamRoute(t *testing.T) {
+	// Case 1: Route /b/ and /:name, request /b
+	// GET /b should match /:name with param name="b", rather than redirecting to /b/
+	tree := &node{}
+	routes := []string{
+		"/b/",
+		"/:name",
+	}
+	for _, route := range routes {
+		tree.addRoute(route, fakeHandler(route))
+	}
+
+	checkRequests(t, tree, testRequests{
+		{"/b", false, "/:name", Params{Param{Key: "name", Value: "b"}}},
+		{"/c", false, "/:name", Params{Param{Key: "name", Value: "c"}}},
+		{"/b/", false, "/b/", nil},
+	})
+	checkPriorities(t, tree)
+
+	// Case 2: Mirror case: /a/b and /:p/b/, request /a/b/
+	// GET /a/b/ should match /:p/b/ with param p="a", rather than redirecting to /a/b
+	tree2 := &node{}
+	routes2 := []string{
+		"/a/b",
+		"/:p/b/",
+	}
+	for _, route := range routes2 {
+		tree2.addRoute(route, fakeHandler(route))
+	}
+
+	checkRequests(t, tree2, testRequests{
+		{"/a/b/", false, "/:p/b/", Params{Param{Key: "p", Value: "a"}}},
+		{"/a/b", false, "/a/b", nil},
+	})
+	checkPriorities(t, tree2)
+
+	// Case 3: When no wildcard route matches the exact request,
+	// TSR should still be recommended for candidate static route
+	tree3 := &node{}
+	routes3 := []string{
+		"/b/",
+		"/:name/x",
+	}
+	for _, route := range routes3 {
+		tree3.addRoute(route, fakeHandler(route))
+	}
+
+	val := tree3.getValue("/b", nil, getSkippedNodes(), false)
+	if val.handlers != nil {
+		t.Errorf("Expected nil handler for '/b', got: %v", val.handlers)
+	}
+	if !val.tsr {
+		t.Errorf("Expected TSR=true for '/b' when '/b/' exists and '/:name/x' does not match")
+	}
+
+	// Case 4: Mirror TSR fallback when no wildcard route matches
+	tree4 := &node{}
+	routes4 := []string{
+		"/a/b",
+		"/:p/b/x",
+	}
+	for _, route := range routes4 {
+		tree4.addRoute(route, fakeHandler(route))
+	}
+
+	val4 := tree4.getValue("/a/b/", nil, getSkippedNodes(), false)
+	if val4.handlers != nil {
+		t.Errorf("Expected nil handler for '/a/b/', got: %v", val4.handlers)
+	}
+	if !val4.tsr {
+		t.Errorf("Expected TSR=true for '/a/b/' when '/a/b' exists and '/:p/b/x' does not match")
+	}
+}