Allow to configure TLD override for outgoing emails

Email addresses are validated using the commons validator, which uses a
list of TLDs from IANA [1].  The TLD list is updated more frequently than
the commons validator is released and upgraded in Gerrit.

It is possible to configure the validator to allow extra custom TLDs, but
so far it was hard-coded only with "local".

Add a new configuration setting to allow administrators to add extra TLDs
that will be allowed by the validator. Make the default an empty list,
which is a behaviour change from the current; administrators of sites that
have users with @local email addresses will need to explicily add "local"
in the sendemail.allowTLD setting.

[1] http://data.iana.org/TLD/

Bug: Issue 5238
Change-Id: I22a14cdd46a2a2454cafd1761866c29ed1509270

Documentation/config-gerrit.txt

@@ -3792,6 +3792,13 @@ Gerrit can parse back a user's reply.
 Defaults to an empty string which adds <<sendemail.from,sendemail.from>> as
 Reply-To if inbound email is enabled and the review's author otherwise.
 
+[[sendemail.allowTLD]]sendemail.allowTLD::
++
+List of custom TLDs to allow sending emails to in addition to those specified
+in the link:http://data.iana.org/TLD/[IANA list].
++
+Defaults to an empty list, meaning no additional TLDs are allowed.
+
 [[site]]
 === Section site
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -413,10 +413,7 @@ public class AccountIT extends AbstractDaemonTest {
     List<String> emails =
         ImmutableList.of(
             "new.email@example.com",
-            "new.email@example.systems",
-
-            // Not in the list of TLDs but added to override in OutgoingEmailValidator
-            "new.email@example.local");
+            "new.email@example.systems");
     Set<String> currentEmails = getEmails();
     for (String email : emails) {
       assertThat(currentEmails).doesNotContain(email);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/EmailValidatorIT.java

@@ -18,11 +18,14 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assert_;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.inject.Inject;
 import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.lang.reflect.Field;
+import org.junit.After;
 import org.junit.Test;
 
 public class EmailValidatorIT extends AbstractDaemonTest {
@@ -30,9 +33,28 @@ public class EmailValidatorIT extends AbstractDaemonTest {
 
   @Inject private OutgoingEmailValidator validator;
 
+  @After
+  public void resetDomainValidator() throws Exception {
+    Class<?> c = Class.forName("org.apache.commons.validator.routines.DomainValidator");
+    Field f = c.getDeclaredField("inUse");
+    f.setAccessible(true);
+    f.setBoolean(c, false);
+  }
+
   @Test
-  public void validateLocalDomain() throws Exception {
-    assertThat(validator.isValid("foo@bar.local")).isTrue();
+  @GerritConfig(name = "sendemail.allowTLD", value = "example")
+  public void testCustomTopLevelDomain() throws Exception {
+    assertThat(validator.isValid("foo@bar.local")).isFalse();
+    assertThat(validator.isValid("foo@bar.example")).isTrue();
+    assertThat(validator.isValid("foo@example")).isTrue();
+  }
+
+  @Test
+  @GerritConfig(name = "sendemail.allowTLD", value = "a")
+  public void testCustomTopLevelDomainOneCharacter() throws Exception {
+    assertThat(validator.isValid("foo@bar.local")).isFalse();
+    assertThat(validator.isValid("foo@bar.a")).isTrue();
+    assertThat(validator.isValid("foo@a")).isTrue();
   }
 
   @Test

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmailValidator.java

@@ -16,9 +16,12 @@ package com.google.gerrit.server.mail.send;
 
 import static org.apache.commons.validator.routines.DomainValidator.ArrayType.GENERIC_PLUS;
 
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import org.apache.commons.validator.routines.DomainValidator;
 import org.apache.commons.validator.routines.EmailValidator;
+import org.eclipse.jgit.lib.Config;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -26,13 +29,17 @@ import org.slf4j.LoggerFactory;
 public class OutgoingEmailValidator {
   private static final Logger log = LoggerFactory.getLogger(OutgoingEmailValidator.class);
 
-  OutgoingEmailValidator() {
-    try {
-      DomainValidator.updateTLDOverride(GENERIC_PLUS, new String[] {"local"});
-    } catch (IllegalStateException e) {
-      // Should only happen in tests, where the OutgoingEmailValidator
-      // is instantiated repeatedly.
-      log.warn("Failed to update TLD override: " + e.getMessage());
+  @Inject
+  OutgoingEmailValidator(@GerritServerConfig Config config) {
+    String[] allowTLD = config.getStringList("sendemail", null, "allowTLD");
+    if (allowTLD.length != 0) {
+      try {
+          DomainValidator.updateTLDOverride(GENERIC_PLUS, allowTLD);
+      } catch (IllegalStateException e) {
+        // Should only happen in tests, where the OutgoingEmailValidator
+        // is instantiated repeatedly.
+        log.error("Failed to update TLD override: " + e.getMessage());
+      }
     }
   }