Merge changes from topic 'conflicts-with-merge' into stable-2.11
* changes:
Fix "Conflicts With" when current change is a merge commit
Fix "Conflicts With" when other change is a merge-commit
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 780d7a84..963fe29 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -17,6 +17,10 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.GitUtil.cloneProject;
+import static com.google.gerrit.acceptance.GitUtil.createCommit;
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import static com.google.gerrit.server.project.Util.category;
+import static com.google.gerrit.server.project.Util.value;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -24,8 +28,10 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.GitUtil.Commit;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -34,6 +40,8 @@
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gwtorm.server.OrmException;
@@ -247,6 +255,64 @@
"Uploaded patch set 3.");
}
+ /**
+ * There was a bug that allowed a user with Forge Committer Identity access
+ * right to upload a commit and put *votes on behalf of another user* on it.
+ * This test checks that this is not possible, but that the votes that are
+ * specified on push are applied only in the name of the uploader.
+ *
+ * This particular bug only occurred when there was more than one label
+ * defined. Hence the test defines a custom label.
+ *
+ * When on upload the committer is forged he is automatically added as
+ * reviewer to the change. This results in a dummy 0 vote. If there was only
+ * one label this dummy 0 vote collided with any vote for the same label that
+ * was specified in the push specification and hence the upload failed, which
+ * means in this case it was not possible to forge a vote.
+ */
+ @Test
+ public void testPushForMasterWithApprovalsForgeCommitterButNoForgeVote()
+ throws GitAPIException, IOException, RestApiException {
+ // add custom label because the bug only allowed to forge a vote when there
+ // were at least two labels
+ LabelType Q = category("CustomLabel",
+ value(1, "Positive"),
+ value(0, "No score"),
+ value(-1, "Negative"));
+ ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
+ cfg.getLabelSections().put(Q.getName(), Q);
+ MetaDataUpdate md = metaDataUpdateFactory.create(allProjects);
+ try {
+ cfg.commit(md);
+ } finally {
+ md.close();
+ }
+ projectCache.evict(allProjects);
+
+ // Create a commit with "User" as author and committer
+ Commit c = createCommit(git, user.getIdent(), PushOneCommit.SUBJECT);
+
+ // Push this commit as "Administrator" (requires Forge Committer Identity)
+ pushHead(git, "refs/for/master/%l=Code-Review+1", false);
+
+ // Expected Code-Review votes:
+ // 1. 0 from User (committer):
+ // When the committer is forged, the committer is automatically added as
+ // reviewer, hence we expect a dummy 0 vote for the committer.
+ // 2. +1 from Administrator (uploader):
+ // On push Code-Review+1 was specified, hence we expect a +1 vote from
+ // the uploader.
+ ChangeInfo ci = get(c.getChangeId());
+ LabelInfo cr = ci.labels.get("Code-Review");
+ assertThat(cr.all).hasSize(2);
+ assertThat(cr.all.get(0).name).isEqualTo("Administrator");
+ assertThat(cr.all.get(0).value.intValue()).is(1);
+ assertThat(cr.all.get(1).name).isEqualTo("User");
+ assertThat(cr.all.get(1).value.intValue()).is(0);
+ assertThat(Iterables.getLast(ci.messages).message).isEqualTo(
+ "Uploaded patch set 1: Code-Review+1.");
+ }
+
@Test
public void testPushNewPatchsetToRefsChanges() throws GitAPIException,
IOException, OrmException {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
index e48477f..771423e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
@@ -72,7 +72,8 @@
}
return sce.getStatusCode() == Response.SC_FORBIDDEN
&& (sce.getEncodedResponse().equals("Authentication required")
- || sce.getEncodedResponse().startsWith("Must be signed-in"));
+ || sce.getEncodedResponse().startsWith("Must be signed-in")
+ || sce.getEncodedResponse().startsWith("Invalid authentication"));
}
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 7265196..31058bc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -222,9 +222,8 @@
}
public void addApprovals(ReviewDb db, ChangeUpdate update,
- LabelTypes labelTypes, PatchSet ps, PatchSetInfo info,
- ChangeControl changeCtl, Map<String, Short> approvals)
- throws OrmException {
+ LabelTypes labelTypes, PatchSet ps, ChangeControl changeCtl,
+ Map<String, Short> approvals) throws OrmException {
if (!approvals.isEmpty()) {
checkApprovals(approvals, changeCtl);
List<PatchSetApproval> cells = new ArrayList<>(approvals.size());
@@ -233,7 +232,7 @@
LabelType lt = labelTypes.byLabel(vote.getKey());
cells.add(new PatchSetApproval(new PatchSetApproval.Key(
ps.getId(),
- info.getCommitter().getAccount(),
+ ps.getUploader(),
lt.getLabelId()),
vote.getValue(),
ts));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index df5c254..15a249f6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -211,8 +211,8 @@
LabelTypes labelTypes = projectControl.getLabelTypes();
approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
- approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo,
- ctl, approvals);
+ approvalsUtil.addApprovals(db, update, labelTypes, patchSet, ctl,
+ approvals);
if (messageIsForChange()) {
cmUtil.addChangeMessage(db, update, changeMessage);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index a950eab..b4b30ec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -2197,7 +2197,7 @@
approvalCopier.copy(db, changeCtl, newPatchSet);
approvalsUtil.addReviewers(db, update, labelTypes, change, newPatchSet,
info, recipients.getReviewers(), oldRecipients.getAll());
- approvalsUtil.addApprovals(db, update, labelTypes, newPatchSet, info,
+ approvalsUtil.addApprovals(db, update, labelTypes, newPatchSet,
changeCtl, approvals);
recipients.add(oldRecipients);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/UploadValidationListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/UploadValidationListener.java
index fefe02a..2c032c4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/UploadValidationListener.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/UploadValidationListener.java
@@ -49,7 +49,7 @@
* These may be RevObject or RevCommit if the processor parsed them.
* Implementors should not rely on the values being parsed.
* @throws ValidationException to block the upload and send a message
- * back to the end-used over the client's protocol connection.
+ * back to the end-user over the client's protocol connection.
*/
public void onPreUpload(Repository repository, Project project,
String remoteHost, UploadPack up, Collection<? extends ObjectId> wants,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
index 299f0f1..153329a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
@@ -99,7 +99,8 @@
out = query();
} catch (QueryParseException e) {
// This is a hack to detect an operator that requires authentication.
- Pattern p = Pattern.compile("^Error in operator (.*:self)$");
+ Pattern p = Pattern.compile(
+ "^Error in operator (.*:self|is:watched|is:owner|is:reviewer|has:.*)$");
Matcher m = p.matcher(e.getMessage());
if (m.matches()) {
String op = m.group(1);
