From 20ee457caeae49ca14f1e3a9eeb5122f24401683 Mon Sep 17 00:00:00 2001 From: Volodymyr Galkin Date: Mon, 29 Aug 2016 17:14:06 +0300 Subject: [PATCH] Add 'status' middleware instead of 'status' directive for 'rewrite' middleware --- caddyhttp/caddyhttp.go | 1 + caddyhttp/caddyhttp_test.go | 2 +- caddyhttp/httpserver/plugin.go | 1 + caddyhttp/rewrite/rewrite.go | 24 +----- caddyhttp/rewrite/rewrite_test.go | 47 +---------- caddyhttp/rewrite/setup.go | 16 +--- caddyhttp/rewrite/setup_test.go | 48 ----------- caddyhttp/status/setup.go | 93 +++++++++++++++++++++ caddyhttp/status/setup_test.go | 131 ++++++++++++++++++++++++++++++ caddyhttp/status/status.go | 59 ++++++++++++++ caddyhttp/status/status_test.go | 76 +++++++++++++++++ 11 files changed, 369 insertions(+), 129 deletions(-) create mode 100644 caddyhttp/status/setup.go create mode 100644 caddyhttp/status/setup_test.go create mode 100644 caddyhttp/status/status.go create mode 100644 caddyhttp/status/status_test.go diff --git a/caddyhttp/caddyhttp.go b/caddyhttp/caddyhttp.go index 86b572132..3f8009fd4 100644 --- a/caddyhttp/caddyhttp.go +++ b/caddyhttp/caddyhttp.go @@ -23,6 +23,7 @@ import ( _ "github.com/mholt/caddy/caddyhttp/redirect" _ "github.com/mholt/caddy/caddyhttp/rewrite" _ "github.com/mholt/caddy/caddyhttp/root" + _ "github.com/mholt/caddy/caddyhttp/status" _ "github.com/mholt/caddy/caddyhttp/templates" _ "github.com/mholt/caddy/caddyhttp/websocket" _ "github.com/mholt/caddy/startupshutdown" diff --git a/caddyhttp/caddyhttp_test.go b/caddyhttp/caddyhttp_test.go index 2d3a411c1..24a2749e7 100644 --- a/caddyhttp/caddyhttp_test.go +++ b/caddyhttp/caddyhttp_test.go @@ -11,7 +11,7 @@ import ( // ensure that the standard plugins are in fact plugged in // and registered properly; this is a quick/naive way to do it. func TestStandardPlugins(t *testing.T) { - numStandardPlugins := 26 // importing caddyhttp plugs in this many plugins + numStandardPlugins := 27 // importing caddyhttp plugs in this many plugins s := caddy.DescribePlugins() if got, want := strings.Count(s, "\n"), numStandardPlugins+5; got != want { t.Errorf("Expected all standard plugins to be plugged in, got:\n%s", s) diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go index 685db0bce..a0114692e 100644 --- a/caddyhttp/httpserver/plugin.go +++ b/caddyhttp/httpserver/plugin.go @@ -412,6 +412,7 @@ var directives = []string{ "search", // github.com/pedronasser/caddy-search "header", "redir", + "status", "cors", // github.com/captncraig/cors/caddy "mime", "basicauth", diff --git a/caddyhttp/rewrite/rewrite.go b/caddyhttp/rewrite/rewrite.go index 3ac8a7935..da086e438 100644 --- a/caddyhttp/rewrite/rewrite.go +++ b/caddyhttp/rewrite/rewrite.go @@ -22,9 +22,6 @@ const ( RewriteIgnored Result = iota // RewriteDone is returned when rewrite is done on request. RewriteDone - // RewriteStatus is returned when rewrite is not needed and status code should be set - // for the request. - RewriteStatus ) // Rewrite is middleware to rewrite request locations internally before being handled. @@ -37,14 +34,9 @@ type Rewrite struct { // ServeHTTP implements the httpserver.Handler interface. func (rw Rewrite) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { if rule := httpserver.ConfigSelector(rw.Rules).Select(r); rule != nil { - switch result := rule.(Rule).Rewrite(rw.FileSys, r); result { - case RewriteStatus: - // only valid for complex rules. - if cRule, ok := rule.(*ComplexRule); ok && cRule.Status != 0 { - return cRule.Status, nil - } - } + rule.(Rule).Rewrite(rw.FileSys, r) } + return rw.Next.ServeHTTP(w, r) } @@ -89,10 +81,6 @@ type ComplexRule struct { // Path to rewrite to To string - // If set, neither performs rewrite nor proceeds - // with request. Only returns code. - Status int - // Extensions to filter by Exts []string @@ -104,7 +92,7 @@ type ComplexRule struct { // NewComplexRule creates a new RegexpRule. It returns an error if regexp // pattern (pattern) or extensions (ext) are invalid. -func NewComplexRule(base, pattern, to string, status int, ext []string, matcher httpserver.RequestMatcher) (*ComplexRule, error) { +func NewComplexRule(base, pattern, to string, ext []string, matcher httpserver.RequestMatcher) (*ComplexRule, error) { // validate regexp if present var r *regexp.Regexp if pattern != "" { @@ -136,7 +124,6 @@ func NewComplexRule(base, pattern, to string, status int, ext []string, matcher return &ComplexRule{ Base: base, To: to, - Status: status, Exts: ext, RequestMatcher: matcher, Regexp: r, @@ -199,11 +186,6 @@ func (r *ComplexRule) Rewrite(fs http.FileSystem, req *http.Request) (re Result) } } - // if status is present, stop rewrite and return it. - if r.Status != 0 { - return RewriteStatus - } - // attempt rewrite return To(fs, req, r.To, replacer) } diff --git a/caddyhttp/rewrite/rewrite_test.go b/caddyhttp/rewrite/rewrite_test.go index 7a8ceadb6..7468a19e0 100644 --- a/caddyhttp/rewrite/rewrite_test.go +++ b/caddyhttp/rewrite/rewrite_test.go @@ -42,7 +42,7 @@ func TestRewrite(t *testing.T) { if s := strings.Split(regexpRule[3], "|"); len(s) > 1 { ext = s[:len(s)-1] } - rule, err := NewComplexRule(regexpRule[0], regexpRule[1], regexpRule[2], 0, ext, httpserver.IfMatcher{}) + rule, err := NewComplexRule(regexpRule[0], regexpRule[1], regexpRule[2], ext, httpserver.IfMatcher{}) if err != nil { t.Fatal(err) } @@ -110,51 +110,6 @@ func TestRewrite(t *testing.T) { i, test.expectedTo, rec.Body.String()) } } - - statusTests := []struct { - status int - base string - to string - regexp string - statusExpected bool - }{ - {400, "/status", "", "", true}, - {400, "/ignore", "", "", false}, - {400, "/", "", "^/ignore", false}, - {400, "/", "", "(.*)", true}, - {400, "/status", "", "", true}, - } - - for i, s := range statusTests { - urlPath := fmt.Sprintf("/status%d", i) - rule, err := NewComplexRule(s.base, s.regexp, s.to, s.status, nil, httpserver.IfMatcher{}) - if err != nil { - t.Fatalf("Test %d: No error expected for rule but found %v", i, err) - } - rw.Rules = []httpserver.HandlerConfig{rule} - req, err := http.NewRequest("GET", urlPath, nil) - if err != nil { - t.Fatalf("Test %d: Could not create HTTP request: %v", i, err) - } - - rec := httptest.NewRecorder() - code, err := rw.ServeHTTP(rec, req) - if err != nil { - t.Fatalf("Test %d: No error expected for handler but found %v", i, err) - } - if s.statusExpected { - if rec.Body.String() != "" { - t.Errorf("Test %d: Expected empty body but found %s", i, rec.Body.String()) - } - if code != s.status { - t.Errorf("Test %d: Expected status code %d found %d", i, s.status, code) - } - } else { - if code != 0 { - t.Errorf("Test %d: Expected no status code found %d", i, code) - } - } - } } func urlPrinter(w http.ResponseWriter, r *http.Request) (int, error) { diff --git a/caddyhttp/rewrite/setup.go b/caddyhttp/rewrite/setup.go index 395f6ca39..71c498e1d 100644 --- a/caddyhttp/rewrite/setup.go +++ b/caddyhttp/rewrite/setup.go @@ -2,7 +2,6 @@ package rewrite import ( "net/http" - "strconv" "strings" "github.com/mholt/caddy" @@ -44,7 +43,6 @@ func rewriteParse(c *caddy.Controller) ([]httpserver.HandlerConfig, error) { var err error var base = "/" var pattern, to string - var status int var ext []string args := c.RemainingArgs() @@ -84,23 +82,15 @@ func rewriteParse(c *caddy.Controller) ([]httpserver.HandlerConfig, error) { return nil, c.ArgErr() } ext = args1 - case "status": - if !c.NextArg() { - return nil, c.ArgErr() - } - status, _ = strconv.Atoi(c.Val()) - if status < 200 || (status > 299 && status < 400) || status > 499 { - return nil, c.Err("status must be 2xx or 4xx") - } default: return nil, c.ArgErr() } } - // ensure to or status is specified - if to == "" && status == 0 { + // ensure to is specified + if to == "" { return nil, c.ArgErr() } - if rule, err = NewComplexRule(base, pattern, to, status, ext, matcher); err != nil { + if rule, err = NewComplexRule(base, pattern, to, ext, matcher); err != nil { return nil, err } rules = append(rules, rule) diff --git a/caddyhttp/rewrite/setup_test.go b/caddyhttp/rewrite/setup_test.go index 0dd149390..fd355102c 100644 --- a/caddyhttp/rewrite/setup_test.go +++ b/caddyhttp/rewrite/setup_test.go @@ -131,54 +131,6 @@ func TestRewriteParse(t *testing.T) { {`rewrite /`, true, []Rule{ &ComplexRule{}, }}, - {`rewrite { - status 500 - }`, true, []Rule{ - &ComplexRule{}, - }}, - {`rewrite { - status 400 - }`, false, []Rule{ - &ComplexRule{Base: "/", Status: 400}, - }}, - {`rewrite { - to /to - status 400 - }`, false, []Rule{ - &ComplexRule{Base: "/", To: "/to", Status: 400}, - }}, - {`rewrite { - status 399 - }`, true, []Rule{ - &ComplexRule{}, - }}, - {`rewrite { - status 200 - }`, false, []Rule{ - &ComplexRule{Base: "/", Status: 200}, - }}, - {`rewrite { - to /to - status 200 - }`, false, []Rule{ - &ComplexRule{Base: "/", To: "/to", Status: 200}, - }}, - {`rewrite { - status 199 - }`, true, []Rule{ - &ComplexRule{}, - }}, - {`rewrite { - status 0 - }`, true, []Rule{ - &ComplexRule{}, - }}, - {`rewrite { - to /to - status 0 - }`, true, []Rule{ - &ComplexRule{}, - }}, {`rewrite { if {path} match / to /to diff --git a/caddyhttp/status/setup.go b/caddyhttp/status/setup.go new file mode 100644 index 000000000..805ba601b --- /dev/null +++ b/caddyhttp/status/setup.go @@ -0,0 +1,93 @@ +package status + +import ( + "strconv" + + "github.com/mholt/caddy" + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +// init registers Status plugin +func init() { + caddy.RegisterPlugin("status", caddy.Plugin{ + ServerType: "http", + Action: setup, + }) +} + +// setup configures new Status middleware instance. +func setup(c *caddy.Controller) error { + rules, err := statusParse(c) + if err != nil { + return err + } + + cfg := httpserver.GetConfig(c) + mid := func(next httpserver.Handler) httpserver.Handler { + return Status{Rules: rules, Next: next} + } + cfg.AddMiddleware(mid) + + return nil +} + +// statusParse parses status directive +func statusParse(c *caddy.Controller) ([]httpserver.HandlerConfig, error) { + var rules []httpserver.HandlerConfig + + for c.Next() { + hadBlock := false + args := c.RemainingArgs() + + switch len(args) { + case 1: + status, err := strconv.Atoi(args[0]) + if err != nil { + return rules, c.Errf("Expecting a numeric status code, got '%s'", args[0]) + } + + for c.NextBlock() { + hadBlock = true + basePath := c.Val() + + for _, cfg := range rules { + rule := cfg.(*Rule) + if rule.Base == basePath { + return rules, c.Errf("Duplicate path: '%s'", basePath) + } + } + + rule := NewRule(basePath, status) + rules = append(rules, rule) + + if c.NextArg() { + return rules, c.ArgErr() + } + } + + if !hadBlock { + return rules, c.ArgErr() + } + case 2: + status, err := strconv.Atoi(args[0]) + if err != nil { + return rules, c.Errf("Expecting a numeric status code, got '%s'", args[0]) + } + + basePath := args[1] + for _, cfg := range rules { + rule := cfg.(*Rule) + if rule.Base == basePath { + return rules, c.Errf("Duplicate path: '%s'", basePath) + } + } + + rule := NewRule(basePath, status) + rules = append(rules, rule) + default: + return rules, c.ArgErr() + } + } + + return rules, nil +} diff --git a/caddyhttp/status/setup_test.go b/caddyhttp/status/setup_test.go new file mode 100644 index 000000000..493d4b906 --- /dev/null +++ b/caddyhttp/status/setup_test.go @@ -0,0 +1,131 @@ +package status + +import ( + "testing" + + "github.com/mholt/caddy" + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +func TestSetup(t *testing.T) { + c := caddy.NewTestController("http", `status 404 /foo`) + err := setup(c) + if err != nil { + t.Errorf("Expected no errors, but got: %v", err) + } + + mids := httpserver.GetConfig(c).Middleware() + if len(mids) == 0 { + t.Fatal("Expected middleware, had 0 instead") + } + + handler := mids[0](httpserver.EmptyNext) + myHandler, ok := handler.(Status) + if !ok { + t.Fatalf("Expected handler to be type Status, got: %#v", + handler) + } + + if !httpserver.SameNext(myHandler.Next, httpserver.EmptyNext) { + t.Error("'Next' field of handler was not set properly") + } + + if len(myHandler.Rules) != 1 { + t.Errorf("Expected handler to have %d rule, has %d instead", 1, len(myHandler.Rules)) + } +} + +func TestStatusParse(t *testing.T) { + tests := []struct { + input string + shouldErr bool + expected []*Rule + }{ + {`status`, true, []*Rule{}}, + {`status /foo`, true, []*Rule{}}, + {`status bar /foo`, true, []*Rule{}}, + {`status 404 /foo bar`, true, []*Rule{}}, + {`status 404 /foo`, false, []*Rule{ + {Base: "/foo", StatusCode: 404}, + }, + }, + {`status { + }`, + true, + []*Rule{}, + }, + {`status 404 { + }`, + true, + []*Rule{}, + }, + {`status 404 { + /foo + /foo + }`, + true, + []*Rule{}, + }, + {`status 404 { + 404 /foo + }`, + true, + []*Rule{}, + }, + {`status 404 { + /foo + /bar + }`, + false, + []*Rule{ + {Base: "/foo", StatusCode: 404}, + {Base: "/bar", StatusCode: 404}, + }, + }, + } + + for i, test := range tests { + actual, err := statusParse(caddy.NewTestController("http", + test.input)) + + if err == nil && test.shouldErr { + t.Errorf("Test %d didn't error, but it should have", i) + } else if err != nil && !test.shouldErr { + t.Errorf("Test %d errored, but it shouldn't have; got '%v'", + i, err) + } else if err != nil && test.shouldErr { + continue + } + + if len(actual) != len(test.expected) { + t.Fatalf("Test %d expected %d rules, but got %d", + i, len(test.expected), len(actual)) + } + + findRule := func(basePath string) (bool, *Rule) { + for _, cfg := range actual { + actualRule := cfg.(*Rule) + + if actualRule.Base == basePath { + return true, actualRule + } + } + + return false, nil + } + + for _, expectedRule := range test.expected { + found, actualRule := findRule(expectedRule.Base) + + if !found { + t.Errorf("Test %d: Missing rule for path '%s'", + i, expectedRule.Base) + } + + if actualRule.StatusCode != expectedRule.StatusCode { + t.Errorf("Test %d: Expected status code %d for path '%s'. Got %d", + i, expectedRule.StatusCode, expectedRule.Base, actualRule.StatusCode) + } + } + } +} diff --git a/caddyhttp/status/status.go b/caddyhttp/status/status.go new file mode 100644 index 000000000..e10a7eef4 --- /dev/null +++ b/caddyhttp/status/status.go @@ -0,0 +1,59 @@ +// Package status is middleware for returning status code for requests +package status + +import ( + "net/http" + + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +// Rule describes status rewriting rule +type Rule struct { + // Base path. Request to this path and sub-paths will be answered with StatusCode + Base string + + // Status code to return + StatusCode int + + // Request matcher + httpserver.RequestMatcher +} + +// NewRule creates new Rule. +func NewRule(basePath string, status int) *Rule { + return &Rule{ + Base: basePath, + StatusCode: status, + RequestMatcher: httpserver.PathMatcher(basePath), + } +} + +// BasePath implements httpserver.HandlerConfig interface +func (rule *Rule) BasePath() string { + return rule.Base +} + +// Status is a middleware to return status code for request +type Status struct { + Rules []httpserver.HandlerConfig + Next httpserver.Handler +} + +// ServeHTTP implements the httpserver.Handler interface +func (status Status) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { + if cfg := httpserver.ConfigSelector(status.Rules).Select(r); cfg != nil { + rule := cfg.(*Rule) + + if rule.StatusCode < 400 { + // There's no ability to return response body -- + // write the response status code in header and signal + // to other handlers that response is already handled + w.WriteHeader(rule.StatusCode) + return 0, nil + } + + return rule.StatusCode, nil + } + + return status.Next.ServeHTTP(w, r) +} diff --git a/caddyhttp/status/status_test.go b/caddyhttp/status/status_test.go new file mode 100644 index 000000000..56048ea08 --- /dev/null +++ b/caddyhttp/status/status_test.go @@ -0,0 +1,76 @@ +package status + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/mholt/caddy/caddyhttp/httpserver" +) + +func TestStatus(t *testing.T) { + status := Status{ + Rules: []httpserver.HandlerConfig{ + NewRule("/foo", http.StatusNotFound), + NewRule("/teapot", http.StatusTeapot), + NewRule("/foo/bar1", http.StatusInternalServerError), + NewRule("/temporary-redirected", http.StatusTemporaryRedirect), + }, + Next: httpserver.HandlerFunc(urlPrinter), + } + + tests := []struct { + path string + statusExpected bool + status int + }{ + {"/foo", true, http.StatusNotFound}, + {"/teapot", true, http.StatusTeapot}, + {"/foo/bar", true, http.StatusNotFound}, + {"/foo/bar1", true, http.StatusInternalServerError}, + {"/someotherpath", false, 0}, + {"/temporary-redirected", false, http.StatusTemporaryRedirect}, + } + + for i, test := range tests { + req, err := http.NewRequest("GET", test.path, nil) + if err != nil { + t.Fatalf("Test %d: Could not create HTTP request: %v", + i, err) + } + + rec := httptest.NewRecorder() + actualStatus, err := status.ServeHTTP(rec, req) + if err != nil { + t.Fatalf("Test %d: Serving request failed with error %v", + i, err) + } + + if test.statusExpected { + if test.status != actualStatus { + t.Errorf("Test %d: Expected status code %d, got %d", + i, test.status, actualStatus) + } + if rec.Body.String() != "" { + t.Errorf("Test %d: Expected empty body, got '%s'", + i, rec.Body.String()) + } + } else { + if test.status != 0 { // Expecting status in response + if test.status != rec.Code { + t.Errorf("Test %d: Expected status code %d, got %d", + i, test.status, rec.Code) + } + } else if rec.Body.String() != test.path { + t.Errorf("Test %d: Expected body '%s', got '%s'", + i, test.path, rec.Body.String()) + } + } + } +} + +func urlPrinter(w http.ResponseWriter, r *http.Request) (int, error) { + fmt.Fprint(w, r.URL.String()) + return 0, nil +}