Throw specific exception caused by invalid email regex in config
Users will see invalid pattern error instead of internal server error.
At present, attempting to push when the project config has an illegible
email pattern restriction such as a single asterisk results in an
internal server error and failed commit. With this change, the response
provides greater clarity.
This is not the solution to allowing illegible email patterns to be set,
but it clarifies what is preventing the commit from going through.
Change-Id: I66447139a3611b81c5c8805c263015702d9f2f5b
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);
+ }
}
}
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();
