From 6ac875930c00c1cf211f49e7b2b6875450789bdc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 21 Dec 2021 20:30:35 +0000 Subject: [PATCH 1/6] Add NotFound handler PR #17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix #18060 Signed-off-by: Andrew Thornton --- integrations/links_test.go | 1 + routers/web/web.go | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/integrations/links_test.go b/integrations/links_test.go index 91166274a2381..a3ae18d0a2703 100644 --- a/integrations/links_test.go +++ b/integrations/links_test.go @@ -49,6 +49,7 @@ func TestRedirectsNoLogin(t *testing.T) { defer prepareTestEnv(t)() var redirects = map[string]string{ + "/user2/repo1/": "/user2/repo1", "/user2/repo1/commits/master": "/user2/repo1/commits/branch/master", "/user2/repo1/src/master": "/user2/repo1/src/branch/master", "/user2/repo1/src/master/file.txt": "/user2/repo1/src/branch/master/file.txt", diff --git a/routers/web/web.go b/routers/web/web.go index 41c4e122fb4e9..99469a2e2ec24 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1078,4 +1078,13 @@ func RegisterRoutes(m *web.Route) { if setting.API.EnableSwagger { m.Get("/swagger.v1.json", SwaggerV1Json) } + m.NotFound(func(w http.ResponseWriter, req *http.Request) { + escapedPath := req.URL.EscapedPath() + if len(escapedPath) > 1 && escapedPath[len(escapedPath)-1] == '/' { + http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusFound) + } + ctx := context.GetContext(req) + ctx.NotFound("", nil) + }) + } From f948fc7862171618ba0e0cee34bbebdf98e7b1e4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 21 Dec 2021 22:44:02 +0000 Subject: [PATCH 2/6] fix test Signed-off-by: Andrew Thornton --- integrations/signout_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/signout_test.go b/integrations/signout_test.go index c31c913070f5d..b54e7ee9eeec4 100644 --- a/integrations/signout_test.go +++ b/integrations/signout_test.go @@ -18,7 +18,7 @@ func TestSignOut(t *testing.T) { session.MakeRequest(t, req, http.StatusFound) // try to view a private repo, should fail - req = NewRequest(t, "GET", "/user2/repo2/") + req = NewRequest(t, "GET", "/user2/repo2") session.MakeRequest(t, req, http.StatusNotFound) // invalidate cached cookies for user2, for subsequent tests From 2ea912dd772e490103165367e1fd8eb7d66534b1 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 22 Dec 2021 06:23:06 +0000 Subject: [PATCH 3/6] Update routers/web/web.go --- routers/web/web.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/web/web.go b/routers/web/web.go index 99469a2e2ec24..fa17c15101767 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1082,6 +1082,7 @@ func RegisterRoutes(m *web.Route) { escapedPath := req.URL.EscapedPath() if len(escapedPath) > 1 && escapedPath[len(escapedPath)-1] == '/' { http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusFound) + return } ctx := context.GetContext(req) ctx.NotFound("", nil) From 7c5f6c2c7a49702a690de1bf3d5de24f416e491d Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 22 Dec 2021 09:20:18 +0000 Subject: [PATCH 4/6] as per review --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index fa17c15101767..bca0dc1034a54 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1081,7 +1081,7 @@ func RegisterRoutes(m *web.Route) { m.NotFound(func(w http.ResponseWriter, req *http.Request) { escapedPath := req.URL.EscapedPath() if len(escapedPath) > 1 && escapedPath[len(escapedPath)-1] == '/' { - http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusFound) + http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusTemporaryRedirect) return } ctx := context.GetContext(req) From e3e0c783849b1777bc48fc5c99b46d85686e60d2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 22 Dec 2021 09:28:47 +0000 Subject: [PATCH 5/6] fix fmt Signed-off-by: Andrew Thornton --- routers/web/web.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/web.go b/routers/web/web.go index bca0dc1034a54..ebd0995df8379 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1082,7 +1082,7 @@ func RegisterRoutes(m *web.Route) { escapedPath := req.URL.EscapedPath() if len(escapedPath) > 1 && escapedPath[len(escapedPath)-1] == '/' { http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusTemporaryRedirect) - return + return } ctx := context.GetContext(req) ctx.NotFound("", nil) From 5235ee823987d82ee4c0c71f54021806ef8edd97 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 22 Dec 2021 09:54:20 +0000 Subject: [PATCH 6/6] fix test Signed-off-by: Andrew Thornton --- integrations/links_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/integrations/links_test.go b/integrations/links_test.go index a3ae18d0a2703..872234e61fcc9 100644 --- a/integrations/links_test.go +++ b/integrations/links_test.go @@ -49,7 +49,6 @@ func TestRedirectsNoLogin(t *testing.T) { defer prepareTestEnv(t)() var redirects = map[string]string{ - "/user2/repo1/": "/user2/repo1", "/user2/repo1/commits/master": "/user2/repo1/commits/branch/master", "/user2/repo1/src/master": "/user2/repo1/src/branch/master", "/user2/repo1/src/master/file.txt": "/user2/repo1/src/branch/master/file.txt", @@ -62,6 +61,16 @@ func TestRedirectsNoLogin(t *testing.T) { resp := MakeRequest(t, req, http.StatusFound) assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp)) } + + var temporaryRedirects = map[string]string{ + "/user2/repo1/": "/user2/repo1", + } + for link, redirectLink := range temporaryRedirects { + req := NewRequest(t, "GET", link) + resp := MakeRequest(t, req, http.StatusTemporaryRedirect) + assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp)) + } + } func TestNoLoginNotExist(t *testing.T) {