From 9539cfd34dd36a043f8c35f9217f9059e47b7726 Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertito@blitiri.com.ar>
Date: Wed, 1 Mar 2017 00:18:25 +0000
Subject: [PATCH] courier: Consider not finding any MX/A records a permanent
 error

Currently, if we can't find a mail server for a domain, we consider that
a transient failure (semi-accidentally, as we iterate over the (empty)
list of MXs and fall through the list).

It should be treated as a permanent error, in line with other DNS
issues (which is not ideal but seems to be generally accepted
behaviour).
---
 internal/courier/smtp.go      |  2 +-
 internal/courier/smtp_test.go | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/internal/courier/smtp.go b/internal/courier/smtp.go
index 3fa98ad..d633649 100644
--- a/internal/courier/smtp.go
+++ b/internal/courier/smtp.go
@@ -72,7 +72,7 @@ func (s *SMTP) Deliver(from string, to string, data []byte) (error, bool) {
 	a.stsPolicy = s.fetchSTSPolicy(a.tr, a.toDomain)
 
 	mxs, err := lookupMXs(a.tr, a.toDomain, a.stsPolicy)
-	if err != nil {
+	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
diff --git a/internal/courier/smtp_test.go b/internal/courier/smtp_test.go
index e1e624c..8b16b60 100644
--- a/internal/courier/smtp_test.go
+++ b/internal/courier/smtp_test.go
@@ -163,4 +163,19 @@ func TestSMTPErrors(t *testing.T) {
 	}
 }
 
+func TestNoMXServer(t *testing.T) {
+	fakeMX["to"] = []string{}
+
+	s, tmpDir := newSMTP(t)
+	defer os.Remove(tmpDir)
+	err, permanent := s.Deliver("me@me", "to@to", []byte("data"))
+	if err == nil {
+		t.Errorf("delivery worked, expected failure")
+	}
+	if !permanent {
+		t.Errorf("expected permanent failure, got transient (%v)", err)
+	}
+	t.Logf("got permanent failure, as expected: %v", err)
+}
+
 // TODO: Test STARTTLS negotiation.