From dde3f51c72feffac4805fc3133b6a681d8c97c6f Mon Sep 17 00:00:00 2001
From: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Date: Sat, 22 Mar 2025 17:30:28 +0000
Subject: [PATCH] [v10.0/forgejo] fix: use correct input for strip slashes
middleware (#7306)
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/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
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/7295): <!--number 7295 --><!--line 0 --><!--description dXNlIGNvcnJlY3QgaW5wdXQgZm9yIHN0cmlwIHNsYXNoZXMgbWlkZGxld2FyZQ==-->use correct input for strip slashes middleware<!--description-->
<!--end release-notes-assistant-->
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7306
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
---
routers/common/middleware.go | 30 +++++++++++++++---------------
routers/common/middleware_test.go | 26 +++++++++++++++++++-------
services/context/repo.go | 2 +-
3 files changed, 35 insertions(+), 23 deletions(-)
diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index 856c10e678..873bc09b1d 100644
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -74,27 +74,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)
})
diff --git a/routers/common/middleware_test.go b/routers/common/middleware_test.go
index 0e111e1261..6126e0afcc 100644
--- a/routers/common/middleware_test.go
+++ b/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)
diff --git a/services/context/repo.go b/services/context/repo.go
index 71cf431273..462d843bfc 100644
--- a/services/context/repo.go
+++ b/services/context/repo.go
@@ -1051,7 +1051,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,