From 547c173dabb10ec973ce9af24243072466960d6e Mon Sep 17 00:00:00 2001 From: Sandeep Bhat Date: Sat, 4 Mar 2023 19:01:24 +0530 Subject: [PATCH] Support sanitising the URL by removing extra slashes in the URL (#21333) Changes in this PR : Strips incoming request URL of additional slashes (/). For example an input like `https://git.data.coop//halfd/new-website.git` is translated to `https://git.data.coop/halfd/new-website.git` Fixes https://github.com/go-gitea/gitea/issues/20462 Fix #23242 --------- Co-authored-by: zeripath Co-authored-by: Jason Song Co-authored-by: Lunny Xiao --- routers/common/middleware.go | 35 +++++++++++++++- routers/common/middleware_test.go | 70 +++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 routers/common/middleware_test.go diff --git a/routers/common/middleware.go b/routers/common/middleware.go index 4f9d43c362..2abdcb583d 100644 --- a/routers/common/middleware.go +++ b/routers/common/middleware.go @@ -16,7 +16,7 @@ import ( "code.gitea.io/gitea/modules/web/routing" "github.com/chi-middleware/proxy" - "github.com/go-chi/chi/v5/middleware" + chi "github.com/go-chi/chi/v5" ) // Middlewares returns common middlewares @@ -48,7 +48,8 @@ func Middlewares() []func(http.Handler) http.Handler { handlers = append(handlers, proxy.ForwardedHeaders(opt)) } - handlers = append(handlers, middleware.StripSlashes) + // Strip slashes. + handlers = append(handlers, stripSlashesMiddleware) if !setting.Log.DisableRouterLog { handlers = append(handlers, routing.NewLoggerHandler()) @@ -81,3 +82,33 @@ func Middlewares() []func(http.Handler) http.Handler { }) return handlers } + +func stripSlashesMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + var urlPath string + rctx := chi.RouteContext(req.Context()) + if rctx != nil && rctx.RoutePath != "" { + urlPath = rctx.RoutePath + } else if req.URL.RawPath != "" { + urlPath = req.URL.RawPath + } else { + urlPath = req.URL.Path + } + + sanitizedPath := &strings.Builder{} + prevWasSlash := false + for _, chr := range strings.TrimRight(urlPath, "/") { + if chr != '/' || !prevWasSlash { + sanitizedPath.WriteRune(chr) + } + prevWasSlash = chr == '/' + } + + if rctx == nil { + req.URL.Path = sanitizedPath.String() + } else { + rctx.RoutePath = sanitizedPath.String() + } + next.ServeHTTP(resp, req) + }) +} diff --git a/routers/common/middleware_test.go b/routers/common/middleware_test.go new file mode 100644 index 0000000000..f16b9374ec --- /dev/null +++ b/routers/common/middleware_test.go @@ -0,0 +1,70 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package common + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStripSlashesMiddleware(t *testing.T) { + type test struct { + name string + expectedPath string + inputPath string + } + + tests := []test{ + { + name: "path with multiple slashes", + inputPath: "https://github.com///go-gitea//gitea.git", + expectedPath: "/go-gitea/gitea.git", + }, + { + name: "path with no slashes", + inputPath: "https://github.com/go-gitea/gitea.git", + expectedPath: "/go-gitea/gitea.git", + }, + { + name: "path with slashes in the middle", + inputPath: "https://git.data.coop//halfd/new-website.git", + expectedPath: "/halfd/new-website.git", + }, + { + name: "path with slashes in the middle", + inputPath: "https://git.data.coop//halfd/new-website.git", + expectedPath: "/halfd/new-website.git", + }, + { + name: "path with slashes in the end", + inputPath: "/user2//repo1/", + expectedPath: "/user2/repo1", + }, + { + name: "path with slashes and query params", + inputPath: "/repo//migrate?service_type=3", + expectedPath: "/repo/migrate", + }, + { + name: "path with encoded slash", + inputPath: "/user2/%2F%2Frepo1", + expectedPath: "/user2/%2F%2Frepo1", + }, + } + + for _, tt := range tests { + testMiddleware := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, tt.expectedPath, r.URL.Path) + }) + + // 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) + } +}