Merge pull request #1042 from mholt/status-middleware

Add 'status' middleware instead of 'status' directive for 'rewrite' middleware

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"

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)

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

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

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) {

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)

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

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

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)
+			}
+		}
+	}
+}

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)
+}

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