From b471b7e83584fa5b02773532e58760d99678f036 Mon Sep 17 00:00:00 2001 From: Tobias Weingartner Date: Tue, 15 Mar 2016 23:11:19 -0700 Subject: [PATCH] Fixup mime middleware to use a map and error on duplicate extensions. - The mime middleware used filepath where it should arguably use path. - Changed the configuration to use a map instead of scanning an array during every request. The map is static (after configuration), so should be fine for concurrent access. - Catch duplicate extensions within a configuration and error out. - Add tests for new error case. --- caddy/setup/mime.go | 17 ++++++++++------- caddy/setup/mime_test.go | 5 +++++ middleware/mime/mime.go | 35 +++++++++++++---------------------- middleware/mime/mime_test.go | 9 ++------- 4 files changed, 30 insertions(+), 36 deletions(-) diff --git a/caddy/setup/mime.go b/caddy/setup/mime.go index 760056eba..59667dc36 100644 --- a/caddy/setup/mime.go +++ b/caddy/setup/mime.go @@ -20,8 +20,8 @@ func Mime(c *Controller) (middleware.Middleware, error) { }, nil } -func mimeParse(c *Controller) ([]mime.Config, error) { - var configs []mime.Config +func mimeParse(c *Controller) (mime.Config, error) { + configs := mime.Config{} for c.Next() { // At least one extension is required @@ -29,22 +29,22 @@ func mimeParse(c *Controller) ([]mime.Config, error) { args := c.RemainingArgs() switch len(args) { case 2: - if err := validateExt(args[0]); err != nil { + if err := validateExt(configs, args[0]); err != nil { return configs, err } - configs = append(configs, mime.Config{Ext: args[0], ContentType: args[1]}) + configs[args[0]] = args[1] case 1: return configs, c.ArgErr() case 0: for c.NextBlock() { ext := c.Val() - if err := validateExt(ext); err != nil { + if err := validateExt(configs, ext); err != nil { return configs, err } if !c.NextArg() { return configs, c.ArgErr() } - configs = append(configs, mime.Config{Ext: ext, ContentType: c.Val()}) + configs[ext] = c.Val() } } @@ -54,9 +54,12 @@ func mimeParse(c *Controller) ([]mime.Config, error) { } // validateExt checks for valid file name extension. -func validateExt(ext string) error { +func validateExt(configs mime.Config, ext string) error { if !strings.HasPrefix(ext, ".") { return fmt.Errorf(`mime: invalid extension "%v" (must start with dot)`, ext) } + if _, ok := configs[ext]; ok { + return fmt.Errorf(`mime: duplicate extension "%v" found`, ext) + } return nil } diff --git a/caddy/setup/mime_test.go b/caddy/setup/mime_test.go index 7f8d8de68..7b11f3d57 100644 --- a/caddy/setup/mime_test.go +++ b/caddy/setup/mime_test.go @@ -42,6 +42,11 @@ func TestMime(t *testing.T) { .html text/html .txt text/plain } `, false}, + {`mime { + .foo text/foo + .bar text/bar + .foo text/foobar + } `, true}, {`mime { .html text/html } `, false}, {`mime { .html } `, true}, diff --git a/middleware/mime/mime.go b/middleware/mime/mime.go index cde699ff3..6990c596d 100644 --- a/middleware/mime/mime.go +++ b/middleware/mime/mime.go @@ -2,40 +2,31 @@ package mime import ( "net/http" - "path/filepath" + "path" "github.com/mholt/caddy/middleware" ) -// Config represent a mime config. -type Config struct { - Ext string - ContentType string -} - -// SetContent sets the Content-Type header of the request if the request path -// is supported. -func (c Config) SetContent(w http.ResponseWriter, r *http.Request) bool { - ext := filepath.Ext(r.URL.Path) - if ext != c.Ext { - return false - } - w.Header().Set("Content-Type", c.ContentType) - return true -} +// Config represent a mime config. Map from extension to mime-type. +// Note, this should be safe with concurrent read access, as this is +// not modified concurrently. +type Config map[string]string // Mime sets Content-Type header of requests based on configurations. type Mime struct { Next middleware.Handler - Configs []Config + Configs Config } // ServeHTTP implements the middleware.Handler interface. func (e Mime) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { - for _, c := range e.Configs { - if ok := c.SetContent(w, r); ok { - break - } + + // Get a clean /-path, grab the extension + ext := path.Ext(path.Clean(r.URL.Path)) + + if contentType, ok := e.Configs[ext]; ok { + w.Header().Set("Content-Type", contentType) } + return e.Next.ServeHTTP(w, r) } diff --git a/middleware/mime/mime_test.go b/middleware/mime/mime_test.go index e70233032..4010b0aef 100644 --- a/middleware/mime/mime_test.go +++ b/middleware/mime/mime_test.go @@ -11,18 +11,13 @@ import ( func TestMimeHandler(t *testing.T) { - mimes := map[string]string{ + mimes := Config{ ".html": "text/html", ".txt": "text/plain", ".swf": "application/x-shockwave-flash", } - var configs []Config - for ext, contentType := range mimes { - configs = append(configs, Config{Ext: ext, ContentType: contentType}) - } - - m := Mime{Configs: configs} + m := Mime{Configs: mimes} w := httptest.NewRecorder() exts := []string{