Instead of using routerCtx just escape the url before routing (#18086) (#18098)

Backport #18086

A consequence of forcibly setting the RoutePath to the escaped url is that the
auto routing to endpoints without terminal slashes fails (Causing #18060.) This
failure raises the possibility that forcibly setting the RoutePath causes other
unexpected behaviors too.

Therefore, instead we should simply pre-escape the URL in the process registering
handler. Then the request URL will be properly escaped for all the following calls.

Fix #17938
Fix #18060
Replace #18062
Replace #17997

Signed-off-by: Andrew Thornton <art27@cantab.net>

diff --git a/integrations/links_test.go b/integrations/links_test.go
index 2b8bbde0861882d55750f101af9c087b8d74708e..c461f81b97b39f7de41f7b1d7c4cfb9802812970 100644 (file)
--- a/integrations/links_test.go
+++ b/integrations/links_test.go
@@ -33,6 +33,7 @@ func TestLinksNoLogin(t *testing.T) {
                "/user/forgot_password",
                "/api/swagger",
                "/user2/repo1",
+               "/user2/repo1/",
                "/user2/repo1/projects",
                "/user2/repo1/projects/1",
                "/assets/img/404.png",
@@ -61,16 +62,6 @@ 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) {

diff --git a/modules/context/context.go b/modules/context/context.go
index 5dcf2e7551b015124d65407b30d1cf3ed6d0500f..651fc42b7eaaf8616dc1657191d72285cda40e0a 100644 (file)
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -673,9 +673,6 @@ func Contexter() func(next http.Handler) http.Handler {
                        var startTime = time.Now()
                        var link = setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/")
 
-                       chiCtx := chi.RouteContext(req.Context())
-                       chiCtx.RoutePath = req.URL.EscapedPath()
-
                        var ctx = Context{
                                Resp:    NewResponse(resp),
                                Cache:   mc.GetCache(),

diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index 1d96522dd9d1933b3a62ff04538ab416b08aa41a..dfb3d8a8e8cd7f78e4fe69b4636c94ff0eeb0204 100644 (file)
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -22,6 +22,9 @@ func Middlewares() []func(http.Handler) http.Handler {
        var handlers = []func(http.Handler) http.Handler{
                func(next http.Handler) http.Handler {
                        return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+                               // First of all escape the URL RawPath to ensure that all routing is done using a correctly escaped URL
+                               req.URL.RawPath = req.URL.EscapedPath()
+
                                next.ServeHTTP(context.NewResponse(resp), req)
                        })
                },

diff --git a/routers/web/web.go b/routers/web/web.go
index 45cf536bc588f02bd3360201662b92d5d6a7bc84..8403084bcead832ba221a803d7a065587fe6ac38 100644 (file)
--- a/routers/web/web.go
+++ b/routers/web/web.go
@@ -1037,11 +1037,6 @@ func RegisterRoutes(m *web.Route) {
                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.StatusTemporaryRedirect)
-                       return
-               }
                ctx := context.GetContext(req)
                ctx.NotFound("", nil)
        })