fix: use correct input for strip slashes middleware (#7295)

- The router must use the escaped path in order to ensure correct functionality (at least, that is what they say). However `req.URL.Path` shouldn't be set to the escaped path, which is fixed in this patch.
- Simplify the logic and no longer try to use `rctx.RoutePath`, this is only useful if the middleware was placed after some routing parsing was done.
- Resolves forgejo/forgejo#7294
- Resolves forgejo/forgejo#7292
- Add unit test

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7295
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>

routers/common/middleware.go

@@ -77,27 +77,27 @@ func ProtocolMiddlewares() (handlers []any) {
 
 func stripSlashesMiddleware(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
+		// Ensure that URL.RawPath is always set.
 		req.URL.RawPath = req.URL.EscapedPath()
 
-		urlPath := req.URL.RawPath
-		rctx := chi.RouteContext(req.Context())
-		if rctx != nil && rctx.RoutePath != "" {
-			urlPath = rctx.RoutePath
-		}
-
-		sanitizedPath := &strings.Builder{}
-		prevWasSlash := false
-		for _, chr := range strings.TrimRight(urlPath, "/") {
-			if chr != '/' || !prevWasSlash {
-				sanitizedPath.WriteRune(chr)
+		sanitize := func(path string) string {
+			sanitizedPath := &strings.Builder{}
+			prevWasSlash := false
+			for _, chr := range strings.TrimRight(path, "/") {
+				if chr != '/' || !prevWasSlash {
+					sanitizedPath.WriteRune(chr)
+				}
+				prevWasSlash = chr == '/'
 			}
-			prevWasSlash = chr == '/'
+			return sanitizedPath.String()
 		}
 
-		req.URL.Path = sanitizedPath.String()
+		// Sanitize the unescaped path for application logic.
+		req.URL.Path = sanitize(req.URL.Path)
+		rctx := chi.RouteContext(req.Context())
 		if rctx != nil {
-			rctx.RoutePath = req.URL.Path
+			// Sanitize the escaped path for routing.
+			rctx.RoutePath = sanitize(req.URL.RawPath)
 		}
 		next.ServeHTTP(resp, req)
 	})

routers/common/middleware_test.go

@@ -15,9 +15,10 @@ import (
 
 func TestStripSlashesMiddleware(t *testing.T) {
 	type test struct {
-		name         string
-		expectedPath string
-		inputPath    string
+		name               string
+		expectedPath       string
+		expectedNormalPath string
+		inputPath          string
 	}
 
 	tests := []test{
@@ -57,9 +58,16 @@ func TestStripSlashesMiddleware(t *testing.T) {
 			expectedPath: "/repo/migrate",
 		},
 		{
-			name:         "path with encoded slash",
-			inputPath:    "/user2/%2F%2Frepo1",
-			expectedPath: "/user2/%2F%2Frepo1",
+			name:               "path with encoded slash",
+			inputPath:          "/user2/%2F%2Frepo1",
+			expectedPath:       "/user2/%2F%2Frepo1",
+			expectedNormalPath: "/user2/repo1",
+		},
+		{
+			name:               "path with space",
+			inputPath:          "/assets/css/theme%20cappuccino.css",
+			expectedPath:       "/assets/css/theme%20cappuccino.css",
+			expectedNormalPath: "/assets/css/theme cappuccino.css",
 		},
 	}
 
@@ -69,7 +77,11 @@ func TestStripSlashesMiddleware(t *testing.T) {
 
 		called := false
 		r.Get("*", func(w http.ResponseWriter, r *http.Request) {
-			assert.Equal(t, tt.expectedPath, r.URL.Path)
+			if tt.expectedNormalPath != "" {
+				assert.Equal(t, tt.expectedNormalPath, r.URL.Path)
+			} else {
+				assert.Equal(t, tt.expectedPath, r.URL.Path)
+			}
 
 			rctx := chi.RouteContext(r.Context())
 			assert.Equal(t, tt.expectedPath, rctx.RoutePath)

services/context/repo.go

@@ -1058,7 +1058,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
 
 			if refType == RepoRefLegacy {
 				// redirect from old URL scheme to new URL scheme
-				prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.PathParamRaw("*"))), strings.ToLower(ctx.Repo.RepoLink))
+				prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.Params("*"))), strings.ToLower(ctx.Repo.RepoLink))
 
 				ctx.Redirect(path.Join(
 					ctx.Repo.RepoLink,