From 8980f8482c749a9b711cebb3b90b74a8c1f12626 Mon Sep 17 00:00:00 2001 From: rajpreetcodes Date: Wed, 14 Oct 2026 19:18:40 +0530 Subject: [PATCH] fix(tree): preserve trailing slash redirect when wildcard and common prefix routes coexist When multiple static routes share a common prefix and a wildcard route is also registered, the radix tree creates an intermediate prefix node without handlers. During path matching where the requested path equals the intermediate prefix, the search previously rolled back to skipped wildcard nodes without checking if the intermediate node contains a trailing slash child with a valid handler. If the backtracked wildcard route did not match, the trailing slash redirect recommendation was lost and returned 404 instead of 301. This commit checks and preserves trailing slash recommendations on intermediate prefix nodes before backtracking to wildcard nodes, ensuring requests redirect correctly when static trailing slash routes exist while preserving exact matches for wildcard routes. Fixes #4890 --- routes_test.go | 12 +++++++++++ tree.go | 57 +++++++++++++++++++++++++++----------------------- tree_test.go | 14 +++++++++++++ 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/routes_test.go b/routes_test.go index a032a2b76a..b32f54103c 100644 --- a/routes_test.go +++ b/routes_test.go @@ -303,6 +303,18 @@ func TestRouteRedirectTrailingSlash(t *testing.T) { assert.Equal(t, http.StatusNotFound, w.Code) } +func TestRouteRedirectTrailingSlashWildcardAndCommonPrefix(t *testing.T) { + router := New() + router.RedirectTrailingSlash = true + router.GET("/:c/bb", func(c *Context) {}) + router.GET("/aa/", func(c *Context) {}) + router.GET("/aa_id", func(c *Context) {}) + + w := PerformRequest(router, http.MethodGet, "/aa") + assert.Equal(t, "/aa/", w.Header().Get("Location")) + assert.Equal(t, http.StatusMovedPermanently, w.Code) +} + func TestRouteRedirectFixedPath(t *testing.T) { router := New() router.RedirectFixedPath = true diff --git a/tree.go b/tree.go index 6842d225fd..91fe062ae6 100644 --- a/tree.go +++ b/tree.go @@ -482,7 +482,7 @@ walk: // Outer loop for walking the tree // 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 || (path == "/" && n.handlers != nil) return value } @@ -537,11 +537,12 @@ walk: // Outer loop for walking the tree } // ... but we can't - value.tsr = len(path) == end+1 + value.tsr = value.tsr || len(path) == end+1 return value } if value.handlers = n.handlers; value.handlers != nil { + value.tsr = false value.fullPath = n.fullPath return value } @@ -549,7 +550,7 @@ walk: // Outer loop for walking the tree // 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 == "/") + value.tsr = value.tsr || (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/") } return value @@ -582,6 +583,7 @@ walk: // Outer loop for walking the tree } value.handlers = n.handlers + value.tsr = false value.fullPath = n.fullPath return value @@ -592,27 +594,10 @@ 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 - // the current node needs to roll back to last valid skippedNode - if n.handlers == nil && 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 - } - } - // 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.tsr = false value.fullPath = n.fullPath return value } @@ -634,10 +619,30 @@ walk: // Outer loop for walking the tree // 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 + child := n.children[i] + if (len(child.path) == 1 && child.handlers != nil) || + (child.nType == catchAll && child.children[0].handlers != nil) { + value.tsr = true + } + break + } + } + + // 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 + // 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 + } } } @@ -646,7 +651,7 @@ walk: // Outer loop for walking the tree // 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 == "/" || + value.tsr = value.tsr || path == "/" || (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && path == prefix[:len(prefix)-1] && n.handlers != nil) diff --git a/tree_test.go b/tree_test.go index 23339af4bb..86f07491e6 100644 --- a/tree_test.go +++ b/tree_test.go @@ -1111,3 +1111,17 @@ func TestTreeFindCaseInsensitivePathWildcardParamAndStaticChild(t *testing.T) { t.Errorf("Wrong result for '/prefix/something': %s", string(out)) } } + +func TestRedirectTrailingSlashWildcardAndCommonPrefix(t *testing.T) { + routes := []string{"/:c/bb", "/aa/", "/aa_id"} + tree := &node{} + for _, r := range routes { + tree.addRoute(r, fakeHandler(r)) + } + + skippedNodes := make([]skippedNode, 0) + value := tree.getValue("/aa", nil, &skippedNodes, false) + if !value.tsr { + t.Fatalf("expected TSR recommendation for '/aa', got tsr=false, handlers=%v", value.handlers) + } +}