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
This commit is contained in:
David Pursehouse 2019-11-15 13:39:57 -08:00
commit bde4262a27
7 changed files with 78 additions and 9 deletions

View File

@ -43,6 +43,7 @@ import com.google.gerrit.server.query.account.AccountPredicates;
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 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
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 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
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 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
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 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
}
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);
}

View File

@ -32,6 +32,7 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
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 @@ import org.kohsuke.args4j.Option;
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 @@ public class QueryGroups implements RestReadView<TopLevelResource> {
@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 @@ public class QueryGroups implements RestReadView<TopLevelResource> {
throw new BadRequestException("missing query field");
}
GroupQueryProcessor queryProcessor = queryProcessorProvider.get();
if (queryProcessor.isDisabled()) {
throw new MethodNotAllowedException("query disabled");
}

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.restapi.project;
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 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input> {
@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());

View File

@ -17,8 +17,10 @@ package com.google.gerrit.server.restapi.project;
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 @@ public class DeleteBranches implements RestModifyView<ProjectResource, DeleteBra
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();

View File

@ -99,7 +99,11 @@ public class MergeSuperSet {
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;

View File

@ -33,6 +33,7 @@ import com.google.gerrit.extensions.api.projects.BranchApi;
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 @@ public class DeleteBranchIT extends AbstractDaemonTest {
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)

View File

@ -35,6 +35,7 @@ import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
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 @@ public class DeleteBranchesIT extends AbstractDaemonTest {
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) {