Skip to content

Commit 71e1ebf

Browse files
authored
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 <[email protected]>
1 parent afe9d2c commit 71e1ebf

File tree

4 files changed

+4
-18
lines changed

4 files changed

+4
-18
lines changed

integrations/links_test.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func TestLinksNoLogin(t *testing.T) {
3333
"/user/forgot_password",
3434
"/api/swagger",
3535
"/user2/repo1",
36+
"/user2/repo1/",
3637
"/user2/repo1/projects",
3738
"/user2/repo1/projects/1",
3839
"/assets/img/404.png",
@@ -61,16 +62,6 @@ func TestRedirectsNoLogin(t *testing.T) {
6162
resp := MakeRequest(t, req, http.StatusFound)
6263
assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp))
6364
}
64-
65-
var temporaryRedirects = map[string]string{
66-
"/user2/repo1/": "/user2/repo1",
67-
}
68-
for link, redirectLink := range temporaryRedirects {
69-
req := NewRequest(t, "GET", link)
70-
resp := MakeRequest(t, req, http.StatusTemporaryRedirect)
71-
assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp))
72-
}
73-
7465
}
7566

7667
func TestNoLoginNotExist(t *testing.T) {

modules/context/context.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -673,9 +673,6 @@ func Contexter() func(next http.Handler) http.Handler {
673673
var startTime = time.Now()
674674
var link = setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/")
675675

676-
chiCtx := chi.RouteContext(req.Context())
677-
chiCtx.RoutePath = req.URL.EscapedPath()
678-
679676
var ctx = Context{
680677
Resp: NewResponse(resp),
681678
Cache: mc.GetCache(),

routers/common/middleware.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ func Middlewares() []func(http.Handler) http.Handler {
2222
var handlers = []func(http.Handler) http.Handler{
2323
func(next http.Handler) http.Handler {
2424
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
25+
// First of all escape the URL RawPath to ensure that all routing is done using a correctly escaped URL
26+
req.URL.RawPath = req.URL.EscapedPath()
27+
2528
next.ServeHTTP(context.NewResponse(resp), req)
2629
})
2730
},

routers/web/web.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,11 +1037,6 @@ func RegisterRoutes(m *web.Route) {
10371037
m.Get("/swagger.v1.json", SwaggerV1Json)
10381038
}
10391039
m.NotFound(func(w http.ResponseWriter, req *http.Request) {
1040-
escapedPath := req.URL.EscapedPath()
1041-
if len(escapedPath) > 1 && escapedPath[len(escapedPath)-1] == '/' {
1042-
http.Redirect(w, req, setting.AppSubURL+escapedPath[:len(escapedPath)-1], http.StatusTemporaryRedirect)
1043-
return
1044-
}
10451040
ctx := context.GetContext(req)
10461041
ctx.NotFound("", nil)
10471042
})

0 commit comments

Comments
 (0)