Improve assets handler middleware (#15961)

* Use route to serve assets but not middleware

* Fix build error with bindata tag

* convert path to absolute

* fix build

* reduce function stack

* Add tests for assets

* Remove test for assets because they are not generated

* Use a http function to serve assets

* Still use middleware to serve assets then less middleware stack for assets

* Move serveContent to original position

* remove unnecessary blank line change

* Fix bug for /assets* requests

* clean code

Co-authored-by: zeripath <art27@cantab.net>

2 files changed, 29 insertions, 42 deletions

diff --git a/routers/routes/install.go b/routers/routes/install.go
index 026d92b13e..18e74f005f 100644
--- a/routers/routes/install.go
+++ b/routers/routes/install.go
@@ -81,6 +81,11 @@ func InstallRoutes() *web.Route {
 		r.Use(middle)
 	}
 
+	r.Use(public.AssetsHandler(&public.Options{
+		Directory: path.Join(setting.StaticRootPath, "public"),
+		Prefix:    "/assets",
+	}))
+
 	r.Use(session.Sessioner(session.Options{
 		Provider:       setting.SessionConfig.Provider,
 		ProviderConfig: setting.SessionConfig.ProviderConfig,
@@ -93,20 +98,6 @@ func InstallRoutes() *web.Route {
 	}))
 
 	r.Use(installRecovery())
-
-	r.Use(public.Custom(
-		&public.Options{
-			SkipLogging: setting.DisableRouterLog,
-		},
-	))
-	r.Use(public.Static(
-		&public.Options{
-			Directory:   path.Join(setting.StaticRootPath, "public"),
-			SkipLogging: setting.DisableRouterLog,
-			Prefix:      "/assets",
-		},
-	))
-
 	r.Use(routers.InstallInit)
 	r.Get("/", routers.Install)
 	r.Post("/", web.Bind(forms.InstallForm{}), routers.InstallPost)
diff --git a/routers/routes/web.go b/routers/routes/web.go
index 008c745d6e..cc65ad6d9f 100644
--- a/routers/routes/web.go
+++ b/routers/routes/web.go
@@ -113,6 +113,8 @@ func commonMiddlewares() []func(http.Handler) http.Handler {
 	return handlers
 }
 
+var corsHandler func(http.Handler) http.Handler
+
 // NormalRoutes represents non install routes
 func NormalRoutes() *web.Route {
 	r := web.NewRoute()
@@ -120,6 +122,21 @@ func NormalRoutes() *web.Route {
 		r.Use(middle)
 	}
 
+	if setting.CORSConfig.Enabled {
+		corsHandler = cors.Handler(cors.Options{
+			//Scheme:           setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
+			AllowedOrigins: setting.CORSConfig.AllowDomain,
+			//setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
+			AllowedMethods:   setting.CORSConfig.Methods,
+			AllowCredentials: setting.CORSConfig.AllowCredentials,
+			MaxAge:           int(setting.CORSConfig.MaxAge.Seconds()),
+		})
+	} else {
+		corsHandler = func(next http.Handler) http.Handler {
+			return next
+		}
+	}
+
 	r.Mount("/", WebRoutes())
 	r.Mount("/api/v1", apiv1.Routes())
 	r.Mount("/api/internal", private.Routes())
@@ -130,6 +147,12 @@ func NormalRoutes() *web.Route {
 func WebRoutes() *web.Route {
 	routes := web.NewRoute()
 
+	routes.Use(public.AssetsHandler(&public.Options{
+		Directory:   path.Join(setting.StaticRootPath, "public"),
+		Prefix:      "/assets",
+		CorsHandler: corsHandler,
+	}))
+
 	routes.Use(session.Sessioner(session.Options{
 		Provider:       setting.SessionConfig.Provider,
 		ProviderConfig: setting.SessionConfig.ProviderConfig,
@@ -143,22 +166,6 @@ func WebRoutes() *web.Route {
 
 	routes.Use(Recovery())
 
-	// TODO: we should consider if there is a way to mount these using r.Route as at present
-	// these two handlers mean that every request has to hit these "filesystems" twice
-	// before finally getting to the router. It allows them to override any matching router below.
-	routes.Use(public.Custom(
-		&public.Options{
-			SkipLogging: setting.DisableRouterLog,
-		},
-	))
-	routes.Use(public.Static(
-		&public.Options{
-			Directory:   path.Join(setting.StaticRootPath, "public"),
-			SkipLogging: setting.DisableRouterLog,
-			Prefix:      "/assets",
-		},
-	))
-
 	// We use r.Route here over r.Use because this prevents requests that are not for avatars having to go through this additional handler
 	routes.Route("/avatars/*", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
 	routes.Route("/repo-avatars/*", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
@@ -348,18 +355,7 @@ func RegisterRoutes(m *web.Route) {
 		m.Post("/authorize", bindIgnErr(forms.AuthorizationForm{}), user.AuthorizeOAuth)
 	}, ignSignInAndCsrf, reqSignIn)
 	m.Get("/login/oauth/userinfo", ignSignInAndCsrf, user.InfoOAuth)
-	if setting.CORSConfig.Enabled {
-		m.Post("/login/oauth/access_token", cors.Handler(cors.Options{
-			//Scheme:           setting.CORSConfig.Scheme, // FIXME: the cors middleware needs scheme option
-			AllowedOrigins: setting.CORSConfig.AllowDomain,
-			//setting.CORSConfig.AllowSubdomain // FIXME: the cors middleware needs allowSubdomain option
-			AllowedMethods:   setting.CORSConfig.Methods,
-			AllowCredentials: setting.CORSConfig.AllowCredentials,
-			MaxAge:           int(setting.CORSConfig.MaxAge.Seconds()),
-		}), bindIgnErr(forms.AccessTokenForm{}), ignSignInAndCsrf, user.AccessTokenOAuth)
-	} else {
-		m.Post("/login/oauth/access_token", bindIgnErr(forms.AccessTokenForm{}), ignSignInAndCsrf, user.AccessTokenOAuth)
-	}
+	m.Post("/login/oauth/access_token", corsHandler, bindIgnErr(forms.AccessTokenForm{}), ignSignInAndCsrf, user.AccessTokenOAuth)
 
 	m.Group("/user/settings", func() {
 		m.Get("", userSetting.Profile)