author | Alberto Bertogli
<albertito@blitiri.com.ar> 2020-03-12 22:04:24 UTC |
committer | Alberto Bertogli
<albertito@blitiri.com.ar> 2020-03-12 22:07:22 UTC |
parent | 6641d858ad72cbf54d749d71ba5f48664420acbd |
internal/courier/smtp.go | +7 | -7 |
internal/courier/smtp_test.go | +20 | -5 |
diff --git a/internal/courier/smtp.go b/internal/courier/smtp.go index 9f0cb74..63119a0 100644 --- a/internal/courier/smtp.go +++ b/internal/courier/smtp.go @@ -68,13 +68,13 @@ func (s *SMTP) Deliver(from string, to string, data []byte) (error, bool) { a.from = "" } - mxs, err := lookupMXs(a.tr, a.toDomain) + mxs, err, perm := lookupMXs(a.tr, a.toDomain) if err != nil || len(mxs) == 0 { // Note this is considered a permanent error. // This is in line with what other servers (Exim) do. However, the // downside is that temporary DNS issues can affect delivery, so we // have to make sure we try hard enough on the lookup above. - return a.tr.Errorf("Could not find mail server: %v", err), true + return a.tr.Errorf("Could not find mail server: %v", err), perm } // Issue an EHLO with a valid domain; otherwise, some servers like postfix @@ -245,10 +245,10 @@ func (s *SMTP) fetchSTSPolicy(tr *trace.Trace, domain string) *sts.Policy { return policy } -func lookupMXs(tr *trace.Trace, domain string) ([]string, error) { +func lookupMXs(tr *trace.Trace, domain string) ([]string, error, bool) { domain, err := idna.ToASCII(domain) if err != nil { - return nil, err + return nil, err, true } mxs := []string{} @@ -264,10 +264,10 @@ func lookupMXs(tr *trace.Trace, domain string) ([]string, error) { dnsErr, ok := err.(*net.DNSError) if !ok { tr.Debugf("MX lookup error: %v", err) - return nil, err + return nil, err, true } else if dnsErr.Temporary() { tr.Debugf("temporary DNS error: %v", dnsErr) - return nil, err + return nil, err, false } // Permanent error, we assume MX does not exist and fall back to A. @@ -292,5 +292,5 @@ func lookupMXs(tr *trace.Trace, domain string) ([]string, error) { } tr.Debugf("MXs: %v", mxs) - return mxs, nil + return mxs, nil, true } diff --git a/internal/courier/smtp_test.go b/internal/courier/smtp_test.go index d9daa26..6cd83df 100644 --- a/internal/courier/smtp_test.go +++ b/internal/courier/smtp_test.go @@ -213,10 +213,13 @@ func TestTooManyMX(t *testing.T) { {Host: "h3", Pref: 30}, {Host: "h4", Pref: 40}, {Host: "h5", Pref: 50}, {Host: "h5", Pref: 60}, } - mxs, err := lookupMXs(tr, "domain") + mxs, err, perm := lookupMXs(tr, "domain") if err != nil { t.Fatalf("unexpected error: %v", err) } + if perm != true { + t.Fatalf("expected perm == true") + } if len(mxs) != 5 { t.Errorf("expected len(mxs) == 5, got: %v", mxs) } @@ -230,10 +233,13 @@ func TestFallbackToA(t *testing.T) { IsTemporary: false, } - mxs, err := lookupMXs(tr, "domain") + mxs, err, perm := lookupMXs(tr, "domain") if err != nil { t.Fatalf("unexpected error: %v", err) } + if perm != true { + t.Fatalf("expected perm == true") + } if !(len(mxs) == 1 && mxs[0] == "domain") { t.Errorf("expected mxs == [domain], got: %v", mxs) } @@ -247,10 +253,13 @@ func TestTemporaryDNSerror(t *testing.T) { IsTemporary: true, } - mxs, err := lookupMXs(tr, "domain") + mxs, err, perm := lookupMXs(tr, "domain") if !(mxs == nil && err == testMXErr["domain"]) { t.Errorf("expected mxs == nil, err == test error, got: %v, %v", mxs, err) } + if perm != false { + t.Errorf("expected perm == false") + } } func TestMXLookupError(t *testing.T) { @@ -258,19 +267,25 @@ func TestMXLookupError(t *testing.T) { testMX["domain"] = nil testMXErr["domain"] = fmt.Errorf("test error") - mxs, err := lookupMXs(tr, "domain") + mxs, err, perm := lookupMXs(tr, "domain") if !(mxs == nil && err == testMXErr["domain"]) { t.Errorf("expected mxs == nil, err == test error, got: %v, %v", mxs, err) } + if perm != true { + t.Fatalf("expected perm == true") + } } func TestLookupInvalidDomain(t *testing.T) { tr := trace.New("test", "test") - mxs, err := lookupMXs(tr, invalidDomain) + mxs, err, perm := lookupMXs(tr, invalidDomain) if !(mxs == nil && err != nil) { t.Errorf("expected err != nil, got: %v, %v", mxs, err) } + if perm != true { + t.Fatalf("expected perm == true") + } } // TODO: Test STARTTLS negotiation.