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>
This commit is contained in:
zeripath 2021-12-26 10:32:04 +00:00 committed by GitHub
parent afe9d2cadd
commit 71e1ebfa60
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 4 additions and 18 deletions

View file

@ -33,6 +33,7 @@ func TestLinksNoLogin(t *testing.T) {
"/user/forgot_password", "/user/forgot_password",
"/api/swagger", "/api/swagger",
"/user2/repo1", "/user2/repo1",
"/user2/repo1/",
"/user2/repo1/projects", "/user2/repo1/projects",
"/user2/repo1/projects/1", "/user2/repo1/projects/1",
"/assets/img/404.png", "/assets/img/404.png",
@ -61,16 +62,6 @@ func TestRedirectsNoLogin(t *testing.T) {
resp := MakeRequest(t, req, http.StatusFound) resp := MakeRequest(t, req, http.StatusFound)
assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp)) 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) { func TestNoLoginNotExist(t *testing.T) {

View file

@ -673,9 +673,6 @@ func Contexter() func(next http.Handler) http.Handler {
var startTime = time.Now() var startTime = time.Now()
var link = setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/") var link = setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/")
chiCtx := chi.RouteContext(req.Context())
chiCtx.RoutePath = req.URL.EscapedPath()
var ctx = Context{ var ctx = Context{
Resp: NewResponse(resp), Resp: NewResponse(resp),
Cache: mc.GetCache(), Cache: mc.GetCache(),

View file

@ -22,6 +22,9 @@ func Middlewares() []func(http.Handler) http.Handler {
var handlers = []func(http.Handler) http.Handler{ var handlers = []func(http.Handler) http.Handler{
func(next http.Handler) http.Handler { func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { 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) next.ServeHTTP(context.NewResponse(resp), req)
}) })
}, },

View file

@ -1037,11 +1037,6 @@ func RegisterRoutes(m *web.Route) {
m.Get("/swagger.v1.json", SwaggerV1Json) m.Get("/swagger.v1.json", SwaggerV1Json)
} }
m.NotFound(func(w http.ResponseWriter, req *http.Request) { 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 := context.GetContext(req)
ctx.NotFound("", nil) ctx.NotFound("", nil)
}) })