git » spf » commit c4a8b79

Explicitly differentiate between a permanent error and NXDOMAIN

author Alberto Bertogli
2022-08-01 19:46:42 UTC
committer Alberto Bertogli
2022-08-02 23:49:38 UTC
parent 32d5fc8b2ac8ff85ac34a190e45dc2a7b5b5967a

Explicitly differentiate between a permanent error and NXDOMAIN

At the top-level check, today we treat any permanent error as if it was
an NXDOMAIN error, which is special because it signals the domain
doesn't exist, and the RFC has specific behaviour expectations for it.

This is because originally Go did not provide a good way of
distinguishing the two, but that is no longer true since Go 1.13.

The consequences are that top-level and some recursive DNS permanent
errors return None instead of PermError, which is not ideal as it can be
misleading for some use cases.

This patch fixes the problem by adding code to handle NXDOMAIN
explicitly.

spf.go +9 -3
spf_test.go +6 -4

diff --git a/spf.go b/spf.go
index 3adbd5e..f664213 100644
--- a/spf.go
+++ b/spf.go
@@ -308,6 +308,12 @@ func (r *resolution) Check(domain string) (Result, error) {
 	r.trace("check %q %d %d", domain, r.count, r.voidcount)
 	txt, err := r.getDNSRecord(domain)
 	if err != nil {
+		if isNotFound(err) {
+			// NXDOMAIN -> None.
+			// https://datatracker.ietf.org/doc/html/rfc7208#section-4.3
+			r.trace("dns domain not found: %v", err)
+			return None, ErrNoResult
+		}
 		if isTemporary(err) {
 			r.trace("dns temp error: %v", err)
 			return TempError, err
@@ -316,10 +322,10 @@ func (r *resolution) Check(domain string) (Result, error) {
 			r.trace("multiple dns records")
 			return PermError, err
 		}
-		// Could not resolve the name, it may be missing the record.
-		// https://tools.ietf.org/html/rfc7208#section-2.6.1
+		// Got another, permanent error.
+		// https://datatracker.ietf.org/doc/html/rfc7208#section-2.6.7
 		r.trace("dns perm error: %v", err)
-		return None, err
+		return PermError, err
 	}
 	r.trace("dns record %q", txt)
 
diff --git a/spf_test.go b/spf_test.go
index bae1098..d1c8ae4 100644
--- a/spf_test.go
+++ b/spf_test.go
@@ -2,7 +2,6 @@ package spf
 
 import (
 	"context"
-	"fmt"
 	"net"
 	"testing"
 
@@ -249,7 +248,10 @@ func TestNoRecord(t *testing.T) {
 	dns := NewDefaultResolver()
 	dns.Txt["d1"] = []string{""}
 	dns.Txt["d2"] = []string{"loco", "v=spf2"}
-	dns.Errors["nospf"] = fmt.Errorf("no such domain")
+	dns.Errors["nospf"] = &net.DNSError{
+		Err:        "record not found for testing",
+		IsNotFound: true,
+	}
 	defaultTrace = t.Logf
 
 	for _, domain := range []string{"d1", "d2", "d3", "nospf"} {
@@ -582,8 +584,8 @@ func TestWithContext(t *testing.T) {
 	cancelF()
 	res, err = CheckHostWithSender(ip1111, "helo", "user@domain1",
 		WithContext(ctx))
-	if res != None || err != context.Canceled {
-		t.Errorf("expected none/context cancelled, got %q / %q", res, err)
+	if res != PermError || err != context.Canceled {
+		t.Errorf("expected permerror/context cancelled, got %q / %q", res, err)
 	}
 }