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>
This commit is contained in:
		
							parent
							
								
									d79c8bc302
								
							
						
					
					
						commit
						effad26c0e
					
				
					 6 changed files with 108 additions and 167 deletions
				
			
		|  | @ -35,6 +35,8 @@ func TestLinksNoLogin(t *testing.T) { | |||
| 		"/user2/repo1", | ||||
| 		"/user2/repo1/projects", | ||||
| 		"/user2/repo1/projects/1", | ||||
| 		"/assets/img/404.png", | ||||
| 		"/assets/img/500.png", | ||||
| 	} | ||||
| 
 | ||||
| 	for _, link := range links { | ||||
|  |  | |||
|  | @ -13,12 +13,11 @@ import ( | |||
| 	"time" | ||||
| ) | ||||
| 
 | ||||
| // Static implements the static handler for serving assets.
 | ||||
| func Static(opts *Options) func(next http.Handler) http.Handler { | ||||
| 	return opts.staticHandler(opts.Directory) | ||||
| func fileSystem(dir string) http.FileSystem { | ||||
| 	return http.Dir(dir) | ||||
| } | ||||
| 
 | ||||
| // ServeContent serve http content
 | ||||
| func ServeContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modtime time.Time, content io.ReadSeeker) { | ||||
| // serveContent serve http content
 | ||||
| func serveContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modtime time.Time, content io.ReadSeeker) { | ||||
| 	http.ServeContent(w, req, fi.Name(), modtime, content) | ||||
| } | ||||
|  |  | |||
|  | @ -5,85 +5,82 @@ | |||
| package public | ||||
| 
 | ||||
| import ( | ||||
| 	"log" | ||||
| 	"net/http" | ||||
| 	"os" | ||||
| 	"path" | ||||
| 	"path/filepath" | ||||
| 	"strings" | ||||
| 
 | ||||
| 	"code.gitea.io/gitea/modules/httpcache" | ||||
| 	"code.gitea.io/gitea/modules/log" | ||||
| 	"code.gitea.io/gitea/modules/setting" | ||||
| ) | ||||
| 
 | ||||
| // Options represents the available options to configure the handler.
 | ||||
| type Options struct { | ||||
| 	Directory   string | ||||
| 	IndexFile   string | ||||
| 	SkipLogging bool | ||||
| 	FileSystem  http.FileSystem | ||||
| 	Prefix      string | ||||
| 	CorsHandler func(http.Handler) http.Handler | ||||
| } | ||||
| 
 | ||||
| // KnownPublicEntries list all direct children in the `public` directory
 | ||||
| var KnownPublicEntries = []string{ | ||||
| 	"css", | ||||
| 	"fonts", | ||||
| 	"img", | ||||
| 	"js", | ||||
| 	"serviceworker.js", | ||||
| 	"vendor", | ||||
| } | ||||
| 
 | ||||
| // Custom implements the static handler for serving custom assets.
 | ||||
| func Custom(opts *Options) func(next http.Handler) http.Handler { | ||||
| 	return opts.staticHandler(path.Join(setting.CustomPath, "public")) | ||||
| } | ||||
| 
 | ||||
| // staticFileSystem implements http.FileSystem interface.
 | ||||
| type staticFileSystem struct { | ||||
| 	dir *http.Dir | ||||
| } | ||||
| 
 | ||||
| func newStaticFileSystem(directory string) staticFileSystem { | ||||
| 	if !filepath.IsAbs(directory) { | ||||
| 		directory = filepath.Join(setting.AppWorkPath, directory) | ||||
| // AssetsHandler implements the static handler for serving custom or original assets.
 | ||||
| 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) | ||||
| 	} | ||||
| 	if !filepath.IsAbs(opts.Directory) { | ||||
| 		opts.Directory = filepath.Join(setting.AppWorkPath, opts.Directory) | ||||
| 	} | ||||
| 	if !strings.HasSuffix(opts.Prefix, "/") { | ||||
| 		opts.Prefix += "/" | ||||
| 	} | ||||
| 	dir := http.Dir(directory) | ||||
| 	return staticFileSystem{&dir} | ||||
| } | ||||
| 
 | ||||
| func (fs staticFileSystem) Open(name string) (http.File, error) { | ||||
| 	return fs.dir.Open(name) | ||||
| } | ||||
| 
 | ||||
| // StaticHandler sets up a new middleware for serving static files in the
 | ||||
| func StaticHandler(dir string, opts *Options) func(next http.Handler) http.Handler { | ||||
| 	return opts.staticHandler(dir) | ||||
| } | ||||
| 
 | ||||
| func (opts *Options) staticHandler(dir string) func(next http.Handler) http.Handler { | ||||
| 	return func(next http.Handler) http.Handler { | ||||
| 		// Defaults
 | ||||
| 		if len(opts.IndexFile) == 0 { | ||||
| 			opts.IndexFile = "index.html" | ||||
| 		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { | ||||
| 			if !strings.HasPrefix(req.URL.Path, opts.Prefix) { | ||||
| 				next.ServeHTTP(resp, req) | ||||
| 				return | ||||
| 			} | ||||
| 		// Normalize the prefix if provided
 | ||||
| 		if opts.Prefix != "" { | ||||
| 			// Ensure we have a leading '/'
 | ||||
| 			if opts.Prefix[0] != '/' { | ||||
| 				opts.Prefix = "/" + opts.Prefix | ||||
| 			} | ||||
| 			// Remove any trailing '/'
 | ||||
| 			opts.Prefix = strings.TrimRight(opts.Prefix, "/") | ||||
| 		} | ||||
| 		if opts.FileSystem == nil { | ||||
| 			opts.FileSystem = newStaticFileSystem(dir) | ||||
| 			if req.Method != "GET" && req.Method != "HEAD" { | ||||
| 				resp.WriteHeader(http.StatusNotFound) | ||||
| 				return | ||||
| 			} | ||||
| 
 | ||||
| 		return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { | ||||
| 			if !opts.handle(w, req, opts) { | ||||
| 				next.ServeHTTP(w, req) | ||||
| 			file := req.URL.Path | ||||
| 			file = file[len(opts.Prefix):] | ||||
| 			if len(file) == 0 { | ||||
| 				resp.WriteHeader(http.StatusNotFound) | ||||
| 				return | ||||
| 			} | ||||
| 			if strings.Contains(file, "\\") { | ||||
| 				resp.WriteHeader(http.StatusBadRequest) | ||||
| 				return | ||||
| 			} | ||||
| 			file = "/" + file | ||||
| 
 | ||||
| 			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) | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|  | @ -98,76 +95,36 @@ func parseAcceptEncoding(val string) map[string]bool { | |||
| 	return types | ||||
| } | ||||
| 
 | ||||
| func (opts *Options) handle(w http.ResponseWriter, req *http.Request, opt *Options) 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 opt.Prefix != "" { | ||||
| 		if !strings.HasPrefix(file, opt.Prefix) { | ||||
| 			return false | ||||
| 		} | ||||
| 		file = file[len(opt.Prefix):] | ||||
| 		if file != "" && file[0] != '/' { | ||||
| 			return false | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	f, err := opt.FileSystem.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 { | ||||
| 		// 404 requests to any known entries in `public`
 | ||||
| 		if path.Base(opts.Directory) == "public" { | ||||
| 			parts := strings.Split(file, "/") | ||||
| 			if len(parts) < 2 { | ||||
| 		if os.IsNotExist(err) { | ||||
| 			return false | ||||
| 		} | ||||
| 			for _, entry := range KnownPublicEntries { | ||||
| 				if entry == parts[1] { | ||||
| 					w.WriteHeader(404) | ||||
| 		w.WriteHeader(http.StatusInternalServerError) | ||||
| 		log.Error("[Static] Open %q failed: %v", file, err) | ||||
| 		return true | ||||
| 	} | ||||
| 			} | ||||
| 		} | ||||
| 		return false | ||||
| 	} | ||||
| 	defer f.Close() | ||||
| 
 | ||||
| 	fi, err := f.Stat() | ||||
| 	if err != nil { | ||||
| 		log.Printf("[Static] %q exists, but fails to open: %v", file, err) | ||||
| 		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() { | ||||
| 		// Redirect if missing trailing slash.
 | ||||
| 		if !strings.HasSuffix(req.URL.Path, "/") { | ||||
| 			http.Redirect(w, req, path.Clean(req.URL.Path+"/"), http.StatusFound) | ||||
| 		w.WriteHeader(http.StatusNotFound) | ||||
| 		return true | ||||
| 	} | ||||
| 
 | ||||
| 		f, err = opt.FileSystem.Open(file) | ||||
| 		if err != nil { | ||||
| 			return false // Discard error.
 | ||||
| 		} | ||||
| 		defer f.Close() | ||||
| 
 | ||||
| 		fi, err = f.Stat() | ||||
| 		if err != nil || fi.IsDir() { | ||||
| 			return false | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if !opt.SkipLogging { | ||||
| 		log.Println("[Static] Serving " + file) | ||||
| 	} | ||||
| 
 | ||||
| 	if httpcache.HandleFileETagCache(req, w, fi) { | ||||
| 		return true | ||||
| 	} | ||||
| 
 | ||||
| 	ServeContent(w, req, fi, fi.ModTime(), f) | ||||
| 	serveContent(w, req, fi, fi.ModTime(), f) | ||||
| 	return true | ||||
| } | ||||
|  |  | |||
|  | @ -20,12 +20,8 @@ import ( | |||
| 	"code.gitea.io/gitea/modules/log" | ||||
| ) | ||||
| 
 | ||||
| // Static implements the static handler for serving assets.
 | ||||
| func Static(opts *Options) func(next http.Handler) http.Handler { | ||||
| 	opts.FileSystem = Assets | ||||
| 	// we don't need to pass the directory, because the directory var is only
 | ||||
| 	// used when in the options there is no FileSystem.
 | ||||
| 	return opts.staticHandler("") | ||||
| func fileSystem(dir string) http.FileSystem { | ||||
| 	return Assets | ||||
| } | ||||
| 
 | ||||
| func Asset(name string) ([]byte, error) { | ||||
|  | @ -59,8 +55,8 @@ func AssetIsDir(name string) (bool, error) { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // ServeContent serve http content
 | ||||
| func ServeContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modtime time.Time, content io.ReadSeeker) { | ||||
| // serveContent serve http content
 | ||||
| func serveContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modtime time.Time, content io.ReadSeeker) { | ||||
| 	encodings := parseAcceptEncoding(req.Header.Get("Accept-Encoding")) | ||||
| 	if encodings["gzip"] { | ||||
| 		if cf, ok := fi.(*vfsgen۰CompressedFileInfo); ok { | ||||
|  | @ -76,7 +72,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 | ||||
| 				} | ||||
| 			} | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue