diff --git a/caddyhttp/httpserver/condition.go b/caddyhttp/httpserver/condition.go index 7722abd19..81f9ece77 100644 --- a/caddyhttp/httpserver/condition.go +++ b/caddyhttp/httpserver/condition.go @@ -53,59 +53,8 @@ const ( matchOp = "match" ) -func operatorError(operator string) error { - return fmt.Errorf("Invalid operator %v", operator) -} - // ifCondition is a 'if' condition. -type ifCondition func(string, string) bool - -var ifConditions = map[string]ifCondition{ - isOp: isFunc, - notOp: notFunc, - hasOp: hasFunc, - startsWithOp: startsWithFunc, - endsWithOp: endsWithFunc, - matchOp: matchFunc, -} - -// isFunc is condition for Is operator. -// It checks for equality. -func isFunc(a, b string) bool { - return a == b -} - -// notFunc is condition for Not operator. -// It checks for inequality. -func notFunc(a, b string) bool { - return !isFunc(a, b) -} - -// hasFunc is condition for Has operator. -// It checks if b is a substring of a. -func hasFunc(a, b string) bool { - return strings.Contains(a, b) -} - -// startsWithFunc is condition for StartsWith operator. -// It checks if b is a prefix of a. -func startsWithFunc(a, b string) bool { - return strings.HasPrefix(a, b) -} - -// endsWithFunc is condition for EndsWith operator. -// It checks if b is a suffix of a. -func endsWithFunc(a, b string) bool { - return strings.HasSuffix(a, b) -} - -// matchFunc is condition for Match operator. -// It does regexp matching of a against pattern in b -// and returns if they match. -func matchFunc(a, b string) bool { - matched, _ := regexp.MatchString(b, a) - return matched -} +type ifFunc func(a, b string) bool // ifCond is statement for a IfMatcher condition. type ifCond struct { @@ -113,40 +62,79 @@ type ifCond struct { op string b string neg bool + rex *regexp.Regexp + f ifFunc } // newIfCond creates a new If condition. -func newIfCond(a, operator, b string) (ifCond, error) { - neg := false - if strings.HasPrefix(operator, "not_") { - neg = true - operator = operator[4:] +func newIfCond(a, op, b string) (ifCond, error) { + i := ifCond{a: a, op: op, b: b} + if strings.HasPrefix(op, "not_") { + i.neg = true + i.op = op[4:] } - if _, ok := ifConditions[operator]; !ok { - return ifCond{}, operatorError(operator) + + switch i.op { + case isOp: + // It checks for equality. + i.f = i.isFunc + case notOp: + // It checks for inequality. + i.f = i.notFunc + case hasOp: + // It checks if b is a substring of a. + i.f = strings.Contains + case startsWithOp: + // It checks if b is a prefix of a. + i.f = strings.HasPrefix + case endsWithOp: + // It checks if b is a suffix of a. + i.f = strings.HasSuffix + case matchOp: + // It does regexp matching of a against pattern in b and returns if they match. + var err error + if i.rex, err = regexp.Compile(i.b); err != nil { + return ifCond{}, fmt.Errorf("Invalid regular expression: '%s', %v", i.b, err) + } + i.f = i.matchFunc + default: + return ifCond{}, fmt.Errorf("Invalid operator %v", i.op) } - return ifCond{ - a: a, - op: operator, - b: b, - neg: neg, - }, nil + + return i, nil +} + +// isFunc is condition for Is operator. +func (i ifCond) isFunc(a, b string) bool { + return a == b +} + +// notFunc is condition for Not operator. +func (i ifCond) notFunc(a, b string) bool { + return a != b +} + +// matchFunc is condition for Match operator. +func (i ifCond) matchFunc(a, b string) bool { + return i.rex.MatchString(a) } // True returns true if the condition is true and false otherwise. // If r is not nil, it replaces placeholders before comparison. func (i ifCond) True(r *http.Request) bool { - if c, ok := ifConditions[i.op]; ok { + if i.f != nil { a, b := i.a, i.b if r != nil { replacer := NewReplacer(r, nil, "") a = replacer.Replace(i.a) - b = replacer.Replace(i.b) + if i.op != matchOp { + b = replacer.Replace(i.b) + } } if i.neg { - return !c(a, b) + return !i.f(a, b) } - return c(a, b) + return i.f(a, b) } return i.neg // false if not negated, true otherwise } diff --git a/caddyhttp/httpserver/condition_test.go b/caddyhttp/httpserver/condition_test.go index c187b33a0..a63cc997b 100644 --- a/caddyhttp/httpserver/condition_test.go +++ b/caddyhttp/httpserver/condition_test.go @@ -2,8 +2,8 @@ package httpserver import ( "context" - "fmt" "net/http" + "regexp" "strings" "testing" @@ -14,66 +14,72 @@ func TestConditions(t *testing.T) { tests := []struct { condition string isTrue bool + shouldErr bool }{ - {"a is b", false}, - {"a is a", true}, - {"a not b", true}, - {"a not a", false}, - {"a has a", true}, - {"a has b", false}, - {"ba has b", true}, - {"bab has b", true}, - {"bab has bb", false}, - {"a not_has a", false}, - {"a not_has b", true}, - {"ba not_has b", false}, - {"bab not_has b", false}, - {"bab not_has bb", true}, - {"bab starts_with bb", false}, - {"bab starts_with ba", true}, - {"bab starts_with bab", true}, - {"bab not_starts_with bb", true}, - {"bab not_starts_with ba", false}, - {"bab not_starts_with bab", false}, - {"bab ends_with bb", false}, - {"bab ends_with bab", true}, - {"bab ends_with ab", true}, - {"bab not_ends_with bb", true}, - {"bab not_ends_with ab", false}, - {"bab not_ends_with bab", false}, - {"a match *", false}, - {"a match a", true}, - {"a match .*", true}, - {"a match a.*", true}, - {"a match b.*", false}, - {"ba match b.*", true}, - {"ba match b[a-z]", true}, - {"b0 match b[a-z]", false}, - {"b0a match b[a-z]", false}, - {"b0a match b[a-z]+", false}, - {"b0a match b[a-z0-9]+", true}, - {"a not_match *", true}, - {"a not_match a", false}, - {"a not_match .*", false}, - {"a not_match a.*", false}, - {"a not_match b.*", true}, - {"ba not_match b.*", false}, - {"ba not_match b[a-z]", false}, - {"b0 not_match b[a-z]", true}, - {"b0a not_match b[a-z]", true}, - {"b0a not_match b[a-z]+", true}, - {"b0a not_match b[a-z0-9]+", false}, + {"a is b", false, false}, + {"a is a", true, false}, + {"a not b", true, false}, + {"a not a", false, false}, + {"a has a", true, false}, + {"a has b", false, false}, + {"ba has b", true, false}, + {"bab has b", true, false}, + {"bab has bb", false, false}, + {"a not_has a", false, false}, + {"a not_has b", true, false}, + {"ba not_has b", false, false}, + {"bab not_has b", false, false}, + {"bab not_has bb", true, false}, + {"bab starts_with bb", false, false}, + {"bab starts_with ba", true, false}, + {"bab starts_with bab", true, false}, + {"bab not_starts_with bb", true, false}, + {"bab not_starts_with ba", false, false}, + {"bab not_starts_with bab", false, false}, + {"bab ends_with bb", false, false}, + {"bab ends_with bab", true, false}, + {"bab ends_with ab", true, false}, + {"bab not_ends_with bb", true, false}, + {"bab not_ends_with ab", false, false}, + {"bab not_ends_with bab", false, false}, + {"a match *", false, true}, + {"a match a", true, false}, + {"a match .*", true, false}, + {"a match a.*", true, false}, + {"a match b.*", false, false}, + {"ba match b.*", true, false}, + {"ba match b[a-z]", true, false}, + {"b0 match b[a-z]", false, false}, + {"b0a match b[a-z]", false, false}, + {"b0a match b[a-z]+", false, false}, + {"b0a match b[a-z0-9]+", true, false}, + {"bac match b[a-z]{2}", true, false}, + {"a not_match *", false, true}, + {"a not_match a", false, false}, + {"a not_match .*", false, false}, + {"a not_match a.*", false, false}, + {"a not_match b.*", true, false}, + {"ba not_match b.*", false, false}, + {"ba not_match b[a-z]", false, false}, + {"b0 not_match b[a-z]", true, false}, + {"b0a not_match b[a-z]", true, false}, + {"b0a not_match b[a-z]+", true, false}, + {"b0a not_match b[a-z0-9]+", false, false}, + {"bac not_match b[a-z]{2}", false, false}, } for i, test := range tests { str := strings.Fields(test.condition) ifCond, err := newIfCond(str[0], str[1], str[2]) if err != nil { - t.Error(err) + if !test.shouldErr { + t.Error(err) + } + continue } isTrue := ifCond.True(nil) if isTrue != test.isTrue { - t.Errorf("Test %d: expected %v found %v", i, test.isTrue, isTrue) + t.Errorf("Test %d: '%s' expected %v found %v", i, test.condition, test.isTrue, isTrue) } } @@ -192,6 +198,7 @@ func TestIfMatcher(t *testing.T) { } func TestSetupIfMatcher(t *testing.T) { + rex_b, _ := regexp.Compile("b") tests := []struct { input string shouldErr bool @@ -201,7 +208,7 @@ func TestSetupIfMatcher(t *testing.T) { if a match b }`, false, IfMatcher{ ifs: []ifCond{ - {a: "a", op: "match", b: "b", neg: false}, + {a: "a", op: "match", b: "b", neg: false, rex: rex_b}, }, }}, {`test { @@ -209,7 +216,7 @@ func TestSetupIfMatcher(t *testing.T) { if_op or }`, false, IfMatcher{ ifs: []ifCond{ - {a: "a", op: "match", b: "b", neg: false}, + {a: "a", op: "match", b: "b", neg: false, rex: rex_b}, }, isOr: true, }}, @@ -255,6 +262,7 @@ func TestSetupIfMatcher(t *testing.T) { for i, test := range tests { c := caddy.NewTestController("http", test.input) c.Next() + matcher, err := SetupIfMatcher(c) if err == nil && test.shouldErr { t.Errorf("Test %d didn't error, but it should have", i) @@ -263,15 +271,60 @@ func TestSetupIfMatcher(t *testing.T) { } else if err != nil && test.shouldErr { continue } - if _, ok := matcher.(IfMatcher); !ok { + + test_if, ok := matcher.(IfMatcher) + if !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)) + + if len(test_if.ifs) != len(test.expected.ifs) { + t.Errorf("Test %d: Expected %d ifConditions, found %v", i, + len(test.expected.ifs), len(test_if.ifs)) + } + + for j, if_c := range test_if.ifs { + expected_c := test.expected.ifs[j] + + if if_c.a != expected_c.a { + t.Errorf("Test %d, ifCond %d: Expected A=%s, got %s", + i, j, if_c.a, expected_c.a) + } + + if if_c.op != expected_c.op { + t.Errorf("Test %d, ifCond %d: Expected Op=%s, got %s", + i, j, if_c.op, expected_c.op) + } + + if if_c.b != expected_c.b { + t.Errorf("Test %d, ifCond %d: Expected B=%s, got %s", + i, j, if_c.b, expected_c.b) + } + + if if_c.neg != expected_c.neg { + t.Errorf("Test %d, ifCond %d: Expected Neg=%v, got %v", + i, j, if_c.neg, expected_c.neg) + } + + if expected_c.rex != nil && if_c.rex == nil { + t.Errorf("Test %d, ifCond %d: Expected Rex=%v, got ", + i, j, expected_c.rex) + } + + if expected_c.rex == nil && if_c.rex != nil { + t.Errorf("Test %d, ifCond %d: Expected Rex=, got %v", + i, j, if_c.rex) + } + + if expected_c.rex != nil && if_c.rex != nil { + if if_c.rex.String() != expected_c.rex.String() { + t.Errorf("Test %d, ifCond %d: Expected Rex=%v, got %v", + i, j, if_c.rex, expected_c.rex) + } + } } } } diff --git a/caddyhttp/rewrite/rewrite.go b/caddyhttp/rewrite/rewrite.go index 5c8fd5ff7..14d25ca58 100644 --- a/caddyhttp/rewrite/rewrite.go +++ b/caddyhttp/rewrite/rewrite.go @@ -84,19 +84,19 @@ type ComplexRule struct { // Request matcher httpserver.RequestMatcher - *regexp.Regexp + Regexp *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, 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 != "" { var err error r, err = regexp.Compile(pattern) if err != nil { - return nil, err + return ComplexRule{}, err } } @@ -105,7 +105,7 @@ func NewComplexRule(base, pattern, to string, ext []string, matcher httpserver.R if len(v) < 2 || (len(v) < 3 && v[0] == '!') { // check if no extension is specified if v != "/" && v != "!/" { - return nil, fmt.Errorf("invalid extension %v", v) + return ComplexRule{}, fmt.Errorf("invalid extension %v", v) } } } @@ -118,7 +118,7 @@ func NewComplexRule(base, pattern, to string, ext []string, matcher httpserver.R httpserver.PathMatcher(base), ) - return &ComplexRule{ + return ComplexRule{ Base: base, To: to, Exts: ext, @@ -128,13 +128,13 @@ func NewComplexRule(base, pattern, to string, ext []string, matcher httpserver.R } // BasePath satisfies httpserver.Config -func (r *ComplexRule) BasePath() string { return r.Base } +func (r ComplexRule) BasePath() string { return r.Base } // Match satisfies httpserver.Config. // // Though ComplexRule embeds a RequestMatcher, additional // checks are needed which requires a custom implementation. -func (r *ComplexRule) Match(req *http.Request) bool { +func (r ComplexRule) Match(req *http.Request) bool { // validate RequestMatcher // includes if and path if !r.RequestMatcher.Match(req) { @@ -155,7 +155,7 @@ func (r *ComplexRule) Match(req *http.Request) bool { } // Rewrite rewrites the internal location of the current request. -func (r *ComplexRule) Rewrite(fs http.FileSystem, req *http.Request) (re Result) { +func (r ComplexRule) Rewrite(fs http.FileSystem, req *http.Request) (re Result) { replacer := newReplacer(req) // validate regexp if present @@ -189,7 +189,7 @@ func (r *ComplexRule) Rewrite(fs http.FileSystem, req *http.Request) (re Result) // matchExt matches rPath against registered file extensions. // Returns true if a match is found and false otherwise. -func (r *ComplexRule) matchExt(rPath string) bool { +func (r ComplexRule) matchExt(rPath string) bool { f := filepath.Base(rPath) ext := path.Ext(f) if ext == "" { @@ -216,14 +216,14 @@ func (r *ComplexRule) matchExt(rPath string) bool { return !mustUse } -func (r *ComplexRule) regexpMatches(rPath string) []string { +func (r ComplexRule) regexpMatches(rPath string) []string { if r.Regexp != nil { // include trailing slash in regexp if present start := len(r.Base) if strings.HasSuffix(r.Base, "/") { start-- } - return r.FindStringSubmatch(rPath[start:]) + return r.Regexp.FindStringSubmatch(rPath[start:]) } return nil } diff --git a/caddyhttp/rewrite/setup_test.go b/caddyhttp/rewrite/setup_test.go index fd355102c..4e32c2eed 100644 --- a/caddyhttp/rewrite/setup_test.go +++ b/caddyhttp/rewrite/setup_test.go @@ -3,6 +3,7 @@ package rewrite import ( "fmt" "regexp" + "strings" "testing" "github.com/mholt/caddy" @@ -97,14 +98,14 @@ func TestRewriteParse(t *testing.T) { r .* to /to /index.php? }`, false, []Rule{ - &ComplexRule{Base: "/", To: "/to /index.php?", Regexp: regexp.MustCompile(".*")}, + ComplexRule{Base: "/", To: "/to /index.php?", Regexp: regexp.MustCompile(".*")}, }}, {`rewrite { regexp .* to /to ext / html txt }`, false, []Rule{ - &ComplexRule{Base: "/", To: "/to", Exts: []string{"/", "html", "txt"}, Regexp: regexp.MustCompile(".*")}, + ComplexRule{Base: "/", To: "/to", Exts: []string{"/", "html", "txt"}, Regexp: regexp.MustCompile(".*")}, }}, {`rewrite /path { r rr @@ -115,27 +116,27 @@ func TestRewriteParse(t *testing.T) { to /to /to2 } `, false, []Rule{ - &ComplexRule{Base: "/path", To: "/dest", Regexp: regexp.MustCompile("rr")}, - &ComplexRule{Base: "/", To: "/to /to2", Regexp: regexp.MustCompile("[a-z]+")}, + ComplexRule{Base: "/path", To: "/dest", Regexp: regexp.MustCompile("rr")}, + ComplexRule{Base: "/", To: "/to /to2", Regexp: regexp.MustCompile("[a-z]+")}, }}, {`rewrite { r .* }`, true, []Rule{ - &ComplexRule{}, + ComplexRule{}, }}, {`rewrite { }`, true, []Rule{ - &ComplexRule{}, + ComplexRule{}, }}, {`rewrite /`, true, []Rule{ - &ComplexRule{}, + ComplexRule{}, }}, {`rewrite { if {path} match / to /to }`, false, []Rule{ - &ComplexRule{Base: "/", To: "/to"}, + ComplexRule{Base: "/", To: "/to"}, }}, } @@ -156,8 +157,8 @@ func TestRewriteParse(t *testing.T) { } for j, e := range test.expected { - actualRule := actual[j].(*ComplexRule) - expectedRule := e.(*ComplexRule) + actualRule := actual[j].(ComplexRule) + expectedRule := e.(ComplexRule) if actualRule.Base != expectedRule.Base { t.Errorf("Test %d, rule %d: Expected Base=%s, got %s", @@ -175,13 +176,15 @@ func TestRewriteParse(t *testing.T) { } if actualRule.Regexp != nil { - if actualRule.String() != expectedRule.String() { + if actualRule.Regexp.String() != expectedRule.Regexp.String() { t.Errorf("Test %d, rule %d: Expected Pattern=%s, got %s", - i, j, expectedRule.String(), actualRule.String()) + i, j, actualRule.Regexp.String(), expectedRule.Regexp.String()) } } + } + if rules_fmt := fmt.Sprintf("%v", actual); strings.HasPrefix(rules_fmt, "%!") { + t.Errorf("Test %d: Failed to string encode: %#v", i, rules_fmt) } } - }