push: Reorder before proxy; and allow zero arguments (cf. #1573)

caddyhttp/httpserver/plugin.go

@@ -475,11 +475,11 @@ var directives = []string{
 	"internal",
 	"pprof",
 	"expvar",
+	"push",
 	"prometheus", // github.com/miekg/caddy-prometheus
 	"proxy",
 	"fastcgi",
 	"cgi", // github.com/jung-kurt/caddy-cgi
-	"push",
 	"websocket",
 	"filemanager", // github.com/hacdias/caddy-filemanager
 	"markdown",

caddyhttp/push/handler.go

@@ -8,22 +8,21 @@ import (
 )
 
 func (h Middleware) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
-
 	pusher, hasPusher := w.(http.Pusher)
 
-	// No Pusher, no cry
+	// no push possible, carry on
 	if !hasPusher {
 		return h.Next.ServeHTTP(w, r)
 	}
 
-	// This is request for the pushed resource - it should not be recursive
+	// check if this is a request for the pushed resource (avoid recursion)
 	if _, exists := r.Header[pushHeader]; exists {
 		return h.Next.ServeHTTP(w, r)
 	}
 
 	headers := h.filterProxiedHeaders(r.Header)
 
-	// Push first
+	// push first
 outer:
 	for _, rule := range h.Rules {
 		if httpserver.Path(r.URL.Path).Matches(rule.Path) {
@@ -33,16 +32,17 @@ outer:
 					Header: h.mergeHeaders(headers, resource.Header),
 				})
 				if pushErr != nil {
-					// If we cannot push (either not supported or concurrent streams are full - break)
+					// if we cannot push (either not supported or concurrent streams are full - break)
 					break outer
 				}
 			}
 		}
 	}
 
-	// Serve later
+	// serve later
 	code, err := h.Next.ServeHTTP(w, r)
 
+	// push resources returned in Link headers from upstream middlewares or proxied apps
 	if links, exists := w.Header()["Link"]; exists {
 		h.servePreloadLinks(pusher, headers, links)
 	}
@@ -51,7 +51,6 @@ outer:
 }
 
 func (h Middleware) servePreloadLinks(pusher http.Pusher, headers http.Header, links []string) {
-outer:
 	for _, link := range links {
 		parts := strings.Split(link, ";")
 
@@ -67,13 +66,12 @@ outer:
 		})
 
 		if err != nil {
-			break outer
+			break
 		}
 	}
 }
 
 func (h Middleware) mergeHeaders(l, r http.Header) http.Header {
-
 	out := http.Header{}
 
 	for k, v := range l {
@@ -90,7 +88,6 @@ func (h Middleware) mergeHeaders(l, r http.Header) http.Header {
 }
 
 func (h Middleware) filterProxiedHeaders(headers http.Header) http.Header {
-
 	filter := http.Header{}
 
 	for _, header := range proxiedHeaders {

caddyhttp/push/setup.go

@@ -45,87 +45,94 @@ func parsePushRules(c *caddy.Controller) ([]Rule, error) {
 	var rules = make(map[string]*Rule)
 
 	for c.NextLine() {
-		if !c.NextArg() {
-			return emptyRules, c.ArgErr()
-		}
-
-		path := c.Val()
-		args := c.RemainingArgs()
-
 		var rule *Rule
 		var resources []Resource
 		var ops []ruleOp
 
-		if existingRule, ok := rules[path]; ok {
-			rule = existingRule
-		} else {
+		parseBlock := func() error {
+			for c.NextBlock() {
+				val := c.Val()
+
+				switch val {
+				case "method":
+					if !c.NextArg() {
+						return c.ArgErr()
+					}
+
+					method := c.Val()
+
+					if err := validateMethod(method); err != nil {
+						return errMethodNotSupported
+					}
+
+					ops = append(ops, setMethodOp(method))
+
+				case "header":
+					args := c.RemainingArgs()
+
+					if len(args) != 2 {
+						return errInvalidHeader
+					}
+
+					if err := validateHeader(args[0]); err != nil {
+						return err
+					}
+
+					ops = append(ops, setHeaderOp(args[0], args[1]))
+				default:
+					resources = append(resources, Resource{
+						Path:   val,
+						Method: http.MethodGet,
+						Header: http.Header{pushHeader: []string{}},
+					})
+				}
+			}
+			return nil
+		}
+
+		args := c.RemainingArgs()
+
+		if len(args) == 0 {
 			rule = new(Rule)
-			rule.Path = path
-			rules[rule.Path] = rule
-		}
+			rule.Path = "/"
+			rules["/"] = rule
+			err := parseBlock()
+			if err != nil {
+				return emptyRules, err
+			}
+		} else {
+			path := args[0]
 
-		for i := 0; i < len(args); i++ {
-			resources = append(resources, Resource{
-				Path:   args[i],
-				Method: http.MethodGet,
-				Header: http.Header{pushHeader: []string{}},
-			})
-		}
+			if existingRule, ok := rules[path]; ok {
+				rule = existingRule
+			} else {
+				rule = new(Rule)
+				rule.Path = path
+				rules[rule.Path] = rule
+			}
 
-		for c.NextBlock() {
-			val := c.Val()
-
-			switch val {
-			case "method":
-				if !c.NextArg() {
-					return emptyRules, c.ArgErr()
-				}
-
-				method := c.Val()
-
-				if err := validateMethod(method); err != nil {
-					return emptyRules, errMethodNotSupported
-				}
-
-				ops = append(ops, setMethodOp(method))
-
-			case "header":
-				args := c.RemainingArgs()
-
-				if len(args) != 2 {
-					return emptyRules, errInvalidHeader
-				}
-
-				if err := validateHeader(args[0]); err != nil {
-					return emptyRules, err
-				}
-
-				ops = append(ops, setHeaderOp(args[0], args[1]))
-
-			default:
+			for i := 1; i < len(args); i++ {
 				resources = append(resources, Resource{
-					Path:   val,
+					Path:   args[i],
 					Method: http.MethodGet,
 					Header: http.Header{pushHeader: []string{}},
 				})
 			}
 
+			err := parseBlock()
+			if err != nil {
+				return emptyRules, err
+			}
 		}
 
 		for _, op := range ops {
 			op(resources)
 		}
-
 		rule.Resources = append(rule.Resources, resources...)
 	}
 
 	var returnRules []Rule
-
-	for path, rule := range rules {
-		if len(rule.Resources) == 0 {
-			return emptyRules, c.Errf("Rule %s has empty push resources list", path)
-		}
-
+	for _, rule := range rules {
 		returnRules = append(returnRules, *rule)
 	}
 
@@ -141,7 +148,6 @@ func setHeaderOp(key, value string) func(resources []Resource) {
 }
 
 func setMethodOp(method string) func(resources []Resource) {
-
 	return func(resources []Resource) {
 		for index := range resources {
 			resources[index].Method = method

caddyhttp/push/setup_test.go

@@ -25,10 +25,10 @@ func TestConfigParse(t *testing.T) {
 		expected  []Rule
 	}{
 		{
-			"ParseInvalidEmptyConfig", `push`, true, []Rule{},
+			"ParseInvalidEmptyConfig", `push`, false, []Rule{{Path: "/"}},
 		},
 		{
-			"ParseInvalidConfig", `push /index.html`, true, []Rule{},
+			"ParseInvalidConfig", `push /index.html`, false, []Rule{{Path: "/index.html"}},
 		},
 		{
 			"ParseInvalidConfigBlock", `push /index.html /index.css {
@@ -255,7 +255,7 @@ func TestSetupInstalledMiddleware(t *testing.T) {
 
 func TestSetupWithError(t *testing.T) {
 	// given
-	c := caddy.NewTestController("http", `push /index.html`)
+	c := caddy.NewTestController("http", "push {\nmethod\n}")
 
 	// when
 	err := setup(c)