Bugfix: redirect: should be redirect= (#507)

docs/spf-optimizer.md

@@ -178,7 +178,7 @@ could exceed 512 bytes, and will require EDNS or a TCP request.
 3. Dnscontrol does not warn if the number of lookups exceeds 10.
 We hope to implement this some day.
 
-4. The `redirect:` directive is only partially implemented.  We only
+4. The `redirect=` directive is only partially implemented.  We only
 handle the case where redirect is the last item in the SPF record.
 In which case, it is equivalent to `include:`.
 

pkg/spflib/parse.go

@@ -69,14 +69,19 @@ func Parse(text string, dnsres Resolver) (*SPFRecord, error) {
 		} else if strings.HasPrefix(part, "ip4:") || strings.HasPrefix(part, "ip6:") {
 			// ip address, 0 lookups
 			continue
-		} else if strings.HasPrefix(part, "include:") || strings.HasPrefix(part, "redirect:") {
+		} else if strings.HasPrefix(part, "include:") || strings.HasPrefix(part, "redirect=") {
-			if strings.HasPrefix(part, "redirect:") {
+			// redirect is only partially implemented. redirect is a
+			// complex and IMHO ambiguously defined feature.  We only
+			// implement the most simple edge case: when it is the last item
+			// in the string.  In that situation, it is the equivalent of
+			// include:.
+			if strings.HasPrefix(part, "redirect=") {
 				// pi + 2: because pi starts at 0 when it iterates starting on parts[1],
 				// and because len(parts) is one bigger than the highest index.
 				if (pi + 2) != len(parts) {
 					return nil, errors.Errorf("%s must be last item", part)
 				}
-				p.IncludeDomain = strings.TrimPrefix(part, "redirect:")
+				p.IncludeDomain = strings.TrimPrefix(part, "redirect=")
 			} else {
 				p.IncludeDomain = strings.TrimPrefix(part, "include:")
 			}

pkg/spflib/parse_test.go

@@ -42,19 +42,46 @@ func TestParseWithDoubleSpaces(t *testing.T) {
 }
 
 func TestParseRedirectNotLast(t *testing.T) {
-	// Make sure redirect:foo fails if it isn't the last item.
+	// Make sure redirect=foo fails if it isn't the last item.
+	dnsres, err := NewCache("testdata-dns1.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = Parse(strings.Join([]string{"v=spf1",
+		"redirect=servers.mcsv.net",
+		"~all"}, " "), dnsres)
+	if err == nil {
+		t.Fatal("should fail")
+	}
+}
+
+func TestParseRedirectColon(t *testing.T) {
+	// Make sure redirect:foo fails.
 	dnsres, err := NewCache("testdata-dns1.json")
 	if err != nil {
 		t.Fatal(err)
 	}
 	_, err = Parse(strings.Join([]string{"v=spf1",
 		"redirect:servers.mcsv.net",
-		"~all"}, " "), dnsres)
+	}, " "), dnsres)
 	if err == nil {
 		t.Fatal("should fail")
 	}
 }
 
+func TestParseRedirectOnly(t *testing.T) {
+	dnsres, err := NewCache("testdata-dns1.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, err := Parse(strings.Join([]string{"v=spf1",
+		"redirect=servers.mcsv.net"}, " "), dnsres)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Log(rec.Print())
+}
+
 func TestParseRedirectLast(t *testing.T) {
 	dnsres, err := NewCache("testdata-dns1.json")
 	if err != nil {
@@ -62,7 +89,7 @@ func TestParseRedirectLast(t *testing.T) {
 	}
 	rec, err := Parse(strings.Join([]string{"v=spf1",
 		"ip4:198.252.206.0/24",
-		"redirect:servers.mcsv.net"}, " "), dnsres)
+		"redirect=servers.mcsv.net"}, " "), dnsres)
 	if err != nil {
 		t.Fatal(err)
 	}