diff --git a/docs/spf-optimizer.md b/docs/spf-optimizer.md index 142a074a3..9f4d7c5e2 100644 --- a/docs/spf-optimizer.md +++ b/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:`. diff --git a/pkg/spflib/parse.go b/pkg/spflib/parse.go index 19491a254..41a78c4e7 100644 --- a/pkg/spflib/parse.go +++ b/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:") { - if strings.HasPrefix(part, "redirect:") { + } else if strings.HasPrefix(part, "include:") || 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:") } diff --git a/pkg/spflib/parse_test.go b/pkg/spflib/parse_test.go index 1b628e10a..1c05611bd 100644 --- a/pkg/spflib/parse_test.go +++ b/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) }