Move all static ref pattern related methods into a RefPattern class
The goal of this refactoring is to have a single place where a ref
pattern is validated as regular expression. So far this was done in 2
places (in RefControl and in ProjectConfig). For validating a ref
pattern as regular expression we must remove placeholder variables
such as '${username}' because it would be an invalid regular
expression otherwise. We don't want to do this in several places
especially because we want to support further placeholder variables in
future.
Change-Id: I11c76e37f22233d89b631375ac6f2ef60f97801f
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index 980b284..4a01128 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -37,7 +37,7 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
-import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.project.RefPattern;
import com.google.gerrit.server.project.SetParent;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
@@ -111,7 +111,7 @@
continue;
}
- RefControl.validateRefPattern(name);
+ RefPattern.validate(name);
replace(config, toDelete, section);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index a0bc332..089afe8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -40,6 +40,7 @@
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.common.data.RefConfigSection;
import com.google.gerrit.common.data.SubscribeSection;
+import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
@@ -53,6 +54,7 @@
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.project.CommentLinkInfo;
+import com.google.gerrit.server.project.RefPattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -645,8 +647,8 @@
private boolean isValidRegex(String refPattern) {
try {
- Pattern.compile(refPattern.replace("${username}/", ""));
- } catch (PatternSyntaxException e) {
+ RefPattern.validateRegExp(refPattern);
+ } catch (InvalidNameException e) {
error(new ValidationError(PROJECT_CONFIG, "Invalid ref name: "
+ e.getMessage()));
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
index ee6fac7..a862ac2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.project;
import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.gerrit.server.project.RefControl.isRE;
+import static com.google.gerrit.server.project.RefPattern.isRE;
import com.google.auto.value.AutoValue;
import com.google.common.collect.Lists;
@@ -74,7 +74,7 @@
PermissionCollection filter(Iterable<SectionMatcher> matcherList,
String ref, CurrentUser user) {
if (isRE(ref)) {
- ref = RefControl.shortestExample(ref);
+ ref = RefPattern.shortestExample(ref);
} else if (ref.endsWith("/*")) {
ref = ref.substring(0, ref.length() - 1);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index cbe7fba..0e60f3f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -14,20 +14,15 @@
package com.google.gerrit.server.project;
-import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
-import com.google.gerrit.common.data.RefConfigSection;
-import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.SystemGroupBackend;
-import dk.brics.automaton.RegExp;
-
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
@@ -47,8 +42,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
/** Manages access control for Git references (aka branches, tags). */
@@ -646,53 +639,4 @@
effective.put(permissionName, mine);
return mine;
}
-
- public static boolean isRE(String refPattern) {
- return refPattern.startsWith(AccessSection.REGEX_PREFIX);
- }
-
- public static String shortestExample(String pattern) {
- if (isRE(pattern)) {
- // Since Brics will substitute dot [.] with \0 when generating
- // shortest example, any usage of dot will fail in
- // Repository.isValidRefName() if not combined with star [*].
- // To get around this, we substitute the \0 with an arbitrary
- // accepted character.
- return toRegExp(pattern).toAutomaton().getShortestExample(true).replace('\0', '-');
- } else if (pattern.endsWith("/*")) {
- return pattern.substring(0, pattern.length() - 1) + '1';
- } else {
- return pattern;
- }
- }
-
- public static RegExp toRegExp(String refPattern) {
- if (isRE(refPattern)) {
- refPattern = refPattern.substring(1);
- }
- return new RegExp(refPattern, RegExp.NONE);
- }
-
- public static void validateRefPattern(String refPattern)
- throws InvalidNameException {
- if (refPattern.startsWith(RefConfigSection.REGEX_PREFIX)) {
- if (!Repository.isValidRefName(RefControl.shortestExample(refPattern))) {
- throw new InvalidNameException(refPattern);
- }
- } else if (refPattern.equals(RefConfigSection.ALL)) {
- // This is a special case we have to allow, it fails below.
- } else if (refPattern.endsWith("/*")) {
- String prefix = refPattern.substring(0, refPattern.length() - 2);
- if (!Repository.isValidRefName(prefix)) {
- throw new InvalidNameException(refPattern);
- }
- } else if (!Repository.isValidRefName(refPattern)) {
- throw new InvalidNameException(refPattern);
- }
- try {
- Pattern.compile(refPattern.replace("${username}/", ""));
- } catch (PatternSyntaxException e) {
- throw new InvalidNameException(refPattern + " " + e.getMessage());
- }
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
new file mode 100644
index 0000000..2b03951
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPattern.java
@@ -0,0 +1,85 @@
+// Copyright (C) 2016 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.project;
+
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.RefConfigSection;
+import com.google.gerrit.common.errors.InvalidNameException;
+
+import dk.brics.automaton.RegExp;
+
+import org.eclipse.jgit.lib.Repository;
+
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+public class RefPattern {
+ public static final String USERNAME = "username";
+
+ public static String shortestExample(String refPattern) {
+ if (isRE(refPattern)) {
+ // Since Brics will substitute dot [.] with \0 when generating
+ // shortest example, any usage of dot will fail in
+ // Repository.isValidRefName() if not combined with star [*].
+ // To get around this, we substitute the \0 with an arbitrary
+ // accepted character.
+ return toRegExp(refPattern).toAutomaton().getShortestExample(true)
+ .replace('\0', '-');
+ } else if (refPattern.endsWith("/*")) {
+ return refPattern.substring(0, refPattern.length() - 1) + '1';
+ } else {
+ return refPattern;
+ }
+ }
+
+ public static boolean isRE(String refPattern) {
+ return refPattern.startsWith(AccessSection.REGEX_PREFIX);
+ }
+
+ public static RegExp toRegExp(String refPattern) {
+ if (isRE(refPattern)) {
+ refPattern = refPattern.substring(1);
+ }
+ return new RegExp(refPattern, RegExp.NONE);
+ }
+
+ public static void validate(String refPattern)
+ throws InvalidNameException {
+ if (refPattern.startsWith(RefConfigSection.REGEX_PREFIX)) {
+ if (!Repository.isValidRefName(shortestExample(refPattern))) {
+ throw new InvalidNameException(refPattern);
+ }
+ } else if (refPattern.equals(RefConfigSection.ALL)) {
+ // This is a special case we have to allow, it fails below.
+ } else if (refPattern.endsWith("/*")) {
+ String prefix = refPattern.substring(0, refPattern.length() - 2);
+ if (!Repository.isValidRefName(prefix)) {
+ throw new InvalidNameException(refPattern);
+ }
+ } else if (!Repository.isValidRefName(refPattern)) {
+ throw new InvalidNameException(refPattern);
+ }
+ validateRegExp(refPattern);
+ }
+
+ public static void validateRegExp(String refPattern)
+ throws InvalidNameException {
+ try {
+ Pattern.compile(refPattern.replace("${" + USERNAME + "}/", ""));
+ } catch (PatternSyntaxException e) {
+ throw new InvalidNameException(refPattern + " " + e.getMessage());
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java
index 96b9e10..97f70b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefPatternMatcher.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.project;
-import static com.google.gerrit.server.project.RefControl.isRE;
+import static com.google.gerrit.server.project.RefPattern.isRE;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -93,12 +93,13 @@
// in a reference and the string :USERNAME: is not likely to
// be a valid part of the regex. This later allows the pattern
// prefix to be clipped, saving time on evaluation.
+ String replacement = ":" + RefPattern.USERNAME.toUpperCase() + ":";
Automaton am =
- RefControl.toRegExp(
- template.replace(Collections.singletonMap("username",
- ":USERNAME:"))).toAutomaton();
+ RefPattern.toRegExp(
+ template.replace(Collections.singletonMap(RefPattern.USERNAME,
+ replacement))).toAutomaton();
String rePrefix = am.getCommonPrefix();
- prefix = rePrefix.substring(0, rePrefix.indexOf(":USERNAME:"));
+ prefix = rePrefix.substring(0, rePrefix.indexOf(replacement));
} else {
prefix = pattern.substring(0, pattern.indexOf("${"));
}
@@ -154,8 +155,8 @@
}
private String expand(ParameterizedString parameterizedRef, String userName) {
- return parameterizedRef.replace(Collections.singletonMap("username",
- userName));
+ return parameterizedRef
+ .replace(Collections.singletonMap(RefPattern.USERNAME, userName));
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java
index 258b386..c0d23b9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetAccess.java
@@ -146,7 +146,7 @@
throw new AuthException("You are not allowed to edit permissions"
+ "for ref: " + name);
}
- RefControl.validateRefPattern(name);
+ RefPattern.validate(name);
}
// Check all permissions for soundness
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/MostSpecificComparator.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/MostSpecificComparator.java
index ce7f24d..159763c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/MostSpecificComparator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/MostSpecificComparator.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.util;
import com.google.gerrit.common.data.RefConfigSection;
-import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.project.RefPattern;
import org.apache.commons.lang.StringUtils;
@@ -82,8 +82,8 @@
private int distance(String pattern) {
String example;
- if (RefControl.isRE(pattern)) {
- example = RefControl.shortestExample(pattern);
+ if (RefPattern.isRE(pattern)) {
+ example = RefPattern.shortestExample(pattern);
} else if (pattern.endsWith("/*")) {
example = pattern;
@@ -98,8 +98,8 @@
}
private boolean finite(String pattern) {
- if (RefControl.isRE(pattern)) {
- return RefControl.toRegExp(pattern).toAutomaton().isFinite();
+ if (RefPattern.isRE(pattern)) {
+ return RefPattern.toRegExp(pattern).toAutomaton().isFinite();
} else if (pattern.endsWith("/*")) {
return false;
@@ -110,8 +110,8 @@
}
private int transitions(String pattern) {
- if (RefControl.isRE(pattern)) {
- return RefControl.toRegExp(pattern).toAutomaton()
+ if (RefPattern.isRE(pattern)) {
+ return RefPattern.toRegExp(pattern).toAutomaton()
.getNumberOfTransitions();
} else if (pattern.endsWith("/*")) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 1e65560..073cfec 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -823,27 +823,26 @@
@Test
public void testValidateRefPatternsOK() throws Exception {
- RefControl.validateRefPattern("refs/*");
- RefControl.validateRefPattern("^refs/heads/*");
- RefControl.validateRefPattern("^refs/tags/[0-9a-zA-Z-_.]+");
- RefControl.validateRefPattern("refs/heads/review/${username}/*");
+ RefPattern.validate("refs/*");
+ RefPattern.validate("^refs/heads/*");
+ RefPattern.validate("^refs/tags/[0-9a-zA-Z-_.]+");
+ RefPattern.validate("refs/heads/review/${username}/*");
}
@Test(expected = InvalidNameException.class)
public void testValidateBadRefPatternDoubleCaret() throws Exception {
- RefControl.validateRefPattern("^^refs/*");
+ RefPattern.validate("^^refs/*");
}
@Test(expected = InvalidNameException.class)
public void testValidateBadRefPatternDanglingCharacter() throws Exception {
- RefControl
- .validateRefPattern("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*");
+ RefPattern
+ .validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*");
}
@Test
public void testValidateRefPatternNoDanglingCharacter() throws Exception {
- RefControl
- .validateRefPattern("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}");
+ RefPattern.validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}");
}
private InMemoryRepository add(ProjectConfig pc) {
