From 92fd3fc4fd369b6a8c0a022a32a80dec2340223a Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Thu, 27 Apr 2023 14:06:45 +0800
Subject: Refactor "route" related code, fix Safari cookie bug (#24330)

Fix #24176

Clean some misuses of route package, clean some legacy FIXMEs

---------

Co-authored-by: Giteabot <teabot@gitea.io>
---
 modules/web/handler.go | 38 ++++++++++----------------------------
 modules/web/route.go   | 20 ++++----------------
 2 files changed, 14 insertions(+), 44 deletions(-)

(limited to 'modules/web')

diff --git a/modules/web/handler.go b/modules/web/handler.go
index 8a44673f12..bfb83820c8 100644
--- a/modules/web/handler.go
+++ b/modules/web/handler.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
-	"strings"
 
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/web/routing"
@@ -131,16 +130,22 @@ func hasResponseBeenWritten(argsIn []reflect.Value) bool {
 // toHandlerProvider converts a handler to a handler provider
 // A handler provider is a function that takes a "next" http.Handler, it can be used as a middleware
 func toHandlerProvider(handler any) func(next http.Handler) http.Handler {
-	if hp, ok := handler.(func(next http.Handler) http.Handler); ok {
-		return hp
-	}
-
 	funcInfo := routing.GetFuncInfo(handler)
 	fn := reflect.ValueOf(handler)
 	if fn.Type().Kind() != reflect.Func {
 		panic(fmt.Sprintf("handler must be a function, but got %s", fn.Type()))
 	}
 
+	if hp, ok := handler.(func(next http.Handler) http.Handler); ok {
+		return func(next http.Handler) http.Handler {
+			h := hp(next) // this handle could be dynamically generated, so we can't use it for debug info
+			return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+				routing.UpdateFuncInfo(req.Context(), funcInfo)
+				h.ServeHTTP(resp, req)
+			})
+		}
+	}
+
 	provider := func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(respOrig http.ResponseWriter, req *http.Request) {
 			// wrap the response writer to check whether the response has been written
@@ -175,26 +180,3 @@ func toHandlerProvider(handler any) func(next http.Handler) http.Handler {
 	provider(nil).ServeHTTP(nil, nil) // do a pre-check to make sure all arguments and return values are supported
 	return provider
 }
-
-// MiddlewareWithPrefix wraps a handler function at a prefix, and make it as a middleware
-// TODO: this design is incorrect, the asset handler should not be a middleware
-func MiddlewareWithPrefix(pathPrefix string, middleware func(handler http.Handler) http.Handler, handlerFunc http.HandlerFunc) func(next http.Handler) http.Handler {
-	funcInfo := routing.GetFuncInfo(handlerFunc)
-	handler := http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
-		routing.UpdateFuncInfo(req.Context(), funcInfo)
-		handlerFunc(resp, req)
-	})
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
-			if !strings.HasPrefix(req.URL.Path, pathPrefix) {
-				next.ServeHTTP(resp, req)
-				return
-			}
-			if middleware != nil {
-				middleware(handler).ServeHTTP(resp, req)
-			} else {
-				handler.ServeHTTP(resp, req)
-			}
-		})
-	}
-}
diff --git a/modules/web/route.go b/modules/web/route.go
index fe35880849..6fd60f4ca7 100644
--- a/modules/web/route.go
+++ b/modules/web/route.go
@@ -44,23 +44,13 @@ type Route struct {
 // NewRoute creates a new route
 func NewRoute() *Route {
 	r := chi.NewRouter()
-	return &Route{
-		R:              r,
-		curGroupPrefix: "",
-		curMiddlewares: []interface{}{},
-	}
+	return &Route{R: r}
 }
 
 // Use supports two middlewares
 func (r *Route) Use(middlewares ...interface{}) {
-	if r.curGroupPrefix != "" {
-		// FIXME: this behavior is incorrect, should use "With" instead
-		r.curMiddlewares = append(r.curMiddlewares, middlewares...)
-	} else {
-		// FIXME: another misuse, the "Use" with empty middlewares is called after "Mount"
-		for _, m := range middlewares {
-			r.R.Use(toHandlerProvider(m))
-		}
+	for _, m := range middlewares {
+		r.R.Use(toHandlerProvider(m))
 	}
 }
 
@@ -116,9 +106,7 @@ func (r *Route) Methods(method, pattern string, h []any) {
 
 // Mount attaches another Route along ./pattern/*
 func (r *Route) Mount(pattern string, subR *Route) {
-	middlewares := make([]interface{}, len(r.curMiddlewares))
-	copy(middlewares, r.curMiddlewares)
-	subR.Use(middlewares...)
+	subR.Use(r.curMiddlewares...)
 	r.R.Mount(r.getPattern(pattern), subR.R)
 }
 
-- 
cgit v1.2.3