From fc6263371d8408369cb93c5846038041f06dad4b Mon Sep 17 00:00:00 2001 From: citizen233 Date: Sun, 29 Aug 2021 21:09:20 +0800 Subject: [PATCH 1/4] fix the misplacement of adding slashes --- tree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree.go b/tree.go index fb0a5935c2..e13be813fc 100644 --- a/tree.go +++ b/tree.go @@ -599,7 +599,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 == "/" || - (len(prefix) == len(path)+1 && n.handlers != nil) + (len(prefix) == len(path)+1 && path == prefix[:len(prefix) - 1] && n.handlers != nil) return } } From d58ba30906a1fa65d9cf7acb7039e10c47104296 Mon Sep 17 00:00:00 2001 From: citizen233 Date: Sun, 29 Aug 2021 21:32:31 +0800 Subject: [PATCH 2/4] feat: add fix #2843 test example --- routes_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/routes_test.go b/routes_test.go index 036fa1c349..380ed1b983 100644 --- a/routes_test.go +++ b/routes_test.go @@ -621,3 +621,24 @@ func TestRouteContextHoldsFullPath(t *testing.T) { w := performRequest(router, http.MethodGet, "/not-found") assert.Equal(t, http.StatusNotFound, w.Code) } + +// Reproduction test for the bug of issue #2843 +func TestRouteTrailingSlashRoute(t *testing.T) { + router := New() + + router.NoRoute(func(c *Context) { + if c.Request.RequestURI == "/login" { + c.String(200, "login") + } + }) + + router.GET("/logout", func(c *Context) { + c.String(200, "logout") + }) + + w := performRequest(router, http.MethodGet, "/login") + assert.Equal(t, "login", w.Body.String()) + + w = performRequest(router, http.MethodGet, "/logout") + assert.Equal(t, "logout", w.Body.String()) +} From 86831de61c0aee0288cb79e9c40e8d14c5424d27 Mon Sep 17 00:00:00 2001 From: citizen233 Date: Mon, 30 Aug 2021 12:35:10 +0800 Subject: [PATCH 3/4] feat: modify the code according to the codereview opinion --- routes_test.go | 38 ++++++++++++++++---------------------- tree.go | 3 ++- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/routes_test.go b/routes_test.go index 380ed1b983..85805f838f 100644 --- a/routes_test.go +++ b/routes_test.go @@ -481,6 +481,21 @@ func TestRouterNotFound(t *testing.T) { router.GET("/a", func(c *Context) {}) w = performRequest(router, http.MethodGet, "/") assert.Equal(t, http.StatusNotFound, w.Code) + + // Reproduction test for the bug of issue #2843 + router = New() + router.NoRoute(func(c *Context) { + if c.Request.RequestURI == "/login" { + c.String(200, "login") + } + }) + router.GET("/logout", func(c *Context) { + c.String(200, "logout") + }) + w = performRequest(router, http.MethodGet, "/login") + assert.Equal(t, "login", w.Body.String()) + w = performRequest(router, http.MethodGet, "/logout") + assert.Equal(t, "logout", w.Body.String()) } func TestRouterStaticFSNotFound(t *testing.T) { @@ -620,25 +635,4 @@ func TestRouteContextHoldsFullPath(t *testing.T) { w := performRequest(router, http.MethodGet, "/not-found") assert.Equal(t, http.StatusNotFound, w.Code) -} - -// Reproduction test for the bug of issue #2843 -func TestRouteTrailingSlashRoute(t *testing.T) { - router := New() - - router.NoRoute(func(c *Context) { - if c.Request.RequestURI == "/login" { - c.String(200, "login") - } - }) - - router.GET("/logout", func(c *Context) { - c.String(200, "logout") - }) - - w := performRequest(router, http.MethodGet, "/login") - assert.Equal(t, "login", w.Body.String()) - - w = performRequest(router, http.MethodGet, "/logout") - assert.Equal(t, "logout", w.Body.String()) -} +} \ No newline at end of file diff --git a/tree.go b/tree.go index e13be813fc..0cd3367b22 100644 --- a/tree.go +++ b/tree.go @@ -599,7 +599,8 @@ 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 == "/" || - (len(prefix) == len(path)+1 && path == prefix[:len(prefix) - 1] && n.handlers != nil) + (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && + path == prefix[:len(prefix) - 1] && n.handlers != nil) return } } From b33f331d7170b2b38d4c4e859e8a333aae05897f Mon Sep 17 00:00:00 2001 From: citizen233 Date: Tue, 31 Aug 2021 10:24:33 +0800 Subject: [PATCH 4/4] feat: format code --- routes_test.go | 2 +- tree.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/routes_test.go b/routes_test.go index 85805f838f..ffe3469e7c 100644 --- a/routes_test.go +++ b/routes_test.go @@ -635,4 +635,4 @@ func TestRouteContextHoldsFullPath(t *testing.T) { w := performRequest(router, http.MethodGet, "/not-found") assert.Equal(t, http.StatusNotFound, w.Code) -} \ No newline at end of file +} diff --git a/tree.go b/tree.go index 0cd3367b22..b23ccfd584 100644 --- a/tree.go +++ b/tree.go @@ -600,7 +600,7 @@ walk: // Outer loop for walking the tree // 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) + path == prefix[:len(prefix)-1] && n.handlers != nil) return } }