Merge "Prevent creation of invalid labels by Prolog rules"

java/com/google/gerrit/server/rules/PrologRuleEvaluator.java

@@ -18,7 +18,10 @@ import static com.google.gerrit.server.project.SubmitRuleEvaluator.createRuleErr
 import static com.google.gerrit.server.project.SubmitRuleEvaluator.defaultRuleError;
 import static com.google.gerrit.server.project.SubmitRuleEvaluator.defaultTypeError;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.CharMatcher;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.extensions.client.SubmitType;
@@ -58,6 +61,15 @@ import java.util.List;
  */
 public class PrologRuleEvaluator {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+  /**
+   * List of characters to allow in the label name, when an invalid name is used. Dash is allowed as
+   * it can't be the first character: we use a prefix.
+   */
+  private static final CharMatcher VALID_LABEL_MATCHER =
+      CharMatcher.is('-')
+          .or(CharMatcher.inRange('a', 'z'))
+          .or(CharMatcher.inRange('A', 'Z'))
+          .or(CharMatcher.inRange('0', '9'));
 
   public interface Factory {
     /** Returns a new {@link PrologRuleEvaluator} with the specified options */
@@ -231,7 +243,7 @@ public class PrologRuleEvaluator {
         SubmitRecord.Label lbl = new SubmitRecord.Label();
         rec.labels.add(lbl);
 
-        lbl.label = state.arg(0).name();
+        lbl.label = checkLabelName(state.arg(0).name());
         Term status = state.arg(1);
 
         try {
@@ -280,6 +292,19 @@ public class PrologRuleEvaluator {
     return out;
   }
 
+  @VisibleForTesting
+  static String checkLabelName(String name) {
+    try {
+      return LabelType.checkName(name);
+    } catch (IllegalArgumentException e) {
+      return LabelType.checkName("Invalid-Prolog-Rules-Label-Name--" + sanitizeLabelName(name));
+    }
+  }
+
+  private static String sanitizeLabelName(String name) {
+    return VALID_LABEL_MATCHER.retainFrom(name);
+  }
+
   private List<SubmitRecord> invalidResult(Term rule, Term record, String reason) {
     return ruleError(
         String.format(

javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java

@@ -0,0 +1,47 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.rules;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class PrologRuleEvaluatorTest {
+
+  @Test
+  public void validLabelNamesAreKept() {
+    for (String labelName : new String[] {"Verified", "Code-Review"}) {
+      assertThat(PrologRuleEvaluator.checkLabelName(labelName)).isEqualTo(labelName);
+    }
+  }
+
+  @Test
+  public void labelWithSpacesIsTransformed() {
+    assertThat(PrologRuleEvaluator.checkLabelName("Label with spaces"))
+        .isEqualTo("Invalid-Prolog-Rules-Label-Name--Labelwithspaces");
+  }
+
+  @Test
+  public void labelStartingWithADashIsTransformed() {
+    assertThat(PrologRuleEvaluator.checkLabelName("-dashed-label"))
+        .isEqualTo("Invalid-Prolog-Rules-Label-Name---dashed-label");
+  }
+
+  @Test
+  public void labelWithInvalidCharactersIsTransformed() {
+    assertThat(PrologRuleEvaluator.checkLabelName("*urgent*"))
+        .isEqualTo("Invalid-Prolog-Rules-Label-Name--urgent");
+  }
+}