Merge pull request #174 from mholt/gzipfix

gzip: Make it gzip again

Removes mimes filter in favor of extensions filter (might be temporary)

config/setup/gzip.go

@@ -28,27 +28,15 @@ 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
+		// No extra args expected
 		if len(c.RemainingArgs()) > 0 {
 			return configs, c.ArgErr()
 		}
 
 		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("Invalid MIME %v.", m)
-					}
-					mimeFilter.Types.Add(m)
-				}
 			case "ext":
 				exts := c.RemainingArgs()
 				if len(exts) == 0 {
@@ -56,7 +44,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 +54,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 +75,17 @@ 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 {
+		// 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)
-
-			// neither is specified, use default mime types
 		} else {
-			config.Filters = append(config.Filters, gzip.DefaultMIMEFilter())
+			config.Filters = append(config.Filters, gzip.DefaultExtFilter())
 		}
 
 		configs = append(configs, config)

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},
 	}

middleware/gzip/filter.go

@@ -3,7 +3,6 @@ package gzip
 import (
 	"net/http"
 	"path"
-	"strings"
 
 	"github.com/mholt/caddy/middleware"
 )
@@ -15,6 +14,18 @@ type Filter interface {
 	ShouldCompress(*http.Request) bool
 }
 
+// defaultExtensions is the list of default extensions for which to enable gzipping.
+var defaultExtensions = []string{"", ".txt", ".htm", ".html", ".css", ".php", ".js", ".json", ".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
@@ -47,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("Content-Type"))
-}
-
-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{}
 

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("Content-Type", 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

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

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("Content-Type", 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 {