Skip to content

Commit ae6f7a3

Browse files
ibraheemdevthinkerou
authored andcommitted
fix tsr with mixed static and wildcard paths (#2924)
1 parent bb945cf commit ae6f7a3

File tree

2 files changed

+49
-26
lines changed

2 files changed

+49
-26
lines changed

tree.go

+24-24
Original file line numberDiff line numberDiff line change
@@ -448,27 +448,26 @@ walk: // Outer loop for walking the tree
448448
continue walk
449449
}
450450
}
451-
// If the path at the end of the loop is not equal to '/' and the current node has no child nodes
452-
// the current node needs to roll back to last vaild skippedNode
453-
454-
if path != "/" && !n.wildChild {
455-
for l := len(*skippedNodes); l > 0; {
456-
skippedNode := (*skippedNodes)[l-1]
457-
*skippedNodes = (*skippedNodes)[:l-1]
458-
if strings.HasSuffix(skippedNode.path, path) {
459-
path = skippedNode.path
460-
n = skippedNode.node
461-
if value.params != nil {
462-
*value.params = (*value.params)[:skippedNode.paramsCount]
451+
452+
if !n.wildChild {
453+
// If the path at the end of the loop is not equal to '/' and the current node has no child nodes
454+
// the current node needs to roll back to last vaild skippedNode
455+
if path != "/" {
456+
for l := len(*skippedNodes); l > 0; {
457+
skippedNode := (*skippedNodes)[l-1]
458+
*skippedNodes = (*skippedNodes)[:l-1]
459+
if strings.HasSuffix(skippedNode.path, path) {
460+
path = skippedNode.path
461+
n = skippedNode.node
462+
if value.params != nil {
463+
*value.params = (*value.params)[:skippedNode.paramsCount]
464+
}
465+
globalParamsCount = skippedNode.paramsCount
466+
continue walk
463467
}
464-
globalParamsCount = skippedNode.paramsCount
465-
continue walk
466468
}
467469
}
468-
}
469470

470-
// If there is no wildcard pattern, recommend a redirection
471-
if !n.wildChild {
472471
// Nothing found.
473472
// We can recommend to redirect to the same URL without a
474473
// trailing slash if a leaf exists for that path.
@@ -615,8 +614,14 @@ walk: // Outer loop for walking the tree
615614
return
616615
}
617616

618-
// roll back to last vaild skippedNode
619-
if path != "/" {
617+
// Nothing found. We can recommend to redirect to the same URL with an
618+
// extra trailing slash if a leaf exists for that path
619+
value.tsr = path == "/" ||
620+
(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
621+
path == prefix[:len(prefix)-1] && n.handlers != nil)
622+
623+
// roll back to last valid skippedNode
624+
if !value.tsr && path != "/" {
620625
for l := len(*skippedNodes); l > 0; {
621626
skippedNode := (*skippedNodes)[l-1]
622627
*skippedNodes = (*skippedNodes)[:l-1]
@@ -632,11 +637,6 @@ walk: // Outer loop for walking the tree
632637
}
633638
}
634639

635-
// Nothing found. We can recommend to redirect to the same URL with an
636-
// extra trailing slash if a leaf exists for that path
637-
value.tsr = path == "/" ||
638-
(len(prefix) == len(path)+1 && prefix[len(path)] == '/' &&
639-
path == prefix[:len(prefix)-1] && n.handlers != nil)
640640
return
641641
}
642642
}

tree_test.go

+25-2
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,14 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
587587
"/doc/go1.html",
588588
"/no/a",
589589
"/no/b",
590-
"/api/hello/:name",
590+
"/api/:page/:name",
591+
"/api/hello/:name/bar/",
592+
"/api/bar/:name",
593+
"/api/baz/foo",
594+
"/api/baz/foo/bar",
595+
"/blog/:p",
596+
"/posts/:b/:c",
597+
"/posts/b/:c/d/",
591598
}
592599
for _, route := range routes {
593600
recv := catchPanic(func() {
@@ -613,7 +620,19 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
613620
"/admin/config/",
614621
"/admin/config/permissions/",
615622
"/doc/",
623+
"/admin/static/",
624+
"/admin/cfg/",
625+
"/admin/cfg/users/",
626+
"/api/hello/x/bar",
627+
"/api/baz/foo/",
628+
"/api/baz/bax/",
629+
"/api/bar/huh/",
630+
"/api/baz/foo/bar/",
631+
"/api/world/abc/",
632+
"/blog/pp/",
633+
"/posts/b/c/d",
616634
}
635+
617636
for _, route := range tsrRoutes {
618637
value := tree.getValue(route, nil, getSkippedNodes(), false)
619638
if value.handlers != nil {
@@ -629,7 +648,11 @@ func TestTreeTrailingSlashRedirect(t *testing.T) {
629648
"/no/",
630649
"/_",
631650
"/_/",
632-
"/api/world/abc",
651+
"/api",
652+
"/api/",
653+
"/api/hello/x/foo",
654+
"/api/baz/foo/bad",
655+
"/foo/p/p",
633656
}
634657
for _, route := range noTsrRoutes {
635658
value := tree.getValue(route, nil, getSkippedNodes(), false)

0 commit comments

Comments
 (0)