fix: always set stripped slashes on http request

- 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
751a3da979/services/context/context.go (L115)
using the value of the http request on not that was set in the Chi context.
This commit is contained in:
Gusted 2025-02-09 11:50:43 +01:00
parent 751a3da979
commit cf157ab360
No known key found for this signature in database
GPG key ID: FD821B732837125F
3 changed files with 24 additions and 10 deletions

View file

@ -95,10 +95,9 @@ func stripSlashesMiddleware(next http.Handler) http.Handler {
prevWasSlash = chr == '/' prevWasSlash = chr == '/'
} }
if rctx == nil { req.URL.Path = sanitizedPath.String()
req.URL.Path = sanitizedPath.String() if rctx != nil {
} else { rctx.RoutePath = req.URL.Path
rctx.RoutePath = sanitizedPath.String()
} }
next.ServeHTTP(resp, req) next.ServeHTTP(resp, req)
}) })

View file

@ -7,6 +7,9 @@ import (
"net/http/httptest" "net/http/httptest"
"testing" "testing"
"code.gitea.io/gitea/modules/web"
chi "github.com/go-chi/chi/v5"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -43,6 +46,11 @@ func TestStripSlashesMiddleware(t *testing.T) {
inputPath: "/user2//repo1/", inputPath: "/user2//repo1/",
expectedPath: "/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", name: "path with slashes and query params",
inputPath: "/repo//migrate?service_type=3", inputPath: "/repo//migrate?service_type=3",
@ -56,15 +64,22 @@ func TestStripSlashesMiddleware(t *testing.T) {
} }
for _, tt := range tests { 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) 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 // create a mock request to use
req := httptest.NewRequest("GET", tt.inputPath, nil) req := httptest.NewRequest("GET", tt.inputPath, nil)
// call the handler using a mock response recorder r.ServeHTTP(httptest.NewRecorder(), req)
handlerToTest.ServeHTTP(httptest.NewRecorder(), req) assert.True(t, called)
} }
} }

View file

@ -1058,7 +1058,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
if refType == RepoRefLegacy { if refType == RepoRefLegacy {
// redirect from old URL scheme to new URL scheme // 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.Redirect(path.Join(
ctx.Repo.RepoLink, ctx.Repo.RepoLink,