commit 0f9499157f28b36daf42ab660e6fe82ae12cca5f
author Jonathan Nieder <jrn@google.com> Wed Nov 15 19:12:51 2017 -0800
committer Jonathan Nieder <jrn@google.com> Wed Nov 15 20:34:11 2017 -0800
tree 5615a202d9a997508208b6bf42850ebf96e3b0f9
parent e1d418cf11e15905b8f37b26283906951b882062

Cope with accounts with missing preferred email

Some gerrit accounts don't have a preferred email set.  Instead of
failing with NullPointerException, treat these as not matching any
email pattern.

Change-Id: I9089d821befc194d15c26057725278649a6986f1

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

diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
index ef0030f..c4ae334 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
@@ -14,6 +14,7 @@
 
 package com.googlesource.gerrit.plugins.uploadvalidator;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.RefConfigSection;
 import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
@@ -126,7 +127,7 @@
     return matchCriteria(config, "ref", ref, true, true);
   }
 
-  private boolean activeForEmail(PluginConfig config, String email) {
+  private boolean activeForEmail(PluginConfig config, @Nullable String email) {
     return matchCriteria(config, "email", email, true, false);
   }
 
@@ -139,15 +140,22 @@
   }
 
   private boolean matchCriteria(
-      PluginConfig config, String criteria, String value, boolean allowRegex, boolean refMatcher) {
-    boolean match = true;
-    for (String s : config.getStringList(criteria)) {
-      if ((allowRegex && match(value, s, refMatcher)) || (!allowRegex && s.equals(value))) {
-        return true;
-      }
-      match = false;
+      PluginConfig config,
+      String criteria,
+      @Nullable String value,
+      boolean allowRegex,
+      boolean refMatcher) {
+    String[] c = config.getStringList(criteria);
+    if (c.length == 0) {
+      return true;
     }
-    return match;
+    if (value == null) {
+      return false;
+    }
+    if (allowRegex) {
+      return Arrays.stream(c).anyMatch(s -> match(value, s, refMatcher));
+    }
+    return Arrays.asList(c).contains(value);
   }
 
   private static boolean match(String value, String pattern, boolean refMatcher) {

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

diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
index b385259..8bdb16c 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
@@ -35,6 +35,16 @@
   }
 
   @Test
+  public void isEnabledForMissingEmailByDefault() throws Exception {
+    IdentifiedUser missingEmail = new FakeUserProvider().get(null);
+    ValidatorConfig config =
+        getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar");
+
+    assertThat(config.isEnabledForRef(missingEmail, projectName, "anyRef", "blockedFileExtension"))
+        .isTrue();
+  }
+
+  @Test
   public void isEnabledForSingleEmail() throws Exception {
     ValidatorConfig config =
         getConfig(
@@ -84,6 +94,21 @@
   }
 
   @Test
+  public void missingEmailDoesNotMatchRegex() throws Exception {
+    IdentifiedUser missingEmail = new FakeUserProvider().get(null);
+    ValidatorConfig config =
+        getConfig(
+            "[plugin \"uploadvalidator\"]\n"
+                + "   email = .*$\n"
+                + "   blockedFileExtension = jar");
+
+    assertThat(
+            config.isEnabledForRef(
+                missingEmail, projectName, "refs/heads/anyref", "blockedFileExtension"))
+        .isFalse();
+  }
+
+  @Test
   public void isEnabledForMultipleEmails() throws Exception {
     IdentifiedUser exampleOrgUser = new FakeUserProvider().get("a@example.org");
     IdentifiedUser xUser = new FakeUserProvider().get("x@example.com");