diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java index b901057a86..2fd2d6530f 100644 --- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java +++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java @@ -109,6 +109,12 @@ public class CreateBranch + MagicBranch.getMagicRefNamePrefix(ref) + "\""); } + if (!isBranchAllowed(ref)) { + throw new BadRequestException( + "Cannot create a branch with name \"" + + ref + + "\". Not allowed to create branches under Gerrit internal or tags refs."); + } BranchNameKey name = BranchNameKey.create(rsrc.getNameKey(), ref); try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) { @@ -187,4 +193,9 @@ public class CreateBranch throw new BadRequestException("invalid revision \"" + input.revision + "\"", e); } } + + /** Branches cannot be created under any Gerrit internal or tags refs. */ + private boolean isBranchAllowed(String branch) { + return !RefNames.isGerritRef(branch) && !branch.startsWith(RefNames.REFS_TAGS); + } } diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java index a539bd5436..129b546a6c 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java @@ -190,15 +190,10 @@ public class CreateChangeIT extends AbstractDaemonTest { .add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS)) .update(); - String disallowedRef = "refs/changes/00/1000"; // All Gerrit internal refs behave the same way - requestScopeOperations.setApiUser(admin.id()); - BranchNameKey branchNameKey = BranchNameKey.create(project, disallowedRef); - createBranch(branchNameKey); - requestScopeOperations.setApiUser(user.id()); ChangeInput ci = newChangeInput(ChangeStatus.NEW); ci.subject = "Subject"; - ci.branch = disallowedRef; + ci.branch = "refs/changes/00/1000"; // disallowedRef Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci)); assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch); @@ -213,15 +208,10 @@ public class CreateChangeIT extends AbstractDaemonTest { .add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS)) .update(); - String branchName = "refs/tags/v1.0"; - requestScopeOperations.setApiUser(admin.id()); - BranchNameKey branchNameKey = BranchNameKey.create(project, branchName); - createBranch(branchNameKey); - requestScopeOperations.setApiUser(user.id()); ChangeInput ci = newChangeInput(ChangeStatus.NEW); ci.subject = "Subject"; - ci.branch = branchName; + ci.branch = "refs/tags/v1.0"; // disallowed ref Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci)); assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch); diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java index 096c72b0fd..93ce255f62 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java @@ -207,7 +207,7 @@ public class CreateBranchIT extends AbstractDaemonTest { } @Test - public void createUserBranch_Conflict() throws Exception { + public void createUserBranch_NotAllowed() throws Exception { projectOperations .project(allUsers) .forUpdate() @@ -217,12 +217,12 @@ public class CreateBranchIT extends AbstractDaemonTest { assertCreateFails( BranchNameKey.create(allUsers, RefNames.refsUsers(Account.id(1))), RefNames.refsUsers(admin.id()), - ResourceConflictException.class, - "Not allowed to create user branch."); + BadRequestException.class, + "Not allowed to create branches under Gerrit internal or tags refs."); } @Test - public void createGroupBranch_Conflict() throws Exception { + public void createGroupBranch_NotAllowed() throws Exception { projectOperations .project(allUsers) .forUpdate() @@ -232,8 +232,8 @@ public class CreateBranchIT extends AbstractDaemonTest { assertCreateFails( BranchNameKey.create(allUsers, RefNames.refsGroups(AccountGroup.uuid("foo"))), RefNames.refsGroups(adminGroupUuid()), - ResourceConflictException.class, - "Not allowed to create group branch."); + BadRequestException.class, + "Not allowed to create branches under Gerrit internal or tags refs."); } @Test @@ -354,6 +354,22 @@ public class CreateBranchIT extends AbstractDaemonTest { "not allowed to create branches under \"" + MagicBranch.NEW_CHANGE + "\""); } + @Test + public void cannotCreateBranchInGerritInternalRefsNamespace() throws Exception { + assertCreateFails( + BranchNameKey.create(project, RefNames.REFS_CHANGES + "00/1000"), + BadRequestException.class, + "Not allowed to create branches under Gerrit internal or tags refs."); + } + + @Test + public void cannotCreateBranchInTagsNamespace() throws Exception { + assertCreateFails( + BranchNameKey.create(project, RefNames.REFS_TAGS + "v1.0"), + BadRequestException.class, + "Not allowed to create branches under Gerrit internal or tags refs."); + } + @Test public void cannotCreateBranchWithInvalidName() throws Exception { assertCreateFails(