OutgoingEmail: Check for valid email address when adding recipients

After finding error logs filled with the message:

  Not emailing NULL (prohibited by allowrcpt)

it turned out that a user's preferred email address had been set to
the literal string "NULL". Presumably this was done while running a
version of Gerrit older than 2.9, which included change I1f8d95dd9
("Validate email address when adding email or creating account").

In combination with the allowrcpt setting being enabled, this was
preventing the user from receiving any mails from Gerrit, and causing
the log to be filled with this message.

When adding recipients to an outgoing mail, check for the email address
being invalid, and log a different message in this case.

Change-Id: Id79e13f5afb1a640b5afed21a9fb50b7fc8aa5f0

1 files changed, 15 insertions, 14 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
index 8a3133ff0f..970a36c687 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
@@ -25,6 +25,7 @@ import com.google.gerrit.server.validators.ValidationException;
 import com.google.gwtorm.server.OrmException;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.commons.validator.routines.EmailValidator;
 import org.apache.velocity.Template;
 import org.apache.velocity.VelocityContext;
 import org.apache.velocity.context.InternalContextAdapterImpl;
@@ -347,21 +348,21 @@ 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) {
-      if (args.emailSender.canEmail(addr.email)) {
-        if (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;
-            case BCC:
-              break;
-          }
-        }
-      } else {
+      if (!EmailValidator.getInstance().isValid(addr.email)) {
+        log.warn("Not emailing " + addr.email + " (invalid email address)");
+      } else if (!args.emailSender.canEmail(addr.email)) {
         log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");
+      } else {
+        switch (rt) {
+          case TO:
+            ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
+            break;
+          case CC:
+            ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr);
+            break;
+          case BCC:
+            break;
+        }
       }
     }
   }