commit d6c39177b7222b85f0abcae344bd232423d4eb03
author Wendy Wen Wang <wendy.wang10@sap.com> Fri Mar 18 07:29:55 2022 +0000
committer Gerrit Code Review <noreply-gerritcodereview@google.com> Fri Mar 18 07:29:55 2022 +0000
tree 419e7b617a13e83ca2eeb48a4522030d8be0db1d
parent 60b2092882a7b4193056402348d811e3ddc80ce0
parent a69e490d274e0470841f9d4d27fd2241b5022f3c

Merge "Completing set of validation for Upload Validator" into stable-3.3

src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java
index 04525ec..eaa147e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java
@@ -33,8 +33,13 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ChangeEmailValidator implements CommitValidationListener {
+  private static final Logger log = LoggerFactory.getLogger(ValidatorConfig.class);
+
   public static AbstractModule module() {
     return new AbstractModule() {
       @Override
@@ -144,8 +149,14 @@
   }
 
   @VisibleForTesting
-  static boolean performValidation(String email, String[] allowedEmailPatterns) {
-    return Arrays.stream(allowedEmailPatterns)
-        .anyMatch(s -> Pattern.matches(s, Strings.nullToEmpty(email)));
+  static boolean performValidation(String email, String[] allowedEmailPatterns)
+      throws CommitValidationException {
+    try {
+      return Arrays.stream(allowedEmailPatterns)
+          .anyMatch(s -> Pattern.matches(s, Strings.nullToEmpty(email)));
+    } catch (PatternSyntaxException e) {
+      log.error("Invalid regex '{}' in plugin (uploadvalidator) config", e.getPattern());
+      throw new CommitValidationException("Invalid regex in email pattern configuration.", e);
+    }
   }
 }

src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/PluginConfigValidator.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/PluginConfigValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/PluginConfigValidator.java
index d612229..61a47b7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/PluginConfigValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/PluginConfigValidator.java
@@ -159,14 +159,13 @@
   public ImmutableList<CommitValidationMessage> validateRegex(
       String fileName, Config cfg, String validatorKey) {
 
-    ImmutableList.Builder<CommitValidationMessage> validationMessages = ImmutableList.builder();
     String pattern = cfg.getString("plugin", pluginName, validatorKey);
 
     if (pattern != null) {
       try {
         Pattern.compile(pattern);
       } catch (PatternSyntaxException e) {
-        validationMessages.add(
+          return ImmutableList.of(
             new CommitValidationMessage(
                 String.format(
                     "The value '%s' configured in %s (parameter %s.%s) is invalid.",
@@ -174,7 +173,7 @@
                 ValidationMessage.Type.ERROR));
       }
     }
-    return validationMessages.build();
+    return ImmutableList.of();
   }
 
   /**

src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java

diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java
index 0b208f2..cd28f73 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java
@@ -15,8 +15,10 @@
 package com.googlesource.gerrit.plugins.uploadvalidator;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.EMPTY_PLUGIN_CONFIG;
 
+import com.google.gerrit.server.git.validators.CommitValidationException;
 import org.junit.Test;
 
 public class ChangeEmailTest {
@@ -28,6 +30,8 @@
     "tester@gerrit\\..*"
   };
 
+  private static final String[] illegalRegexEmailPatterns = {"*"};
+
   @Test
   public void testEmailValid() throws Exception {
     assertThat(
@@ -67,6 +71,15 @@
   }
 
   @Test
+  public void testEmailIllegalConfig() throws Exception {
+    assertThrows(
+        CommitValidationException.class,
+        () ->
+            ChangeEmailValidator.performValidation(
+                "emailtester@gerritnet.com", illegalRegexEmailPatterns));
+  }
+
+  @Test
   public void validatorBehaviorWhenAuthorConfigEmpty() {
     assertThat(ChangeEmailValidator.isAuthorActive(EMPTY_PLUGIN_CONFIG)).isFalse();
     assertThat(ChangeEmailValidator.getAllowedAuthorEmailPatterns(EMPTY_PLUGIN_CONFIG)).isEmpty();