From 0aa872c4e3a319e892f088859111b384a9c11e9c Mon Sep 17 00:00:00 2001
From: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Date: Mon, 10 Feb 2025 20:00:25 +0000
Subject: [PATCH] [v10.0/forgejo] fix: always set stripped slashes on http
request (#6884)
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/6866
- The middleware that takes care of normalizing `//user2/////repo1` to `/user2/repo1` would only set the normalized value to the Chi (Forgejo's http router) `RoutePath` field, so Chi would correctly do the routing. However not all components in Forgejo (like Forgejo's `context` module) rely on Chi to get this updated path and some still rely on the value of `(http.Request).URL.Path`, so always set the normalized value to the http request.
- Adjusted unit test.
- Resolves forgejo/forgejo#6822
- The related issue was caused by
https://codeberg.org/forgejo/forgejo/src/commit/751a3da979db06adc99cc64fac6aad9505fd5a96/services/context/context.go#L115
using the value of the http request on not that was set in the Chi context.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6884
Reviewed-by: Otto <otto@codeberg.org>
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 | 7 +++----
routers/common/middleware_test.go | 25 ++++++++++++++++++++-----
services/context/repo.go | 2 +-
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/routers/common/middleware.go b/routers/common/middleware.go
index 59e59b8d3f..856c10e678 100644
--- a/routers/common/middleware.go
+++ b/routers/common/middleware.go
@@ -92,10 +92,9 @@ func stripSlashesMiddleware(next http.Handler) http.Handler {
prevWasSlash = chr == '/'
}
- if rctx == nil {
- req.URL.Path = sanitizedPath.String()
- } else {
- rctx.RoutePath = sanitizedPath.String()
+ req.URL.Path = sanitizedPath.String()
+ if rctx != nil {
+ rctx.RoutePath = req.URL.Path
}
next.ServeHTTP(resp, req)
})
diff --git a/routers/common/middleware_test.go b/routers/common/middleware_test.go
index f16b9374ec..0e111e1261 100644
--- a/routers/common/middleware_test.go
+++ b/routers/common/middleware_test.go
@@ -7,6 +7,9 @@ import (
"net/http/httptest"
"testing"
+ "code.gitea.io/gitea/modules/web"
+
+ chi "github.com/go-chi/chi/v5"
"github.com/stretchr/testify/assert"
)
@@ -43,6 +46,11 @@ func TestStripSlashesMiddleware(t *testing.T) {
inputPath: "/user2//repo1/",
expectedPath: "/user2/repo1",
},
+ {
+ name: "path with slashes in the beginning",
+ inputPath: "https://codeberg.org//user2/repo1/",
+ expectedPath: "/user2/repo1",
+ },
{
name: "path with slashes and query params",
inputPath: "/repo//migrate?service_type=3",
@@ -56,15 +64,22 @@ func TestStripSlashesMiddleware(t *testing.T) {
}
for _, tt := range tests {
- testMiddleware := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ r := web.NewRoute()
+ r.Use(stripSlashesMiddleware)
+
+ called := false
+ r.Get("*", func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, tt.expectedPath, r.URL.Path)
+
+ rctx := chi.RouteContext(r.Context())
+ assert.Equal(t, tt.expectedPath, rctx.RoutePath)
+
+ called = true
})
- // pass the test middleware to validate the changes
- handlerToTest := stripSlashesMiddleware(testMiddleware)
// create a mock request to use
req := httptest.NewRequest("GET", tt.inputPath, nil)
- // call the handler using a mock response recorder
- handlerToTest.ServeHTTP(httptest.NewRecorder(), req)
+ r.ServeHTTP(httptest.NewRecorder(), req)
+ assert.True(t, called)
}
}
diff --git a/services/context/repo.go b/services/context/repo.go
index 462d843bfc..71cf431273 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.Params("*"))), strings.ToLower(ctx.Repo.RepoLink))
+ prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.PathParamRaw("*"))), strings.ToLower(ctx.Repo.RepoLink))
ctx.Redirect(path.Join(
ctx.Repo.RepoLink,