From ce2a9cd8f9b0ac6eea60bb3e3abd3d566b9ebcf5 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 17 Apr 2017 22:06:17 -0600 Subject: [PATCH] push: Reorder before proxy; and allow zero arguments (cf. #1573) --- caddyhttp/httpserver/plugin.go | 2 +- caddyhttp/push/handler.go | 17 ++--- caddyhttp/push/setup.go | 126 +++++++++++++++++---------------- caddyhttp/push/setup_test.go | 6 +- 4 files changed, 77 insertions(+), 74 deletions(-) diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go index c53ac767..5a33173d 100644 --- a/caddyhttp/httpserver/plugin.go +++ b/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", diff --git a/caddyhttp/push/handler.go b/caddyhttp/push/handler.go index 6e0c8ee3..a651338e 100644 --- a/caddyhttp/push/handler.go +++ b/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 { diff --git a/caddyhttp/push/setup.go b/caddyhttp/push/setup.go index e13d77d0..b6c33176 100644 --- a/caddyhttp/push/setup.go +++ b/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 diff --git a/caddyhttp/push/setup_test.go b/caddyhttp/push/setup_test.go index 871b2ea4..e1d21ff1 100644 --- a/caddyhttp/push/setup_test.go +++ b/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)