Merge "Modify EmailSender interface to optionally accept HTML bodies"

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.mail;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.errors.EmailException;
 
 import java.util.Collection;
@@ -32,7 +33,36 @@ public interface EmailSender {
   boolean canEmail(String address);
 
   /**
-   * Sends an email message.
+   * Sends an email message. Messages always contain a text body, but messages
+   * can optionally include an additional HTML body. If both body types are
+   * present, {@code send} should construct a {@code multipart/alternative}
+   * message with an appropriately-selected boundary.
+   *
+   * @param from who the message is from.
+   * @param rcpt one or more address where the message will be delivered to.
+   *        This list overrides any To or CC headers in {@code headers}.
+   * @param headers message headers.
+   * @param textBody text to appear in the {@code text/plain} body of the
+   *        message.
+   * @param htmlBody optional HTML code to appear in the {@code text/html} body
+   *        of the message.
+   * @throws EmailException the message cannot be sent.
+   */
+  default void send(Address from, Collection<Address> rcpt,
+      Map<String, EmailHeader> headers, String textBody,
+      @Nullable String htmlBody) throws EmailException {
+    send(from, rcpt, headers, textBody);
+  }
+
+  /**
+   * Sends an email message with a text body only (i.e. not HTML or multipart).
+   *
+   * Authors of new implementations of this interface should not use this method
+   * to send a message because this method does not accept the HTML body.
+   * Instead, authors should use the above signature of {@code send}.
+   *
+   * This version of the method is preserved for support of legacy
+   * implementations.
    *
    * @param from who the message is from.
    * @param rcpt one or more address where the message will be delivered to.

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

@@ -18,7 +18,6 @@ import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailSt
 import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.common.io.BaseEncoding;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
@@ -55,7 +54,6 @@ import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.ThreadLocalRandom;
 
 /** Sends an email to one or more interested parties. */
 public abstract class OutgoingEmail {
@@ -157,20 +155,11 @@ public abstract class OutgoingEmail {
       va.smtpRcptTo = smtpRcptTo;
       va.headers = headers;
 
+      va.body = textPart;
       if (useHtml()) {
-        String htmlPart = htmlBody.toString();
-        String boundary = generateMultipartBoundary(textPart, htmlPart);
-
-        va.body = buildMultipartBody(boundary, textPart, htmlPart);
-        va.textBody = textPart;
-        va.htmlBody = htmlPart;
-        va.headers.put("Content-Type", new EmailHeader.String(
-            "multipart/alternative; "
-            + "boundary=\"" + boundary + "\"; "
-            + "charset=UTF-8"));
+        va.htmlBody = htmlBody.toString();
       } else {
-        va.body = textPart;
-        va.textBody = textPart;
+        va.htmlBody = null;
       }
 
       for (OutgoingEmailValidationListener validator : args.outgoingEmailValidationListeners) {
@@ -181,53 +170,11 @@ public abstract class OutgoingEmail {
         }
       }
 
-      args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body);
+      args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers,
+          va.body, va.htmlBody);
     }
   }
 
-  protected String buildMultipartBody(String boundary, String textPart,
-      String htmlPart) {
-    return
-        // Output the text part:
-        "--" + boundary + "\r\n"
-        + "Content-Type: text/plain; charset=UTF-8\r\n"
-        + "Content-Transfer-Encoding: 8bit\r\n"
-        + "\r\n"
-        + textPart + "\r\n"
-
-        // Output the HTML part:
-        + "--" + boundary + "\r\n"
-        + "Content-Type: text/html; charset=UTF-8\r\n"
-        + "Content-Transfer-Encoding: 8bit\r\n"
-        + "\r\n"
-        + htmlPart + "\r\n"
-
-        // Output the closing boundary.
-        + "--" + boundary + "--\r\n";
-  }
-
-  protected String generateMultipartBoundary(String textBody, String htmlBody)
-      throws EmailException {
-    byte[] bytes = new byte[8];
-    ThreadLocalRandom rng = ThreadLocalRandom.current();
-
-    // The probability of the boundary being valid is approximately
-    // (2^64 - len(message)) / 2^64.
-    //
-    // The message is much shorter than 2^64 bytes, so if two tries don't
-    // suffice, something is seriously wrong.
-    for (int i = 0; i < 2; i++) {
-      rng.nextBytes(bytes);
-      String boundary = BaseEncoding.base64().encode(bytes);
-      String encBoundary = "--" + boundary;
-      if (textBody.contains(encBoundary) || htmlBody.contains(encBoundary)) {
-        continue;
-      }
-      return boundary;
-    }
-    throw new EmailException("Gave up generating unique MIME boundary");
-  }
-
   /** Format the message body by calling {@link #appendText(String)}. */
   protected abstract void format() throws EmailException;
 

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

@@ -16,7 +16,9 @@ package com.google.gerrit.server.mail;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.io.BaseEncoding;
 import com.google.common.primitives.Ints;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.Version;
 import com.google.gerrit.common.errors.EmailException;
@@ -42,6 +44,7 @@ import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
 
 /** Sends email via a nearby SMTP server. */
@@ -146,8 +149,15 @@ public class SmtpEmailSender implements EmailSender {
 
   @Override
   public void send(final Address from, Collection<Address> rcpt,
-      final Map<String, EmailHeader> callerHeaders, final String body)
+      final Map<String, EmailHeader> callerHeaders, String body)
       throws EmailException {
+    send(from, rcpt, callerHeaders, body, null);
+  }
+
+  @Override
+  public void send(final Address from, Collection<Address> rcpt,
+      final Map<String, EmailHeader> callerHeaders, String textBody,
+      @Nullable String htmlBody) throws EmailException {
     if (!isEnabled()) {
       throw new EmailException("Sending email is disabled");
     }
@@ -155,7 +165,6 @@ public class SmtpEmailSender implements EmailSender {
     final Map<String, EmailHeader> hdrs =
         new LinkedHashMap<>(callerHeaders);
     setMissingHeader(hdrs, "MIME-Version", "1.0");
-    setMissingHeader(hdrs, "Content-Type", "text/plain; charset=UTF-8");
     setMissingHeader(hdrs, "Content-Transfer-Encoding", "8bit");
     setMissingHeader(hdrs, "Content-Disposition", "inline");
     setMissingHeader(hdrs, "User-Agent", "Gerrit/" + Version.getVersion());
@@ -169,6 +178,18 @@ public class SmtpEmailSender implements EmailSender {
         new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss Z").format(expiry));
     }
 
+    String encodedBody;
+    if (htmlBody == null) {
+      setMissingHeader(hdrs, "Content-Type", "text/plain; charset=UTF-8");
+      encodedBody = textBody;
+    } else {
+      String boundary = generateMultipartBoundary(textBody, htmlBody);
+      setMissingHeader(hdrs, "Content-Type", "multipart/alternative; "
+          + "boundary=\"" + boundary + "\"; "
+          + "charset=UTF-8");
+      encodedBody = buildMultipartBody(boundary, textBody, htmlBody);
+    }
+
     StringBuffer rejected = new StringBuffer();
     try {
       final SMTPClient client = open();
@@ -214,7 +235,7 @@ public class SmtpEmailSender implements EmailSender {
           }
 
           w.write("\r\n");
-          w.write(body);
+          w.write(encodedBody);
           w.flush();
         }
 
@@ -235,6 +256,49 @@ public class SmtpEmailSender implements EmailSender {
     }
   }
 
+  public static String generateMultipartBoundary(String textBody,
+      String htmlBody) throws EmailException {
+    byte[] bytes = new byte[8];
+    ThreadLocalRandom rng = ThreadLocalRandom.current();
+
+    // The probability of the boundary being valid is approximately
+    // (2^64 - len(message)) / 2^64.
+    //
+    // The message is much shorter than 2^64 bytes, so if two tries don't
+    // suffice, something is seriously wrong.
+    for (int i = 0; i < 2; i++) {
+      rng.nextBytes(bytes);
+      String boundary = BaseEncoding.base64().encode(bytes);
+      String encBoundary = "--" + boundary;
+      if (textBody.contains(encBoundary) || htmlBody.contains(encBoundary)) {
+        continue;
+      }
+      return boundary;
+    }
+    throw new EmailException("Gave up generating unique MIME boundary");
+  }
+
+  protected String buildMultipartBody(String boundary, String textPart,
+      String htmlPart) {
+    return
+        // Output the text part:
+        "--" + boundary + "\r\n"
+        + "Content-Type: text/plain; charset=UTF-8\r\n"
+        + "Content-Transfer-Encoding: 8bit\r\n"
+        + "\r\n"
+        + textPart + "\r\n"
+
+        // Output the HTML part:
+        + "--" + boundary + "\r\n"
+        + "Content-Type: text/html; charset=UTF-8\r\n"
+        + "Content-Transfer-Encoding: 8bit\r\n"
+        + "\r\n"
+        + htmlPart + "\r\n"
+
+        // Output the closing boundary.
+        + "--" + boundary + "--\r\n";
+  }
+
   private void setMissingHeader(final Map<String, EmailHeader> hdrs,
       final String name, final String value) {
     if (!hdrs.containsKey(name) || hdrs.get(name).isEmpty()) {

gerrit-server/src/main/java/com/google/gerrit/server/validators/OutgoingEmailValidationListener.java

@@ -33,13 +33,12 @@ public interface OutgoingEmailValidationListener {
   class Args {
     // in arguments
     public String messageClass;
-    public String textBody;
     @Nullable public String htmlBody;
 
     // in/out arguments
     public Address smtpFromAddress;
     public Set<Address> smtpRcptTo;
-    public String body;
+    public String body; // The text body of the email.
     public Map<String, EmailHeader> headers;
   }