diff options
author | zeripath | 2021-12-26 10:32:04 +0000 |
---|---|---|
committer | GitHub | 2021-12-26 10:32:04 +0000 |
commit | 71e1ebfa60c27706656b44133a4c04e929bddc8d (patch) | |
tree | 7ca5b3aa5bb01f03959e6fc6dc0b8c9c3476053e | |
parent | afe9d2cadd8ee778f698d5b2874b74b09c7d7d9d (diff) |
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>
-rw-r--r-- | integrations/links_test.go | 11 | ||||
-rw-r--r-- | modules/context/context.go | 3 | ||||
-rw-r--r-- | routers/common/middleware.go | 3 | ||||
-rw-r--r-- | routers/web/web.go | 5 |
4 files changed, 4 insertions, 18 deletions
diff --git a/integrations/links_test.go b/integrations/links_test.go index 2b8bbde08..c461f81b9 100644 --- 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 5dcf2e755..651fc42b7 100644 --- 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 1d96522dd..dfb3d8a8e 100644 --- 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 45cf536bc..8403084bc 100644 --- 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) }) |