]> source.dussan.org Git - gitea.git/commitdiff
Improve assets handler middleware (#15961)
authorLunny Xiao <xiaolunwen@gmail.com>
Sun, 30 May 2021 10:25:11 +0000 (18:25 +0800)
committerGitHub <noreply@github.com>
Sun, 30 May 2021 10:25:11 +0000 (18:25 +0800)
* 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>
integrations/links_test.go
modules/public/dynamic.go
modules/public/public.go
modules/public/static.go
routers/routes/install.go
routers/routes/web.go

index 3b9c245fc37c163ca0e0e59415fdb71fe59fde19..03229e10e1222187912edde6b33286d65429f26f 100644 (file)
@@ -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 {
index a57b6363692932fc108f589532c959222451872e..0bfe38bc3f3f16a59cdd8c80bb1f199011c69129 100644 (file)
@@ -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)
 }
index c68f980352ab4ee1d408d620e404a6341c662088..a58709d86fc9c192c95e58f30732b653ccb11a3e 100644 (file)
@@ -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)
-}
+       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
+                       }
 
-// 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)
-}
+                       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
+                       }
 
-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"
-               }
-               // Normalize the prefix if provided
-               if opts.Prefix != "" {
-                       // Ensure we have a leading '/'
-                       if opts.Prefix[0] != '/' {
-                               opts.Prefix = "/" + opts.Prefix
+                       // custom files
+                       if opts.handle(resp, req, http.Dir(custPath), file) {
+                               return
                        }
-                       // Remove any trailing '/'
-                       opts.Prefix = strings.TrimRight(opts.Prefix, "/")
-               }
-               if opts.FileSystem == nil {
-                       opts.FileSystem = newStaticFileSystem(dir)
-               }
 
-               return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-                       if !opts.handle(w, req, opts) {
-                               next.ServeHTTP(w, req)
+                       // 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 {
-                               return false
-                       }
-                       for _, entry := range KnownPublicEntries {
-                               if entry == parts[1] {
-                                       w.WriteHeader(404)
-                                       return true
-                               }
-                       }
+               if os.IsNotExist(err) {
+                       return false
                }
-               return false
+               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 {
-               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)
-                       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)
+               w.WriteHeader(http.StatusNotFound)
+               return true
        }
 
        if httpcache.HandleFileETagCache(req, w, fi) {
                return true
        }
 
-       ServeContent(w, req, fi, fi.ModTime(), f)
+       serveContent(w, req, fi, fi.ModTime(), f)
        return true
 }
index 36cfdbe44f32ed5e3ad1c45d8e63cb30a95e89d5..827dc2a1e0e8d17cbd86ecf364a8fa740b55fb83 100644 (file)
@@ -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
                                }
                        }
index 026d92b13ecec9edd3dcb0ed13c8692ec6c31941..18e74f005fa6d875f020a8914cecc1978b9ae52c 100644 (file)
@@ -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)
index 008c745d6e0909c5dcd4bfa24a6c46a3a0ba51cc..cc65ad6d9fcdb758fed76f481bbf056377d6b168 100644 (file)
@@ -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)