git » spf » commit a2c3438

diff --git a/spf.go b/spf.go
index d1154f3..04d7034 100644
--- a/spf.go
+++ b/spf.go
@@ -329,6 +329,19 @@ type resolution struct {
 // https://tools.ietf.org/html/rfc7208#section-4.6.1
 var modifierRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9._-]*=`)
 
+// isNamedModifier returns whether the given term is the modifier with the
+// given name, like "redirect" or "exp".
+//
+// Modifier names are case-insensitive (on ASCII, as per the ABNF), and take
+// no qualifier, so this must be given the term before any qualifier has been
+// stripped from it.
+// https://tools.ietf.org/html/rfc7208#section-4.6.1
+func isNamedModifier(field, name string) bool {
+	return len(field) > len(name) &&
+		field[len(name)] == '=' &&
+		asciiEqualFold(field[:len(name)], name)
+}
+
 var aField = regexp.MustCompile(`^(a$|a:|a/)`)
 var mxField = regexp.MustCompile(`^(mx$|mx:|mx/)`)
 var ptrField = regexp.MustCompile(`^(ptr$|ptr:)`)
@@ -368,9 +381,12 @@ func (r *resolution) Check(domain string) (Result, error) {
 
 	// Redirects must be handled after the rest; instead of having two loops,
 	// we just move them to the end.
+	// Note this has to be consistent with how we recognize the modifier
+	// below, otherwise a redirect could be evaluated in place, and bypass the
+	// check for duplicates.
 	var newfields, redirects []string
 	for _, field := range fields {
-		if strings.HasPrefix(field, "redirect=") {
+		if isNamedModifier(field, "redirect") {
 			redirects = append(redirects, field)
 		} else {
 			newfields = append(newfields, field)
@@ -396,11 +412,13 @@ func (r *resolution) Check(domain string) (Result, error) {
 			continue
 		}
 
-		// Is this a modifier? Note we check this before stripping the
+		// Is this a modifier? Note we check these before stripping the
 		// qualifier below, because modifiers don't take one:
 		//   directive = [ qualifier ] mechanism
 		//   modifier  = redirect / explanation / unknown-modifier
 		// https://tools.ietf.org/html/rfc7208#section-4.6.1
+		isExp := isNamedModifier(field, "exp")
+		isRedirect := isNamedModifier(field, "redirect")
 		isModifier := modifierRegexp.MatchString(field)
 
 		// See if we have a qualifier, defaulting to + (pass).
@@ -450,10 +468,10 @@ func (r *resolution) Check(domain string) (Result, error) {
 				r.trace("%q %v, %v", field, res, err)
 				return res, err
 			}
-		} else if strings.HasPrefix(lfield, "exp=") {
+		} else if isExp {
 			r.trace("exp= ignored")
 			continue
-		} else if strings.HasPrefix(lfield, "redirect=") {
+		} else if isRedirect {
 			res, err := r.redirectField(field, domain)
 			r.trace("%q: %v, %v", field, res, err)
 			return res, err
diff --git a/spf_test.go b/spf_test.go
index e527aa0..fb11565 100644
--- a/spf_test.go
+++ b/spf_test.go
@@ -304,16 +304,71 @@ func TestRedirectOrder(t *testing.T) {
 	dns.Txt["faildom"] = []string{"v=spf1 -all"}
 	defaultTrace = t.Logf
 
-	dns.Txt["domain"] = []string{"v=spf1 redirect=faildom"}
-	res, err := CheckHost(ip1111, "domain")
-	if res != Fail || err != ErrMatchedAll {
-		t.Errorf("expected fail, got %v (%v)", res, err)
+	// Modifier names are case-insensitive, so how the modifier is spelled
+	// must not change whether it is deferred to the end.
+	// https://tools.ietf.org/html/rfc7208#section-4.6.1
+	cases := []struct {
+		txt string
+		res Result
+	}{
+		{"v=spf1 redirect=faildom", Fail},
+		{"v=spf1 redirect=faildom all", Pass},
+		{"v=spf1 Redirect=faildom", Fail},
+		{"v=spf1 Redirect=faildom all", Pass},
+		{"v=spf1 REDIRECT=faildom all", Pass},
+		{"v=spf1 ReDiReCt=faildom all", Pass},
 	}
 
-	dns.Txt["domain"] = []string{"v=spf1 redirect=faildom all"}
-	res, err = CheckHost(ip1111, "domain")
-	if res != Pass || err != ErrMatchedAll {
-		t.Errorf("expected pass, got %v (%v)", res, err)
+	for _, c := range cases {
+		dns.Txt["domain"] = []string{c.txt}
+		res, err := CheckHost(ip1111, "domain")
+		if res != c.res || err != ErrMatchedAll {
+			t.Errorf("%q: expected %v, got %v (%v)", c.txt, c.res, res, err)
+		}
+	}
+}
+
+func TestModifierNames(t *testing.T) {
+	dns := NewDefaultResolver()
+	dns.Txt["alldom"] = []string{"v=spf1 +all"}
+	defaultTrace = t.Logf
+
+	cases := []struct {
+		txt string
+		res Result
+		err error
+	}{
+		// At most one redirect is allowed, and the check must not depend on
+		// how each of them is spelled.
+		// https://tools.ietf.org/html/rfc7208#section-6
+		{"v=spf1 redirect=alldom redirect=alldom", PermError, ErrInvalidDomain},
+		{"v=spf1 redirect=alldom Redirect=alldom", PermError, ErrInvalidDomain},
+		{"v=spf1 Redirect=alldom REDIRECT=alldom", PermError, ErrInvalidDomain},
+
+		// A single one is fine, either way.
+		{"v=spf1 redirect=alldom", Pass, ErrMatchedAll},
+		{"v=spf1 Redirect=alldom", Pass, ErrMatchedAll},
+
+		// "exp" is ignored, in any case.
+		{"v=spf1 exp=blah +all", Pass, ErrMatchedAll},
+		{"v=spf1 Exp=blah +all", Pass, ErrMatchedAll},
+		{"v=spf1 EXP=blah +all", Pass, ErrMatchedAll},
+
+		// Modifiers take no qualifier, so these are not modifiers, and not
+		// valid mechanisms either.
+		// https://tools.ietf.org/html/rfc7208#section-4.6.1
+		{"v=spf1 -redirect=alldom all", PermError, ErrUnknownField},
+		{"v=spf1 +redirect=alldom all", PermError, ErrUnknownField},
+		{"v=spf1 -exp=blah all", PermError, ErrUnknownField},
+	}
+
+	for _, c := range cases {
+		dns.Txt["domain"] = []string{c.txt}
+		res, err := CheckHost(ip1111, "domain")
+		if res != c.res || err != c.err {
+			t.Errorf("%q: expected [%v/%v], got [%v/%v]",
+				c.txt, c.res, c.err, res, err)
+		}
 	}
 }