Condition upgrades (if, if_op) for rewrite, redir (#889)

* checkpoint

* Added RequestMatcher interface. Extract 'if' condition into a RequestMatcher.

* Added tests for IfMatcher

* Minor refactors

* Refactors

* Use if_op

* conform with new 0.9 beta function changes.

caddyhttp/rewrite/condition.go → caddyhttp/httpserver/condition.go

-package rewrite
+package httpserver
 
 import (
 	"fmt"
@@ -6,41 +6,70 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/mholt/caddy/caddyhttp/httpserver"
+	"github.com/mholt/caddy/caddyfile"
 )
 
-// Operators
+// SetupIfMatcher parses `if` or `if_type` in the current dispenser block.
+// It returns a RequestMatcher and an error if any.
+func SetupIfMatcher(c caddyfile.Dispenser) (RequestMatcher, error) {
+	var matcher IfMatcher
+	for c.NextBlock() {
+		switch c.Val() {
+		case "if":
+			args1 := c.RemainingArgs()
+			if len(args1) != 3 {
+				return matcher, c.ArgErr()
+			}
+			ifc, err := newIfCond(args1[0], args1[1], args1[2])
+			if err != nil {
+				return matcher, err
+			}
+			matcher.ifs = append(matcher.ifs, ifc)
+		case "if_op":
+			if !c.NextArg() {
+				return matcher, c.ArgErr()
+			}
+			switch c.Val() {
+			case "and":
+				matcher.isOr = false
+			case "or":
+				matcher.isOr = true
+			default:
+				return matcher, c.ArgErr()
+			}
+		}
+	}
+	return matcher, nil
+}
+
+// operators
 const (
-	Is         = "is"
-	Not        = "not"
-	Has        = "has"
-	NotHas     = "not_has"
-	StartsWith = "starts_with"
-	EndsWith   = "ends_with"
-	Match      = "match"
-	NotMatch   = "not_match"
+	isOp         = "is"
+	notOp        = "not"
+	hasOp        = "has"
+	notHasOp     = "not_has"
+	startsWithOp = "starts_with"
+	endsWithOp   = "ends_with"
+	matchOp      = "match"
+	notMatchOp   = "not_match"
 )
 
 func operatorError(operator string) error {
 	return fmt.Errorf("Invalid operator %v", operator)
 }
 
-func newReplacer(r *http.Request) httpserver.Replacer {
-	return httpserver.NewReplacer(r, nil, "")
-}
-
-// condition is a rewrite condition.
-type condition func(string, string) bool
-
-var conditions = map[string]condition{
-	Is:         isFunc,
-	Not:        notFunc,
-	Has:        hasFunc,
-	NotHas:     notHasFunc,
-	StartsWith: startsWithFunc,
-	EndsWith:   endsWithFunc,
-	Match:      matchFunc,
-	NotMatch:   notMatchFunc,
+// ifCondition is a 'if' condition.
+type ifCondition func(string, string) bool
+
+var ifConditions = map[string]ifCondition{
+	isOp:         isFunc,
+	notOp:        notFunc,
+	hasOp:        hasFunc,
+	notHasOp:     notHasFunc,
+	startsWithOp: startsWithFunc,
+	endsWithOp:   endsWithFunc,
+	matchOp:      matchFunc,
+	notMatchOp:   notMatchFunc,
 }
 
 // isFunc is condition for Is operator.
@@ -95,36 +124,76 @@ func notMatchFunc(a, b string) bool {
 	return !matched
 }
 
-// If is statement for a rewrite condition.
-type If struct {
-	A        string
-	Operator string
-	B        string
+// ifCond is statement for a IfMatcher condition.
+type ifCond struct {
+	a  string
+	op string
+	b  string
+}
+
+// newIfCond creates a new If condition.
+func newIfCond(a, operator, b string) (ifCond, error) {
+	if _, ok := ifConditions[operator]; !ok {
+		return ifCond{}, operatorError(operator)
+	}
+	return ifCond{
+		a:  a,
+		op: operator,
+		b:  b,
+	}, nil
 }
 
 // True returns true if the condition is true and false otherwise.
 // If r is not nil, it replaces placeholders before comparison.
-func (i If) True(r *http.Request) bool {
-	if c, ok := conditions[i.Operator]; ok {
-		a, b := i.A, i.B
+func (i ifCond) True(r *http.Request) bool {
+	if c, ok := ifConditions[i.op]; ok {
+		a, b := i.a, i.b
 		if r != nil {
-			replacer := newReplacer(r)
-			a = replacer.Replace(i.A)
-			b = replacer.Replace(i.B)
+			replacer := NewReplacer(r, nil, "")
+			a = replacer.Replace(i.a)
+			b = replacer.Replace(i.b)
 		}
 		return c(a, b)
 	}
 	return false
 }
 
-// NewIf creates a new If condition.
-func NewIf(a, operator, b string) (If, error) {
-	if _, ok := conditions[operator]; !ok {
-		return If{}, operatorError(operator)
+// IfMatcher is a RequestMatcher for 'if' conditions.
+type IfMatcher struct {
+	ifs  []ifCond // list of If
+	isOr bool     // if true, conditions are 'or' instead of 'and'
+}
+
+// Match satisfies RequestMatcher interface.
+// It returns true if the conditions in m are true.
+func (m IfMatcher) Match(r *http.Request) bool {
+	if m.isOr {
+		return m.Or(r)
 	}
-	return If{
-		A:        a,
-		Operator: operator,
-		B:        b,
-	}, nil
+	return m.And(r)
+}
+
+// And returns true if all conditions in m are true.
+func (m IfMatcher) And(r *http.Request) bool {
+	for _, i := range m.ifs {
+		if !i.True(r) {
+			return false
+		}
+	}
+	return true
+}
+
+// Or returns true if any of the conditions in m is true.
+func (m IfMatcher) Or(r *http.Request) bool {
+	for _, i := range m.ifs {
+		if i.True(r) {
+			return true
+		}
+	}
+	return false
+}
+
+// IfMatcherKeyword returns if k is a keyword for 'if' config block.
+func IfMatcherKeyword(k string) bool {
+	return k == "if" || k == "if_cond"
 }

caddyhttp/rewrite/condition_test.go → caddyhttp/httpserver/condition_test.go

-package rewrite
+package httpserver
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"testing"
+
+	"github.com/mholt/caddy"
 )
 
 func TestConditions(t *testing.T) {
@@ -57,19 +60,19 @@ func TestConditions(t *testing.T) {
 
 	for i, test := range tests {
 		str := strings.Fields(test.condition)
-		ifCond, err := NewIf(str[0], str[1], str[2])
+		ifCond, err := newIfCond(str[0], str[1], str[2])
 		if err != nil {
 			t.Error(err)
 		}
 		isTrue := ifCond.True(nil)
 		if isTrue != test.isTrue {
-			t.Errorf("Test %v: expected %v found %v", i, test.isTrue, isTrue)
+			t.Errorf("Test %d: expected %v found %v", i, test.isTrue, isTrue)
 		}
 	}
 
 	invalidOperators := []string{"ss", "and", "if"}
 	for _, op := range invalidOperators {
-		_, err := NewIf("a", op, "b")
+		_, err := newIfCond("a", op, "b")
 		if err == nil {
 			t.Errorf("Invalid operator %v used, expected error.", op)
 		}
@@ -94,7 +97,7 @@ func TestConditions(t *testing.T) {
 			t.Error(err)
 		}
 		str := strings.Fields(test.condition)
-		ifCond, err := NewIf(str[0], str[1], str[2])
+		ifCond, err := newIfCond(str[0], str[1], str[2])
 		if err != nil {
 			t.Error(err)
 		}
@@ -104,3 +107,159 @@ func TestConditions(t *testing.T) {
 		}
 	}
 }
+
+func TestIfMatcher(t *testing.T) {
+	tests := []struct {
+		conditions []string
+		isOr       bool
+		isTrue     bool
+	}{
+		{
+			[]string{
+				"a is a",
+				"b is b",
+				"c is c",
+			},
+			false,
+			true,
+		},
+		{
+			[]string{
+				"a is b",
+				"b is c",
+				"c is c",
+			},
+			true,
+			true,
+		},
+		{
+			[]string{
+				"a is a",
+				"b is a",
+				"c is c",
+			},
+			false,
+			false,
+		},
+		{
+			[]string{
+				"a is b",
+				"b is c",
+				"c is a",
+			},
+			true,
+			false,
+		},
+		{
+			[]string{},
+			false,
+			true,
+		},
+		{
+			[]string{},
+			true,
+			false,
+		},
+	}
+
+	for i, test := range tests {
+		matcher := IfMatcher{isOr: test.isOr}
+		for _, condition := range test.conditions {
+			str := strings.Fields(condition)
+			ifCond, err := newIfCond(str[0], str[1], str[2])
+			if err != nil {
+				t.Error(err)
+			}
+			matcher.ifs = append(matcher.ifs, ifCond)
+		}
+		isTrue := matcher.Match(nil)
+		if isTrue != test.isTrue {
+			t.Errorf("Test %d: expected %v found %v", i, test.isTrue, isTrue)
+		}
+	}
+}
+
+func TestSetupIfMatcher(t *testing.T) {
+	tests := []struct {
+		input     string
+		shouldErr bool
+		expected  IfMatcher
+	}{
+		{`test {
+			if	a match b
+		 }`, false, IfMatcher{
+			ifs: []ifCond{
+				{a: "a", op: "match", b: "b"},
+			},
+		}},
+		{`test {
+			if a match b
+			if_op or
+		 }`, false, IfMatcher{
+			ifs: []ifCond{
+				{a: "a", op: "match", b: "b"},
+			},
+			isOr: true,
+		}},
+		{`test {
+			if	a match
+		 }`, true, IfMatcher{},
+		},
+		{`test {
+			if	a isnt b
+		 }`, true, IfMatcher{},
+		},
+		{`test {
+			if a match b c
+		 }`, true, IfMatcher{},
+		},
+		{`test {
+			if goal has go
+			if cook not_has go 
+		 }`, false, IfMatcher{
+			ifs: []ifCond{
+				{a: "goal", op: "has", b: "go"},
+				{a: "cook", op: "not_has", b: "go"},
+			},
+		}},
+		{`test {
+			if goal has go
+			if cook not_has go 
+			if_op and
+		 }`, false, IfMatcher{
+			ifs: []ifCond{
+				{a: "goal", op: "has", b: "go"},
+				{a: "cook", op: "not_has", b: "go"},
+			},
+		}},
+		{`test {
+			if goal has go
+			if cook not_has go 
+			if_op not
+		 }`, true, IfMatcher{},
+		},
+	}
+
+	for i, test := range tests {
+		c := caddy.NewTestController("http", test.input)
+		c.Next()
+		matcher, err := SetupIfMatcher(c.Dispenser)
+		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 _, ok := matcher.(IfMatcher); !ok {
+			t.Error("RequestMatcher should be of type IfMatcher")
+		}
+		if err != nil {
+			t.Errorf("Expected no error, but got: %v", err)
+		}
+		if fmt.Sprint(matcher) != fmt.Sprint(test.expected) {
+			t.Errorf("Test %v: Expected %v, found %v", i,
+				fmt.Sprint(test.expected), fmt.Sprint(matcher))
+		}
+	}
+}

caddyhttp/httpserver/middleware.go

@@ -45,6 +45,16 @@ type (
 	// ServeHTTP returns a status code and an error. See Handler
 	// documentation for more information.
 	HandlerFunc func(http.ResponseWriter, *http.Request) (int, error)
+
+	// RequestMatcher checks to see if current request should be handled
+	// by underlying handler.
+	//
+	// TODO The long term plan is to get all middleware implement this
+	// interface and have validation done before requests are dispatched
+	// to each middleware.
+	RequestMatcher interface {
+		Match(r *http.Request) bool
+	}
 )
 
 // ServeHTTP implements the Handler interface.
@@ -135,6 +145,24 @@ func (p Path) Matches(other string) bool {
 	return strings.HasPrefix(strings.ToLower(string(p)), strings.ToLower(other))
 }
 
+// MergeRequestMatchers merges multiple RequestMatchers into one.
+// This allows a middleware to use multiple RequestMatchers.
+func MergeRequestMatchers(matchers ...RequestMatcher) RequestMatcher {
+	return requestMatchers(matchers)
+}
+
+type requestMatchers []RequestMatcher
+
+// Match satisfies RequestMatcher interface.
+func (m requestMatchers) Match(r *http.Request) bool {
+	for _, matcher := range m {
+		if !matcher.Match(r) {
+			return false
+		}
+	}
+	return true
+}
+
 // currentTime, as it is defined here, returns time.Now().
 // It's defined as a variable for mocking time in tests.
 var currentTime = func() time.Time { return time.Now() }

caddyhttp/redirect/redirect.go

@@ -19,7 +19,7 @@ type Redirect struct {
 // ServeHTTP implements the httpserver.Handler interface.
 func (rd Redirect) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	for _, rule := range rd.Rules {
-		if (rule.FromPath == "/" || r.URL.Path == rule.FromPath) && schemeMatches(rule, r) {
+		if (rule.FromPath == "/" || r.URL.Path == rule.FromPath) && schemeMatches(rule, r) && rule.Match(r) {
 			to := httpserver.NewReplacer(r, nil, "").Replace(rule.To)
 			if rule.Meta {
 				safeTo := html.EscapeString(to)
@@ -43,6 +43,7 @@ type Rule struct {
 	FromScheme, FromPath, To string
 	Code                     int
 	Meta                     bool
+	httpserver.RequestMatcher
 }
 
 // Script tag comes first since that will better imitate a redirect in the browser's

caddyhttp/redirect/redirect_test.go

@@ -47,16 +47,16 @@ func TestRedirect(t *testing.T) {
 				return 0, nil
 			}),
 			Rules: []Rule{
-				{FromPath: "/from", To: "/to", Code: http.StatusMovedPermanently},
-				{FromPath: "/a", To: "/b", Code: http.StatusTemporaryRedirect},
+				{FromPath: "/from", To: "/to", Code: http.StatusMovedPermanently, RequestMatcher: httpserver.IfMatcher{}},
+				{FromPath: "/a", To: "/b", Code: http.StatusTemporaryRedirect, RequestMatcher: httpserver.IfMatcher{}},
 
 				// These http and https schemes would never actually be mixed in the same
 				// redirect rule with Caddy because http and https schemes have different listeners,
 				// so they don't share a redirect rule. So although these tests prove something
 				// impossible with Caddy, it's extra bulletproofing at very little cost.
-				{FromScheme: "http", FromPath: "/scheme", To: "https://localhost/scheme", Code: http.StatusMovedPermanently},
-				{FromScheme: "https", FromPath: "/scheme2", To: "http://localhost/scheme2", Code: http.StatusMovedPermanently},
-				{FromScheme: "", FromPath: "/scheme3", To: "https://localhost/scheme3", Code: http.StatusMovedPermanently},
+				{FromScheme: "http", FromPath: "/scheme", To: "https://localhost/scheme", Code: http.StatusMovedPermanently, RequestMatcher: httpserver.IfMatcher{}},
+				{FromScheme: "https", FromPath: "/scheme2", To: "http://localhost/scheme2", Code: http.StatusMovedPermanently, RequestMatcher: httpserver.IfMatcher{}},
+				{FromScheme: "", FromPath: "/scheme3", To: "https://localhost/scheme3", Code: http.StatusMovedPermanently, RequestMatcher: httpserver.IfMatcher{}},
 			},
 		}
 
@@ -90,7 +90,7 @@ func TestRedirect(t *testing.T) {
 func TestParametersRedirect(t *testing.T) {
 	re := Redirect{
 		Rules: []Rule{
-			{FromPath: "/", Meta: false, To: "http://example.com{uri}"},
+			{FromPath: "/", Meta: false, To: "http://example.com{uri}", RequestMatcher: httpserver.IfMatcher{}},
 		},
 	}
 
@@ -108,7 +108,7 @@ func TestParametersRedirect(t *testing.T) {
 
 	re = Redirect{
 		Rules: []Rule{
-			{FromPath: "/", Meta: false, To: "http://example.com/a{path}?b=c&{query}"},
+			{FromPath: "/", Meta: false, To: "http://example.com/a{path}?b=c&{query}", RequestMatcher: httpserver.IfMatcher{}},
 		},
 	}
 
@@ -127,8 +127,8 @@ func TestParametersRedirect(t *testing.T) {
 func TestMetaRedirect(t *testing.T) {
 	re := Redirect{
 		Rules: []Rule{
-			{FromPath: "/whatever", Meta: true, To: "/something"},
-			{FromPath: "/", Meta: true, To: "https://example.com/"},
+			{FromPath: "/whatever", Meta: true, To: "/something", RequestMatcher: httpserver.IfMatcher{}},
+			{FromPath: "/", Meta: true, To: "https://example.com/", RequestMatcher: httpserver.IfMatcher{}},
 		},
 	}
 

caddyhttp/redirect/setup.go

@@ -63,13 +63,23 @@ func redirParse(c *caddy.Controller) ([]Rule, error) {
 	}
 
 	for c.Next() {
+		matcher, err := httpserver.SetupIfMatcher(c.Dispenser)
+		if err != nil {
+			return nil, err
+		}
 		args := c.RemainingArgs()
 
 		var hadOptionalBlock bool
 		for c.NextBlock() {
+			if httpserver.IfMatcherKeyword(c.Val()) {
+				continue
+			}
+
 			hadOptionalBlock = true
 
-			var rule Rule
+			var rule = Rule{
+				RequestMatcher: matcher,
+			}
 
 			if cfg.TLS.Enabled {
 				rule.FromScheme = "https"
@@ -126,7 +136,9 @@ func redirParse(c *caddy.Controller) ([]Rule, error) {
 		}
 
 		if !hadOptionalBlock {
-			var rule Rule
+			var rule = Rule{
+				RequestMatcher: matcher,
+			}
 
 			if cfg.TLS.Enabled {
 				rule.FromScheme = "https"

caddyhttp/rewrite/rewrite.go

@@ -97,15 +97,15 @@ type ComplexRule struct {
 	// Extensions to filter by
 	Exts []string
 
-	// Rewrite conditions
-	Ifs []If
+	// Request matcher
+	httpserver.RequestMatcher
 
 	*regexp.Regexp
 }
 
 // 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, ifs []If) (*ComplexRule, error) {
+func NewComplexRule(base, pattern, to string, status int, ext []string, m httpserver.RequestMatcher) (*ComplexRule, error) {
 	// validate regexp if present
 	var r *regexp.Regexp
 	if pattern != "" {
@@ -127,12 +127,12 @@ func NewComplexRule(base, pattern, to string, status int, ext []string, ifs []If
 	}
 
 	return &ComplexRule{
-		Base:   base,
-		To:     to,
-		Status: status,
-		Exts:   ext,
-		Ifs:    ifs,
-		Regexp: r,
+		Base:           base,
+		To:             to,
+		Status:         status,
+		Exts:           ext,
+		RequestMatcher: m,
+		Regexp:         r,
 	}, nil
 }
 
@@ -182,11 +182,9 @@ func (r *ComplexRule) Rewrite(fs http.FileSystem, req *http.Request) (re Result)
 		}
 	}
 
-	// validate rewrite conditions
-	for _, i := range r.Ifs {
-		if !i.True(req) {
-			return
-		}
+	// validate if conditions
+	if !r.RequestMatcher.Match(req) {
+		return
 	}
 
 	// if status is present, stop rewrite and return it.
@@ -230,6 +228,10 @@ func (r *ComplexRule) matchExt(rPath string) bool {
 	return true
 }
 
+func newReplacer(r *http.Request) httpserver.Replacer {
+	return httpserver.NewReplacer(r, nil, "")
+}
+
 // When a rewrite is performed, this header is added to the request
 // and is for internal use only, specifically the fastcgi middleware.
 // It contains the original request URI before the rewrite.

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, nil)
+		rule, err := NewComplexRule(regexpRule[0], regexpRule[1], regexpRule[2], 0, ext, httpserver.IfMatcher{})
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -127,7 +127,7 @@ func TestRewrite(t *testing.T) {
 
 	for i, s := range statusTests {
 		urlPath := fmt.Sprintf("/status%d", i)
-		rule, err := NewComplexRule(s.base, s.regexp, s.to, s.status, nil, nil)
+		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)
 		}

caddyhttp/rewrite/setup.go

@@ -50,13 +50,19 @@ func rewriteParse(c *caddy.Controller) ([]Rule, error) {
 
 		args := c.RemainingArgs()
 
-		var ifs []If
+		var matcher httpserver.RequestMatcher
 
 		switch len(args) {
 		case 1:
 			base = args[0]
 			fallthrough
 		case 0:
+			// Integrate request matcher for 'if' conditions.
+			matcher, err = httpserver.SetupIfMatcher(c.Dispenser)
+			if err != nil {
+				return nil, err
+			}
+		block:
 			for c.NextBlock() {
 				switch c.Val() {
 				case "r", "regexp":
@@ -76,16 +82,6 @@ func rewriteParse(c *caddy.Controller) ([]Rule, error) {
 						return nil, c.ArgErr()
 					}
 					ext = args1
-				case "if":
-					args1 := c.RemainingArgs()
-					if len(args1) != 3 {
-						return nil, c.ArgErr()
-					}
-					ifCond, err := NewIf(args1[0], args1[1], args1[2])
-					if err != nil {
-						return nil, err
-					}
-					ifs = append(ifs, ifCond)
 				case "status":
 					if !c.NextArg() {
 						return nil, c.ArgErr()
@@ -95,6 +91,9 @@ func rewriteParse(c *caddy.Controller) ([]Rule, error) {
 						return nil, c.Err("status must be 2xx or 4xx")
 					}
 				default:
+					if httpserver.IfMatcherKeyword(c.Val()) {
+						continue block
+					}
 					return nil, c.ArgErr()
 				}
 			}
@@ -102,7 +101,7 @@ func rewriteParse(c *caddy.Controller) ([]Rule, error) {
 			if to == "" && status == 0 {
 				return nil, c.ArgErr()
 			}
-			if rule, err = NewComplexRule(base, pattern, to, status, ext, ifs); err != nil {
+			if rule, err = NewComplexRule(base, pattern, to, status, ext, matcher); err != nil {
 				return nil, err
 			}
 			regexpRules = append(regexpRules, rule)

caddyhttp/rewrite/setup_test.go

@@ -131,12 +131,6 @@ func TestRewriteParse(t *testing.T) {
 		{`rewrite /`, true, []Rule{
 			&ComplexRule{},
 		}},
-		{`rewrite {
-			to	/to
-			if {path} is a
-		 }`, false, []Rule{
-			&ComplexRule{Base: "/", To: "/to", Ifs: []If{{A: "{path}", Operator: "is", B: "a"}}},
-		}},
 		{`rewrite {
 			status 500
 		 }`, true, []Rule{
@@ -229,11 +223,6 @@ func TestRewriteParse(t *testing.T) {
 				}
 			}
 
-			if fmt.Sprint(actualRule.Ifs) != fmt.Sprint(expectedRule.Ifs) {
-				t.Errorf("Test %d, rule %d: Expected Pattern=%s, got %s",
-					i, j, fmt.Sprint(expectedRule.Ifs), fmt.Sprint(actualRule.Ifs))
-			}
-
 		}
 	}