Sanitize project name on parsing.
The gerrit API can be called with a non-sanitized projet name (e.g. it
ends with "/" or ".git"). The open-source Gerrit usually rejects such
names, while internal google version sanitize it, but only in some
situations (e.g. when the repository is open). To avoid ambiguity
between different APIs, this change sanitize project name when parsing.
The BranchUtil file is extracted from the ProjectUtil to avoid complex
dependencies for the newly created project_util BUILD target.
Bug: Google b/273188083
Forward-Compatible: checked
Release-Notes: skip
Change-Id: Ia203428244c48c7cb11b4785693c7c8795a3ff6b
diff --git a/java/com/google/gerrit/entities/BUILD b/java/com/google/gerrit/entities/BUILD
index c0f5de6..dbbcf71 100644
--- a/java/com/google/gerrit/entities/BUILD
+++ b/java/com/google/gerrit/entities/BUILD
@@ -10,6 +10,7 @@
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server:project_util",
"//lib:gson",
"//lib:guava",
"//lib:jgit",
diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java
index b587b1d..b43650b 100644
--- a/java/com/google/gerrit/entities/Project.java
+++ b/java/com/google/gerrit/entities/Project.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.server.ProjectUtil;
import java.io.Serializable;
import java.util.Arrays;
import java.util.HashMap;
@@ -61,7 +62,7 @@
/** Parse a Project.NameKey out of a string representation. */
public static NameKey parse(String str) {
- return nameKey(KeyUtil.decode(str));
+ return nameKey(ProjectUtil.sanitizeProjectName(KeyUtil.decode(str)));
}
private final String name;
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 2be3383..3338464 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -14,12 +14,22 @@
"account/externalids/testing/ExternalIdTestUtil.java",
]
+PROJECT_UTIL_SRC = [
+ "ProjectUtil.java",
+]
+
java_library(
name = "constants",
srcs = CONSTANTS_SRC,
visibility = ["//visibility:public"],
)
+java_library(
+ name = "project_util",
+ srcs = PROJECT_UTIL_SRC,
+ visibility = ["//visibility:public"],
+)
+
# Giant kitchen-sink target.
#
# The only reason this hasn't been split up further is because we have too many
@@ -30,13 +40,14 @@
name = "server",
srcs = glob(
["**/*.java"],
- exclude = CONSTANTS_SRC + GERRIT_GLOBAL_MODULE_SRC + TESTING_SRC,
+ exclude = CONSTANTS_SRC + GERRIT_GLOBAL_MODULE_SRC + TESTING_SRC + PROJECT_UTIL_SRC,
),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/server"],
visibility = ["//visibility:public"],
deps = [
":constants",
+ ":project_util",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
diff --git a/java/com/google/gerrit/server/BranchUtil.java b/java/com/google/gerrit/server/BranchUtil.java
new file mode 100644
index 0000000..78f693d
--- /dev/null
+++ b/java/com/google/gerrit/server/BranchUtil.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2023 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;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import java.io.IOException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Repository;
+
+public class BranchUtil {
+ /**
+ * Checks whether the specified branch exists.
+ *
+ * @param repoManager Git repository manager to open the git repository
+ * @param branch the branch for which it should be checked if it exists
+ * @return {@code true} if the specified branch exists or if {@code HEAD} points to this branch,
+ * otherwise {@code false}
+ * @throws RepositoryNotFoundException the repository of the branch's project does not exist.
+ * @throws IOException error while retrieving the branch from the repository.
+ */
+ public static boolean branchExists(final GitRepositoryManager repoManager, BranchNameKey branch)
+ throws RepositoryNotFoundException, IOException {
+ try (Repository repo = repoManager.openRepository(branch.project())) {
+ boolean exists = repo.getRefDatabase().exactRef(branch.branch()) != null;
+ if (!exists) {
+ exists =
+ repo.getFullBranch().equals(branch.branch())
+ || RefNames.REFS_CONFIG.equals(branch.branch());
+ }
+ return exists;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/ProjectUtil.java b/java/com/google/gerrit/server/ProjectUtil.java
index fa056b3..e87c8bf 100644
--- a/java/com/google/gerrit/server/ProjectUtil.java
+++ b/java/com/google/gerrit/server/ProjectUtil.java
@@ -14,38 +14,7 @@
package com.google.gerrit.server;
-import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import java.io.IOException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Repository;
-
public class ProjectUtil {
-
- /**
- * Checks whether the specified branch exists.
- *
- * @param repoManager Git repository manager to open the git repository
- * @param branch the branch for which it should be checked if it exists
- * @return {@code true} if the specified branch exists or if {@code HEAD} points to this branch,
- * otherwise {@code false}
- * @throws RepositoryNotFoundException the repository of the branch's project does not exist.
- * @throws IOException error while retrieving the branch from the repository.
- */
- public static boolean branchExists(final GitRepositoryManager repoManager, BranchNameKey branch)
- throws RepositoryNotFoundException, IOException {
- try (Repository repo = repoManager.openRepository(branch.project())) {
- boolean exists = repo.getRefDatabase().exactRef(branch.branch()) != null;
- if (!exists) {
- exists =
- repo.getFullBranch().equals(branch.branch())
- || RefNames.REFS_CONFIG.equals(branch.branch());
- }
- return exists;
- }
- }
-
public static String sanitizeProjectName(String name) {
name = stripGitSuffix(name);
name = stripTrailingSlash(name);
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index dd0ec78d..4ce8c42 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -21,6 +21,7 @@
"//java/com/google/gerrit/json",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server:project_util",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 5fc4f41..b1f1da5 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -43,11 +43,11 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.server.BranchUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
@@ -198,7 +198,7 @@
Change change = rsrc.getChange();
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
- } else if (!ProjectUtil.branchExists(repoManager, change.getDest())) {
+ } else if (!BranchUtil.branchExists(repoManager, change.getDest())) {
throw new ResourceConflictException(
String.format("destination branch \"%s\" not found.", change.getDest().branch()));
} else if (!rsrc.getPatchSet().id().equals(change.currentPatchSetId())) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java
index de73c00..1790133 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIdIT.java
@@ -54,6 +54,28 @@
}
@Test
+ public void projectChangeNumberReturnsChangeWhenProjectEndsWithSlash() throws Exception {
+ Project.NameKey p = projectOperations.newProject().create();
+ ChangeInfo ci = gApi.changes().create(new ChangeInput(p.get(), "master", "msg")).get();
+
+ ChangeInfo changeInfo = gApi.changes().id(p.get() + "/", ci._number).get();
+
+ assertThat(changeInfo.changeId).isEqualTo(ci.changeId);
+ assertThat(changeInfo.project).isEqualTo(p.get());
+ }
+
+ @Test
+ public void projectChangeNumberReturnsChangeWhenProjectEndsWithDotGit() throws Exception {
+ Project.NameKey p = projectOperations.newProject().create();
+ ChangeInfo ci = gApi.changes().create(new ChangeInput(p.get(), "master", "msg")).get();
+
+ ChangeInfo changeInfo = gApi.changes().id(p.get() + ".git", ci._number).get();
+
+ assertThat(changeInfo.changeId).isEqualTo(ci.changeId);
+ assertThat(changeInfo.project).isEqualTo(p.get());
+ }
+
+ @Test
public void wrongProjectInProjectChangeNumberReturnsNotFound() throws Exception {
ResourceNotFoundException thrown =
assertThrows(
