Move allowrcpt handling up to EmailSender to fix empty envelope

If nobody on the SMTP envelope was actually on the allowrcpt list,
we were still trying to push a message into the SMTP server but
it had no destinations.  All SMTP servers refuse to accept the
'DATA' command in this case, as there was no 'RCPT TO' before it.

The breakage occurred when I stopped CC'ing the sender of emails
by default.  This meant that I was no longer on the RCPT TO list
for a message in my test environment, but that was the only valid
address in the sendemail.allowrcpt configuration variable.

Change-Id: I2e1810d3e2a8e82aa96c93f2cfd9849f7bed6e8a
Signed-off-by: Shawn O. Pearce <sop@google.com>

gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailSender.java

@@ -21,6 +21,14 @@ import java.util.Map;
 public interface EmailSender {
   boolean isEnabled();
 
+  /**
+   * Can the address receive messages from us?
+   *
+   * @param address the address to consider.
+   * @return true if this sender will deliver to the address.
+   */
+  boolean canEmail(String address);
+
   /**
    * Sends an email message.
    *

gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java

@@ -45,6 +45,8 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 
 import org.eclipse.jgit.util.SystemReader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -65,6 +67,8 @@ import javax.annotation.Nullable;
 
 /** Sends an email to one or more interested parties. */
 public abstract class OutgoingEmail {
+  private static final Logger log = LoggerFactory.getLogger(OutgoingEmail.class);
+
   private static final String HDR_TO = "To";
   private static final String HDR_CC = "CC";
 
@@ -180,10 +184,6 @@ public abstract class OutgoingEmail {
           // If they don't want a copy, but we queued one up anyway,
           // drop them from the recipient lists.
           //
-          if (rcptTo.isEmpty()) {
-            return;
-          }
-
           final String fromEmail = fromUser.getPreferredEmail();
           for (Iterator<Address> i = smtpRcptTo.iterator(); i.hasNext();) {
             if (i.next().email.equals(fromEmail)) {
@@ -195,6 +195,10 @@ public abstract class OutgoingEmail {
               ((AddressList) hdr).remove(fromEmail);
             }
           }
+
+          if (smtpRcptTo.isEmpty()) {
+            return;
+          }
         }
       }
 
@@ -564,7 +568,7 @@ public abstract class OutgoingEmail {
       return false;
     }
 
-    if (rcptTo.isEmpty()) {
+    if (smtpRcptTo.isEmpty()) {
       // If we have nobody to send this message to, then all of our
       // selection filters previously for this type of message were
       // unable to match a destination. Don't bother sending it.
@@ -724,14 +728,18 @@ public abstract class OutgoingEmail {
   /** Schedule delivery of this message to the given account. */
   protected void add(final RecipientType rt, final Address addr) {
     if (addr != null && addr.email != null && addr.email.length() > 0) {
-      smtpRcptTo.add(addr);
-      switch (rt) {
-        case TO:
-          ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
-          break;
-        case CC:
-          ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
-          break;
+      if (emailSender.canEmail(addr.email)) {
+        smtpRcptTo.add(addr);
+        switch (rt) {
+          case TO:
+            ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
+            break;
+          case CC:
+            ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
+            break;
+        }
+      } else {
+        log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");
       }
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java

@@ -29,8 +29,11 @@ import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.Writer;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Set;
 
 /** Sends email via a nearby SMTP server. */
 @Singleton
@@ -47,7 +50,7 @@ public class SmtpEmailSender implements EmailSender {
   private String smtpPass;
   private Encryption smtpEncryption;
   private boolean sslVerify;
-  private String[] allowrcpt;
+  private Set<String> allowrcpt;
 
   @Inject
   SmtpEmailSender(@GerritServerConfig final Config cfg) {
@@ -79,7 +82,12 @@ public class SmtpEmailSender implements EmailSender {
 
     smtpUser = cfg.getString("sendemail", null, "smtpuser");
     smtpPass = cfg.getString("sendemail", null, "smtppass");
-    allowrcpt = cfg.getStringList("sendemail", null, "allowrcpt");
+
+    Set<String> rcpt = new HashSet<String>();
+    for (String addr : cfg.getStringList("sendemail", null, "allowrcpt")) {
+      rcpt.add(addr);
+    }
+    allowrcpt = Collections.unmodifiableSet(rcpt);
   }
 
   @Override
@@ -88,7 +96,29 @@ public class SmtpEmailSender implements EmailSender {
   }
 
   @Override
-  public void send(final Address from, final Collection<Address> rcpt,
+  public boolean canEmail(String address) {
+    if (!isEnabled()) {
+      return false;
+    }
+
+    if (allowrcpt.isEmpty()) {
+      return true;
+    }
+
+    if (allowrcpt.contains(address)) {
+      return true;
+    }
+
+    String domain = address.substring(address.lastIndexOf('@') + 1);
+    if (allowrcpt.contains(domain) || allowrcpt.contains("@" + domain)) {
+      return true;
+    }
+
+    return false;
+  }
+
+  @Override
+  public void send(final Address from, Collection<Address> rcpt,
       final Map<String, EmailHeader> callerHeaders, final String body)
       throws EmailException {
     if (!isEnabled()) {
@@ -161,7 +191,6 @@ public class SmtpEmailSender implements EmailSender {
 
   private SMTPClient open() throws EmailException {
     final AuthSMTPClient client = new AuthSMTPClient("UTF-8");
-    client.setAllowRcpt(allowrcpt);
 
     if (smtpEncryption == Encryption.SSL) {
       client.enableSSL(sslVerify);