Merge branch 'stable-3.0' into stable-3.1 * stable-3.0: QueryAccount/QueryGroups: Enable retries MergeSuperSet: Include change ID into error message DeleteBranch/DeleteBranches: Disallow deletion of HEAD to prevent NPE DeleteBranches: Disallow deletion of refs/meta/config branch Change-Id: I9e5fb05c39b69847c8af9bca09a2d7d3979b184d
diff --git a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java index a40cdd0..f9e753c 100644 --- a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java +++ b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
@@ -43,6 +43,7 @@ import com.google.gerrit.server.query.account.AccountQueryBuilder; import com.google.gerrit.server.query.account.AccountQueryProcessor; import com.google.inject.Inject; +import com.google.inject.Provider; import java.util.Collections; import java.util.EnumSet; import java.util.LinkedHashMap; @@ -58,12 +59,13 @@ private final PermissionBackend permissionBackend; private final AccountLoader.Factory accountLoaderFactory; private final AccountQueryBuilder queryBuilder; - private final AccountQueryProcessor queryProcessor; + private final Provider<AccountQueryProcessor> queryProcessorProvider; private final boolean suggestConfig; private final int suggestFrom; private AccountLoader accountLoader; private boolean suggest; + private Integer limit; private int suggestLimit = 10; private String query; private Integer start; @@ -80,7 +82,7 @@ metaVar = "CNT", usage = "maximum number of users to return") public void setLimit(int n) { - queryProcessor.setUserProvidedLimit(n); + this.limit = n; if (n < 0) { suggestLimit = 10; @@ -124,12 +126,12 @@ PermissionBackend permissionBackend, AccountLoader.Factory accountLoaderFactory, AccountQueryBuilder queryBuilder, - AccountQueryProcessor queryProcessor, + Provider<AccountQueryProcessor> queryProcessorProvider, @GerritServerConfig Config cfg) { this.permissionBackend = permissionBackend; this.accountLoaderFactory = accountLoaderFactory; this.queryBuilder = queryBuilder; - this.queryProcessor = queryProcessor; + this.queryProcessorProvider = queryProcessorProvider; this.suggestFrom = cfg.getInt("suggest", null, "from", 0); this.options = EnumSet.noneOf(ListAccountsOption.class); @@ -186,10 +188,15 @@ } accountLoader = accountLoaderFactory.create(fillOptions); + AccountQueryProcessor queryProcessor = queryProcessorProvider.get(); if (queryProcessor.isDisabled()) { throw new MethodNotAllowedException("query disabled"); } + if (limit != null) { + queryProcessor.setUserProvidedLimit(limit); + } + if (start != null) { queryProcessor.setStart(start); }
diff --git a/java/com/google/gerrit/server/restapi/group/QueryGroups.java b/java/com/google/gerrit/server/restapi/group/QueryGroups.java index 816fcf6..a233111 100644 --- a/java/com/google/gerrit/server/restapi/group/QueryGroups.java +++ b/java/com/google/gerrit/server/restapi/group/QueryGroups.java
@@ -32,6 +32,7 @@ import com.google.gerrit.server.query.group.GroupQueryBuilder; import com.google.gerrit.server.query.group.GroupQueryProcessor; import com.google.inject.Inject; +import com.google.inject.Provider; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; @@ -39,7 +40,7 @@ public class QueryGroups implements RestReadView<TopLevelResource> { private final GroupQueryBuilder queryBuilder; - private final GroupQueryProcessor queryProcessor; + private final Provider<GroupQueryProcessor> queryProcessorProvider; private final GroupJson json; private String query; @@ -88,9 +89,11 @@ @Inject protected QueryGroups( - GroupQueryBuilder queryBuilder, GroupQueryProcessor queryProcessor, GroupJson json) { + GroupQueryBuilder queryBuilder, + Provider<GroupQueryProcessor> queryProcessorProvider, + GroupJson json) { this.queryBuilder = queryBuilder; - this.queryProcessor = queryProcessor; + this.queryProcessorProvider = queryProcessorProvider; this.json = json; } @@ -101,6 +104,8 @@ throw new BadRequestException("missing query field"); } + GroupQueryProcessor queryProcessor = queryProcessorProvider.get(); + if (queryProcessor.isDisabled()) { throw new MethodNotAllowedException("query disabled"); }
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteBranch.java b/java/com/google/gerrit/server/restapi/project/DeleteBranch.java index 2a20283..6248a61 100644 --- a/java/com/google/gerrit/server/restapi/project/DeleteBranch.java +++ b/java/com/google/gerrit/server/restapi/project/DeleteBranch.java
@@ -17,6 +17,7 @@ import static com.google.gerrit.entities.RefNames.isConfigRef; import static org.eclipse.jgit.lib.Constants.R_HEADS; +import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.common.Input; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -46,7 +47,9 @@ @Override public Response<?> apply(BranchResource rsrc, Input input) throws RestApiException, IOException, PermissionBackendException { - if (isConfigRef(rsrc.getBranchKey().branch())) { + if (RefNames.HEAD.equals(rsrc.getBranchKey().branch())) { + throw new MethodNotAllowedException("not allowed to delete HEAD"); + } else if (isConfigRef(rsrc.getBranchKey().branch())) { // Never allow to delete the meta config branch. throw new MethodNotAllowedException( "not allowed to delete branch " + rsrc.getBranchKey().branch());
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteBranches.java b/java/com/google/gerrit/server/restapi/project/DeleteBranches.java index d429655..ca5962e 100644 --- a/java/com/google/gerrit/server/restapi/project/DeleteBranches.java +++ b/java/com/google/gerrit/server/restapi/project/DeleteBranches.java
@@ -17,8 +17,10 @@ import static org.eclipse.jgit.lib.Constants.R_HEADS; import com.google.common.collect.ImmutableSet; +import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.api.projects.DeleteBranchesInput; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestModifyView; @@ -43,6 +45,14 @@ if (input == null || input.branches == null || input.branches.isEmpty()) { throw new BadRequestException("branches must be specified"); } + + if (input.branches.contains(RefNames.HEAD)) { + throw new MethodNotAllowedException("not allowed to delete HEAD"); + } else if (input.branches.stream().anyMatch(RefNames::isConfigRef)) { + // Never allow to delete the meta config branch. + throw new MethodNotAllowedException("not allowed to delete branch " + RefNames.REFS_CONFIG); + } + deleteRef.deleteMultipleRefs( project.getProjectState(), ImmutableSet.copyOf(input.branches), R_HEADS); return Response.none();
diff --git a/java/com/google/gerrit/server/submit/MergeSuperSet.java b/java/com/google/gerrit/server/submit/MergeSuperSet.java index c690e0b..bcebc7f 100644 --- a/java/com/google/gerrit/server/submit/MergeSuperSet.java +++ b/java/com/google/gerrit/server/submit/MergeSuperSet.java
@@ -99,7 +99,11 @@ closeOrm = true; } List<ChangeData> cds = queryProvider.get().byLegacyChangeId(change.getId()); - checkState(cds.size() == 1, "Expected exactly one ChangeData, got " + cds.size()); + checkState( + cds.size() == 1, + "Expected exactly one ChangeData for change ID %s, got %s", + change.getId(), + cds.size()); ChangeData cd = Iterables.getFirst(cds, null); boolean visible = false;
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java index 4f1be30..5636014 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java
@@ -33,6 +33,7 @@ import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.IdString; +import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.inject.Inject; @@ -172,6 +173,24 @@ assertThat(thrown).hasMessageThat().contains("Not allowed to delete group branch."); } + @Test + public void cannotDeleteRefsMetaConfig() throws Exception { + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> branch(BranchNameKey.create(allUsers, RefNames.REFS_CONFIG)).delete()); + assertThat(thrown).hasMessageThat().contains("not allowed to delete branch refs/meta/config"); + } + + @Test + public void cannotDeleteHead() throws Exception { + MethodNotAllowedException thrown = + assertThrows( + MethodNotAllowedException.class, + () -> branch(BranchNameKey.create(allUsers, RefNames.HEAD)).delete()); + assertThat(thrown).hasMessageThat().contains("not allowed to delete HEAD"); + } + private void blockForcePush() throws Exception { projectOperations .project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java index 3cdeafd..ad90109 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
@@ -35,6 +35,7 @@ import com.google.gerrit.extensions.api.projects.ProjectApi; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.inject.Inject; import java.util.HashMap; @@ -156,6 +157,26 @@ assertThat(thrown).hasMessageThat().contains("branches must be specified"); } + @Test + public void cannotDeleteRefsMetaConfig() throws Exception { + DeleteBranchesInput input = new DeleteBranchesInput(); + input.branches = Lists.newArrayList(); + input.branches.add(RefNames.REFS_CONFIG); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> project().deleteBranches(input)); + assertThat(thrown).hasMessageThat().contains("not allowed to delete branch refs/meta/config"); + } + + @Test + public void cannotDeleteHead() throws Exception { + DeleteBranchesInput input = new DeleteBranchesInput(); + input.branches = Lists.newArrayList(); + input.branches.add(RefNames.HEAD); + MethodNotAllowedException thrown = + assertThrows(MethodNotAllowedException.class, () -> project().deleteBranches(input)); + assertThat(thrown).hasMessageThat().contains("not allowed to delete HEAD"); + } + private String errorMessageForBranches(List<String> branches) { StringBuilder message = new StringBuilder(); for (String branch : branches) {