From abf22909f1d3c93a4a2016538431f641f7972c18 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 1 Jul 2015 18:56:30 -0600 Subject: [PATCH 1/3] gzip: Make it gzip again --- config/setup/gzip.go | 33 +++++++++++++++------------------ middleware/gzip/filter.go | 14 +++++++++++++- middleware/gzip/gzip.go | 3 +-- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/config/setup/gzip.go b/config/setup/gzip.go index ea93a128..6c3ad22b 100644 --- a/config/setup/gzip.go +++ b/config/setup/gzip.go @@ -31,7 +31,7 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { mimeFilter := gzip.MIMEFilter{make(gzip.Set)} extFilter := gzip.ExtFilter{make(gzip.Set)} - // no extra args expected + // No extra args expected if len(c.RemainingArgs()) > 0 { return configs, c.ArgErr() } @@ -45,7 +45,7 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { } for _, m := range mimes { if !gzip.ValidMIME(m) { - return configs, fmt.Errorf("Invalid MIME %v.", m) + return configs, fmt.Errorf("gzip: invalid MIME %v", m) } mimeFilter.Types.Add(m) } @@ -56,7 +56,7 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { } for _, e := range exts { if !strings.HasPrefix(e, ".") { - return configs, fmt.Errorf(`Invalid extension %v. Should start with "."`, e) + return configs, fmt.Errorf(`gzip: invalid extension "%v" (must start with dot)`, e) } extFilter.Exts.Add(e) } @@ -66,14 +66,13 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { return configs, c.ArgErr() } for _, p := range paths { + if p == "/" { + return configs, fmt.Errorf(`gzip: cannot exclude path "/" - remove directive entirely instead`) + } if !strings.HasPrefix(p, "/") { - return configs, fmt.Errorf(`Invalid path %v. Should start with "/"`, p) + return configs, fmt.Errorf(`gzip: invalid path "%v" (must start with /)`, p) } pathFilter.IgnoredPaths.Add(p) - // Warn user if / is used - if p == "/" { - fmt.Println("Warning: Paths ignored by gzip includes wildcard(/). No request will be gzipped.\nRemoving gzip directive from Caddyfile is preferred if this is intended.") - } } case "level": if !c.NextArg() { @@ -88,22 +87,20 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { config.Filters = []gzip.Filter{} - // if ignored paths are specified, put in front to filter with path first + // If ignored paths are specified, put in front to filter with path first if len(pathFilter.IgnoredPaths) > 0 { config.Filters = []gzip.Filter{pathFilter} } - // if mime types are specified, use it and ignore extensions - if len(mimeFilter.Types) > 0 { - config.Filters = append(config.Filters, mimeFilter) - - // if extensions are specified, use it - } else if len(extFilter.Exts) > 0 { + // If extensions specified, use it over MIME types (if any). + // Otherwise, if specified, use MIME types. + // Otherwise, use default extensions filter. + if len(extFilter.Exts) > 0 { config.Filters = append(config.Filters, extFilter) - - // neither is specified, use default mime types + } else if len(mimeFilter.Types) > 0 { + config.Filters = append(config.Filters, mimeFilter) } else { - config.Filters = append(config.Filters, gzip.DefaultMIMEFilter()) + config.Filters = append(config.Filters, gzip.DefaultExtFilter()) } configs = append(configs, config) diff --git a/middleware/gzip/filter.go b/middleware/gzip/filter.go index a945fed9..d8fe174d 100644 --- a/middleware/gzip/filter.go +++ b/middleware/gzip/filter.go @@ -15,6 +15,18 @@ type Filter interface { ShouldCompress(*http.Request) bool } +// defaultExtensions is the list of default extensions for which to enable gzipping. +var defaultExtensions = []string{"", ".txt", ".html", ".css", ".json", ".js", ".md", ".xml"} + +// DefaultExtFilter creates an ExtFilter with default extensions. +func DefaultExtFilter() ExtFilter { + m := ExtFilter{Exts: make(Set)} + for _, extension := range defaultExtensions { + m.Exts.Add(extension) + } + return m +} + // ExtFilter is Filter for file name extensions. type ExtFilter struct { // Exts is the file name extensions to accept @@ -72,7 +84,7 @@ func DefaultMIMEFilter() MIMEFilter { // matches any of the registered ones. It returns true if // found and false otherwise. func (m MIMEFilter) ShouldCompress(r *http.Request) bool { - return m.Types.Contains(r.Header.Get("Content-Type")) + return m.Types.Contains(r.Header.Get("Accept")) } func ValidMIME(mime string) bool { diff --git a/middleware/gzip/gzip.go b/middleware/gzip/gzip.go index b9663144..9acec5eb 100644 --- a/middleware/gzip/gzip.go +++ b/middleware/gzip/gzip.go @@ -36,8 +36,7 @@ func (g Gzip) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { outer: for _, c := range g.Configs { - // Check filters to determine if gzipping is permitted for this - // request + // Check filters to determine if gzipping is permitted for this request for _, filter := range c.Filters { if !filter.ShouldCompress(r) { continue outer From 32ef35b9527f682a528c0f04872b0b89b18f4049 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 1 Jul 2015 19:05:31 -0600 Subject: [PATCH 2/3] gzip: Fix tests --- middleware/gzip/filter_test.go | 2 +- middleware/gzip/gzip_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/gzip/filter_test.go b/middleware/gzip/filter_test.go index c3664cdd..616e0612 100644 --- a/middleware/gzip/filter_test.go +++ b/middleware/gzip/filter_test.go @@ -108,7 +108,7 @@ func TestMIMEFilter(t *testing.T) { } for i, m := range mimes { r := urlRequest("file" + m) - r.Header.Set("Content-Type", m) + r.Header.Set("Accept", m) if !filter.ShouldCompress(r) { t.Errorf("Test %v: Should be valid filter", i) } diff --git a/middleware/gzip/gzip_test.go b/middleware/gzip/gzip_test.go index ae0e300c..fd55f7ab 100644 --- a/middleware/gzip/gzip_test.go +++ b/middleware/gzip/gzip_test.go @@ -89,7 +89,7 @@ func TestGzipHandler(t *testing.T) { if err != nil { t.Error(err) } - r.Header.Set("Content-Type", m) + r.Header.Set("Accept", m) r.Header.Set("Accept-Encoding", "gzip") _, err = gz.ServeHTTP(w, r) if err != nil { From b5579ca910a996ca47d6de9f3103afbe18411e31 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 3 Jul 2015 18:13:30 -0600 Subject: [PATCH 3/3] gzip: Remove mimes --- config/setup/gzip.go | 17 +-------------- config/setup/gzip_test.go | 12 ---------- middleware/gzip/filter.go | 36 +----------------------------- middleware/gzip/filter_test.go | 26 ---------------------- middleware/gzip/gzip_test.go | 40 ---------------------------------- 5 files changed, 2 insertions(+), 129 deletions(-) diff --git a/config/setup/gzip.go b/config/setup/gzip.go index 6c3ad22b..933ca592 100644 --- a/config/setup/gzip.go +++ b/config/setup/gzip.go @@ -28,7 +28,6 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { config := gzip.Config{} pathFilter := gzip.PathFilter{make(gzip.Set)} - mimeFilter := gzip.MIMEFilter{make(gzip.Set)} extFilter := gzip.ExtFilter{make(gzip.Set)} // No extra args expected @@ -38,17 +37,6 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { for c.NextBlock() { switch c.Val() { - case "mimes": - mimes := c.RemainingArgs() - if len(mimes) == 0 { - return configs, c.ArgErr() - } - for _, m := range mimes { - if !gzip.ValidMIME(m) { - return configs, fmt.Errorf("gzip: invalid MIME %v", m) - } - mimeFilter.Types.Add(m) - } case "ext": exts := c.RemainingArgs() if len(exts) == 0 { @@ -92,13 +80,10 @@ func gzipParse(c *Controller) ([]gzip.Config, error) { config.Filters = []gzip.Filter{pathFilter} } - // If extensions specified, use it over MIME types (if any). - // Otherwise, if specified, use MIME types. + // Then, if extensions are specified, use those to filter. // Otherwise, use default extensions filter. if len(extFilter.Exts) > 0 { config.Filters = append(config.Filters, extFilter) - } else if len(mimeFilter.Types) > 0 { - config.Filters = append(config.Filters, mimeFilter) } else { config.Filters = append(config.Filters, gzip.DefaultExtFilter()) } diff --git a/config/setup/gzip_test.go b/config/setup/gzip_test.go index 30ef7422..30843e6b 100644 --- a/config/setup/gzip_test.go +++ b/config/setup/gzip_test.go @@ -59,25 +59,13 @@ func TestGzip(t *testing.T) { level 3 } `, false}, - {`gzip { mimes text/html - }`, false}, - {`gzip { mimes text/html application/json - }`, false}, - {`gzip { mimes text/html application/ - }`, true}, - {`gzip { mimes text/html /json - }`, true}, - {`gzip { mimes /json text/html - }`, true}, {`gzip { not /file ext .html level 1 - mimes text/html text/plain } gzip { not /file1 ext .htm level 3 - mimes text/html text/css } `, false}, } diff --git a/middleware/gzip/filter.go b/middleware/gzip/filter.go index d8fe174d..e490722b 100644 --- a/middleware/gzip/filter.go +++ b/middleware/gzip/filter.go @@ -3,7 +3,6 @@ package gzip import ( "net/http" "path" - "strings" "github.com/mholt/caddy/middleware" ) @@ -16,7 +15,7 @@ type Filter interface { } // defaultExtensions is the list of default extensions for which to enable gzipping. -var defaultExtensions = []string{"", ".txt", ".html", ".css", ".json", ".js", ".md", ".xml"} +var defaultExtensions = []string{"", ".txt", ".htm", ".html", ".css", ".php", ".js", ".json", ".md", ".xml"} // DefaultExtFilter creates an ExtFilter with default extensions. func DefaultExtFilter() ExtFilter { @@ -59,39 +58,6 @@ func (p PathFilter) ShouldCompress(r *http.Request) bool { }) } -// MIMEFilter is Filter for request content types. -type MIMEFilter struct { - // Types is the MIME types to accept. - Types Set -} - -// defaultMIMETypes is the list of default MIME types to use. -var defaultMIMETypes = []string{ - "text/plain", "text/html", "text/css", "application/json", "application/javascript", - "text/x-markdown", "text/xml", "application/xml", -} - -// DefaultMIMEFilter creates a MIMEFilter with default types. -func DefaultMIMEFilter() MIMEFilter { - m := MIMEFilter{Types: make(Set)} - for _, mime := range defaultMIMETypes { - m.Types.Add(mime) - } - return m -} - -// ShouldCompress checks if the content type of the request -// matches any of the registered ones. It returns true if -// found and false otherwise. -func (m MIMEFilter) ShouldCompress(r *http.Request) bool { - return m.Types.Contains(r.Header.Get("Accept")) -} - -func ValidMIME(mime string) bool { - s := strings.Split(mime, "/") - return len(s) == 2 && strings.TrimSpace(s[0]) != "" && strings.TrimSpace(s[1]) != "" -} - // Set stores distinct strings. type Set map[string]struct{} diff --git a/middleware/gzip/filter_test.go b/middleware/gzip/filter_test.go index 616e0612..9cc4b8ba 100644 --- a/middleware/gzip/filter_test.go +++ b/middleware/gzip/filter_test.go @@ -100,32 +100,6 @@ func TestPathFilter(t *testing.T) { } } -func TestMIMEFilter(t *testing.T) { - var filter Filter = DefaultMIMEFilter() - _ = filter.(MIMEFilter) - var mimes = []string{ - "text/html", "text/css", "application/json", - } - for i, m := range mimes { - r := urlRequest("file" + m) - r.Header.Set("Accept", m) - if !filter.ShouldCompress(r) { - t.Errorf("Test %v: Should be valid filter", i) - } - } - mimes = []string{ - "image/jpeg", "image/png", - } - filter = DefaultMIMEFilter() - for i, m := range mimes { - r := urlRequest("file" + m) - r.Header.Set("Content-Type", m) - if filter.ShouldCompress(r) { - t.Errorf("Test %v: Should not be valid filter", i) - } - } -} - func urlRequest(url string) *http.Request { r, _ := http.NewRequest("GET", url, nil) return r diff --git a/middleware/gzip/gzip_test.go b/middleware/gzip/gzip_test.go index fd55f7ab..8798f4c5 100644 --- a/middleware/gzip/gzip_test.go +++ b/middleware/gzip/gzip_test.go @@ -76,46 +76,6 @@ func TestGzipHandler(t *testing.T) { t.Error(err) } } - - gz.Configs[0].Filters[1] = DefaultMIMEFilter() - w = httptest.NewRecorder() - gz.Next = nextFunc(true) - var mimes = []string{ - "text/html", "text/css", "application/json", - } - for _, m := range mimes { - url := "/file" - r, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Error(err) - } - r.Header.Set("Accept", m) - r.Header.Set("Accept-Encoding", "gzip") - _, err = gz.ServeHTTP(w, r) - if err != nil { - t.Error(err) - } - } - - w = httptest.NewRecorder() - gz.Next = nextFunc(false) - mimes = []string{ - "image/jpeg", "image/png", - } - for _, m := range mimes { - url := "/file" - r, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Error(err) - } - r.Header.Set("Content-Type", m) - r.Header.Set("Accept-Encoding", "gzip") - _, err = gz.ServeHTTP(w, r) - if err != nil { - t.Error(err) - } - } - } func nextFunc(shouldGzip bool) middleware.Handler {