git » spf » commit 42e3519

diff --git a/spf.go b/spf.go
index 97d2208..4ed1539 100644
--- a/spf.go
+++ b/spf.go
@@ -357,11 +357,6 @@ func (r *resolution) Check(domain string) (Result, error) {
 			continue
 		}
 
-		if r.voidcount > r.maxvoidcount {
-			r.trace("void lookup limit reached")
-			return PermError, ErrVoidLookupLimitReached
-		}
-
 		// See if we have a qualifier, defaulting to + (pass).
 		// https://tools.ietf.org/html/rfc7208#section-4.6.2
 		result, ok := qualToResult[field[0]]
@@ -492,23 +487,26 @@ func (r *resolution) countLookup() error {
 }
 
 // Check if the given DNS error is a "void lookup" (0 answers, or nxdomain),
-// and if so increment the void lookup counter.
-func (r *resolution) checkVoidLookup(nanswers int, err error) {
+// and return an error if the limit was exceeded.
+// Unlike countLookup, we can only tell a lookup was void after doing it, so
+// fields must call this right after the lookup, and stop the evaluation if it
+// returns an error.
+// https://tools.ietf.org/html/rfc7208#section-4.6.4
+func (r *resolution) countVoidLookup(nanswers int, err error) error {
+	derr, isDNSErr := err.(*net.DNSError)
 	if err == nil && nanswers == 0 {
 		r.voidcount++
 		r.trace("void lookup: no answers")
-		return
-	}
-
-	derr, ok := err.(*net.DNSError)
-	if !ok {
-		return
-	}
-
-	if derr.IsNotFound {
+	} else if isDNSErr && derr.IsNotFound {
 		r.voidcount++
 		r.trace("void lookup: nxdomain")
 	}
+
+	if r.voidcount > r.maxvoidcount {
+		r.trace("void lookup limit reached")
+		return ErrVoidLookupLimitReached
+	}
+	return nil
 }
 
 // ipField processes an "ip" field.
@@ -560,7 +558,9 @@ func (r *resolution) ptrField(res Result, field, domain string) (bool, Result, e
 		}
 		r.ipNames = []string{}
 		ns, err := r.resolver.LookupAddr(r.ctx, r.ip.String())
-		r.checkVoidLookup(len(ns), err)
+		if verr := r.countVoidLookup(len(ns), err); verr != nil {
+			return true, PermError, verr
+		}
 		if err != nil {
 			// https://tools.ietf.org/html/rfc7208#section-5
 			if isNotFound(err) {
@@ -625,7 +625,9 @@ func (r *resolution) existsField(res Result, field, domain string) (bool, Result
 		return true, PermError, err
 	}
 	ips, err := r.resolver.LookupIPAddr(r.ctx, eDomain)
-	r.checkVoidLookup(len(ips), err)
+	if verr := r.countVoidLookup(len(ips), err); verr != nil {
+		return true, PermError, verr
+	}
 	if err != nil {
 		// https://tools.ietf.org/html/rfc7208#section-5
 		if isNotFound(err) {
@@ -759,7 +761,9 @@ func (r *resolution) aField(res Result, field, domain string) (bool, Result, err
 		return true, PermError, err
 	}
 	ips, err := r.resolver.LookupIPAddr(r.ctx, aDomain)
-	r.checkVoidLookup(len(ips), err)
+	if verr := r.countVoidLookup(len(ips), err); verr != nil {
+		return true, PermError, verr
+	}
 	if err != nil {
 		// https://tools.ietf.org/html/rfc7208#section-5
 		if isNotFound(err) {
@@ -794,7 +798,9 @@ func (r *resolution) mxField(res Result, field, domain string) (bool, Result, er
 		return true, PermError, err
 	}
 	mxs, err := r.resolver.LookupMX(r.ctx, mxDomain)
-	r.checkVoidLookup(len(mxs), err)
+	if verr := r.countVoidLookup(len(mxs), err); verr != nil {
+		return true, PermError, verr
+	}
 
 	// If we get some results, use them even if we get an error alongisde.
 	// This happens when one of the records is invalid, because Go library can
diff --git a/testdata/blitirispf-tests.yml b/testdata/blitirispf-tests.yml
index b6f8f62..cc2c002 100644
--- a/testdata/blitirispf-tests.yml
+++ b/testdata/blitirispf-tests.yml
@@ -318,6 +318,69 @@ zonedata:
   4.3.2.1.in-addr.arpa:
     - PTR: ptrmatch.com
 ---
+description: Void lookup limit
+tests:
+  void-limit-at-2:
+    description: |
+      Check that 2 void lookups (the default limit) are allowed, and that the
+      terms after them are still evaluated.
+    mailfrom: "foo@atvoid2"
+    host: 1.2.3.4
+    result: pass
+  void-limit-over-a:
+    description: |
+      Check that exceeding the void lookup limit on an "a" term causes a
+      permerror, even when it is the last term. The limit must be checked
+      right after the lookup: checking it only when moving on to the next
+      term lets the excess go unnoticed if there is no next term.
+    mailfrom: "foo@overvoid-a"
+    host: 1.2.3.4
+    result: permerror
+  void-limit-over-mx:
+    description: |
+      Same as void-limit-over-a, for the "mx" mechanism. The domain exists but
+      has no MX records, which is a void lookup.
+    mailfrom: "foo@overvoid-mx"
+    host: 1.2.3.4
+    result: permerror
+  void-limit-over-exists:
+    description: |
+      Same as void-limit-over-a, for the "exists" mechanism.
+    mailfrom: "foo@overvoid-exists"
+    host: 1.2.3.4
+    result: permerror
+  void-limit-over-ptr:
+    description: |
+      Same as void-limit-over-a, for the "ptr" mechanism. The address has no
+      PTR records, which is a void lookup.
+    mailfrom: "foo@overvoid-ptr"
+    host: 1.2.3.4
+    result: permerror
+  void-limit-over-in-include:
+    description: |
+      Check that exceeding the void lookup limit inside an "include" makes the
+      whole evaluation return permerror, instead of just making the include
+      not match. Note the include is the last term, so nothing after it can
+      turn the excess into a permerror on its own.
+    mailfrom: "foo@overvoid-include"
+    host: 1.2.3.4
+    result: permerror
+zonedata:
+  atvoid2:
+    - SPF: v=spf1 a:void1 a:void2 ip4:1.2.3.4 -all
+  overvoid-a:
+    - SPF: v=spf1 a:void1 a:void2 a:void3
+  overvoid-mx:
+    - SPF: v=spf1 a:void1 a:void2 mx:onlya
+  overvoid-exists:
+    - SPF: v=spf1 a:void1 a:void2 exists:void3
+  overvoid-ptr:
+    - SPF: v=spf1 a:void1 a:void2 ptr
+  overvoid-include:
+    - SPF: v=spf1 include:overvoid-a
+  onlya:
+    - A: 1.2.3.4
+---
 description: MX resolution limits
 tests:
   mx-resolution-10-terms: