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

Author: lunny

modules/public/public.go

@@ -7,6 +7,7 @@ package public
 import (
 	"net/http"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -17,12 +18,13 @@ import (
 
 // Options represents the available options to configure the handler.
 type Options struct {
-	Directory string
-	Prefix    string
+	Directory   string
+	Prefix      string
+	CorsHandler func(http.Handler) http.Handler
 }
 
 // AssetsHandler implements the static handler for serving custom or original assets.
-func AssetsHandler(opts *Options) func(resp http.ResponseWriter, req *http.Request) {
+func AssetsHandler(opts *Options) func(next http.Handler) http.Handler {
 	var custPath = filepath.Join(setting.CustomPath, "public")
 	if !filepath.IsAbs(custPath) {
 		custPath = filepath.Join(setting.AppWorkPath, custPath)
@@ -31,19 +33,55 @@ func AssetsHandler(opts *Options) func(resp http.ResponseWriter, req *http.Reque
 	if !filepath.IsAbs(opts.Directory) {
 		opts.Directory = filepath.Join(setting.AppWorkPath, opts.Directory)
 	}
+	if opts.Prefix == "" {
+		opts.Prefix = "/"
+	}
 
-	return func(resp http.ResponseWriter, req *http.Request) {
-		// custom files
-		if opts.handle(resp, req, http.Dir(custPath), opts.Prefix) {
-			return
-		}
-
-		// internal files
-		if opts.handle(resp, req, fileSystem(opts.Directory), opts.Prefix) {
-			return
-		}
-
-		resp.WriteHeader(404)
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+			if !strings.HasPrefix(req.URL.Path, opts.Prefix) {
+				next.ServeHTTP(resp, req)
+				return
+			}
+			if req.Method != "GET" && req.Method != "HEAD" {
+				resp.WriteHeader(http.StatusNotFound)
+				return
+			}
+
+			file := req.URL.Path
+			file = file[len(opts.Prefix):]
+			if len(file) == 0 {
+				resp.WriteHeader(http.StatusNotFound)
+				return
+			}
+			if !strings.HasPrefix(file, "/") {
+				next.ServeHTTP(resp, req)
+				return
+			}
+
+			var written bool
+			if opts.CorsHandler != nil {
+				written = true
+				opts.CorsHandler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+					written = false
+				})).ServeHTTP(resp, req)
+			}
+			if written {
+				return
+			}
+
+			// custom files
+			if opts.handle(resp, req, http.Dir(custPath), file) {
+				return
+			}
+
+			// internal files
+			if opts.handle(resp, req, fileSystem(opts.Directory), file) {
+				return
+			}
+
+			resp.WriteHeader(http.StatusNotFound)
+		})
 	}
 }
 
@@ -57,44 +95,29 @@ func parseAcceptEncoding(val string) map[string]bool {
 	return types
 }
 
-func (opts *Options) handle(w http.ResponseWriter, req *http.Request, fs http.FileSystem, prefix string) bool {
-	if req.Method != "GET" && req.Method != "HEAD" {
-		return false
-	}
-
-	file := req.URL.Path
-	// if we have a prefix, filter requests by stripping the prefix
-	if prefix != "" {
-		if !strings.HasPrefix(file, prefix) {
-			return false
-		}
-		file = file[len(prefix):]
-		if file != "" && file[0] != '/' {
-			return false
-		}
-	}
-
-	f, err := fs.Open(file)
+func (opts *Options) handle(w http.ResponseWriter, req *http.Request, fs http.FileSystem, file string) bool {
+	// use clean to keep the file is a valid path with no . or ..
+	f, err := fs.Open(path.Clean(file))
 	if err != nil {
 		if os.IsNotExist(err) {
 			return false
 		}
-		w.WriteHeader(500)
+		w.WriteHeader(http.StatusInternalServerError)
 		log.Error("[Static] Open %q failed: %v", file, err)
 		return true
 	}
 	defer f.Close()
 
 	fi, err := f.Stat()
 	if err != nil {
-		w.WriteHeader(500)
+		w.WriteHeader(http.StatusInternalServerError)
 		log.Error("[Static] %q exists, but fails to open: %v", file, err)
 		return true
 	}
 
 	// Try to serve index file
 	if fi.IsDir() {
-		w.WriteHeader(404)
+		w.WriteHeader(http.StatusNotFound)
 		return true
 	}
 

modules/public/static.go

@@ -41,7 +41,7 @@ func serveContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modt
 				_, err := rd.Seek(0, io.SeekStart) // rewind to output whole file
 				if err != nil {
 					log.Error("rd.Seek error: %v", err)
-					http.Error(w, http.StatusText(500), 500)
+					http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 					return
 				}
 			}

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,
@@ -95,11 +100,6 @@ func InstallRoutes() *web.Route {
 	r.Use(installRecovery())
 	r.Use(routers.InstallInit)
 
-	r.Route("/assets/*", "GET, HEAD", corsHandler, public.AssetsHandler(&public.Options{
-		Directory: path.Join(setting.StaticRootPath, "public"),
-		Prefix:    "/assets",
-	}))
-
 	r.Get("/", routers.Install)
 	r.Post("/", web.Bind(forms.InstallForm{}), routers.InstallPost)
 	r.NotFound(func(w http.ResponseWriter, req *http.Request) {

routers/routes/web.go

@@ -133,9 +133,7 @@ func NormalRoutes() *web.Route {
 		})
 	} else {
 		corsHandler = func(next http.Handler) http.Handler {
-			return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
-				next.ServeHTTP(resp, req)
-			})
+			return next
 		}
 	}
 
@@ -149,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,
@@ -162,11 +166,6 @@ func WebRoutes() *web.Route {
 
 	routes.Use(Recovery())
 
-	routes.Route("/assets/*", "GET, HEAD", corsHandler, public.AssetsHandler(&public.Options{
-		Directory: path.Join(setting.StaticRootPath, "public"),
-		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))
@@ -356,11 +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", corsHandler, 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)