Merge "Compare incoming change-id to target change"
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 8ed1ef0..5e60906 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -569,7 +569,8 @@
new Branch.NameKey(ctx.getProject(), refName),
ctx.getIdentifiedUser(),
new NoSshInfo(),
- ctx.getRevWalk())
+ ctx.getRevWalk(),
+ change)
.validate(event);
}
} catch (CommitValidationException e) {
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 3cf0dc5..3a32f8f 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -345,7 +345,8 @@
origNotes.getChange().getDest(),
ctx.getIdentifiedUser(),
new NoSshInfo(),
- ctx.getRevWalk())
+ ctx.getRevWalk(),
+ origNotes.getChange())
.validate(event);
} catch (CommitValidationException e) {
throw new ResourceConflictException(e.getFullMessage());
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 10b761f..292fa7a 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1856,7 +1856,8 @@
logDebug("Creating new change for {} even though it is already tracked", name);
}
- if (!validCommit(rp.getRevWalk(), magicBranch.perm, magicBranch.dest, magicBranch.cmd, c)) {
+ if (!validCommit(
+ rp.getRevWalk(), magicBranch.perm, magicBranch.dest, magicBranch.cmd, c, null)) {
// Not a change the user can propose? Abort as early as possible.
newChanges = Collections.emptyList();
logDebug("Aborting early due to invalid commit");
@@ -2434,7 +2435,7 @@
}
PermissionBackend.ForRef perm = permissions.ref(change.getDest().get());
- if (!validCommit(rp.getRevWalk(), perm, change.getDest(), inputCommand, newCommit)) {
+ if (!validCommit(rp.getRevWalk(), perm, change.getDest(), inputCommand, newCommit, change)) {
return false;
}
rp.getRevWalk().parseBody(priorCommit);
@@ -2798,7 +2799,7 @@
}
if (existing.keySet().contains(c)) {
continue;
- } else if (!validCommit(walk, perm, branch, cmd, c)) {
+ } else if (!validCommit(walk, perm, branch, cmd, c, null)) {
break;
}
@@ -2820,7 +2821,8 @@
PermissionBackend.ForRef perm,
Branch.NameKey branch,
ReceiveCommand cmd,
- ObjectId id)
+ ObjectId id,
+ @Nullable Change change)
throws IOException {
if (validCommits.contains(id)) {
@@ -2841,7 +2843,7 @@
? commitValidatorsFactory.forMergedCommits(
project.getNameKey(), perm, user.asIdentifiedUser())
: commitValidatorsFactory.forReceiveCommits(
- perm, branch, user.asIdentifiedUser(), sshInfo, repo, rw);
+ perm, branch, user.asIdentifiedUser(), sshInfo, repo, rw, change);
messages.addAll(validators.validate(receiveEvent));
} catch (CommitValidationException e) {
logDebug("Commit validation failed on {}", c.name());
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index a80ff73..05ca98b 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
import static java.util.stream.Collectors.toList;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.FooterConstants;
@@ -30,6 +31,8 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Branch.NameKey;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
@@ -46,6 +49,7 @@
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectCache;
@@ -125,7 +129,8 @@
IdentifiedUser user,
SshInfo sshInfo,
Repository repo,
- RevWalk rw)
+ RevWalk rw,
+ @Nullable Change change)
throws IOException {
NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw);
ProjectState projectState = projectCache.checkedGet(branch.getParentKey());
@@ -138,7 +143,12 @@
new CommitterUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
- projectState, user, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
+ projectState,
+ user,
+ canonicalWebUrl,
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
@@ -148,11 +158,12 @@
}
public CommitValidators forGerritCommits(
- PermissionBackend.ForRef perm,
- Branch.NameKey branch,
+ ForRef perm,
+ NameKey branch,
IdentifiedUser user,
SshInfo sshInfo,
- RevWalk rw)
+ RevWalk rw,
+ @Nullable Change change)
throws IOException {
ProjectState projectState = projectCache.checkedGet(branch.getParentKey());
return new CommitValidators(
@@ -163,7 +174,12 @@
new AuthorUploaderValidator(user, perm, canonicalWebUrl),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())),
new ChangeIdValidator(
- projectState, user, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
+ projectState,
+ user,
+ canonicalWebUrl,
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -231,6 +247,15 @@
"[%s] invalid "
+ FooterConstants.CHANGE_ID.getName()
+ " line format in commit message footer";
+
+ @VisibleForTesting
+ public static final String CHANGE_ID_MISMATCH_MSG =
+ "[%s] "
+ + FooterConstants.CHANGE_ID.getName()
+ + " in commit message footer does not match"
+ + FooterConstants.CHANGE_ID.getName()
+ + " of target change";
+
private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN);
private final ProjectState projectState;
@@ -238,18 +263,21 @@
private final String installCommitMsgHookCommand;
private final SshInfo sshInfo;
private final IdentifiedUser user;
+ private final Change change;
public ChangeIdValidator(
ProjectState projectState,
IdentifiedUser user,
String canonicalWebUrl,
String installCommitMsgHookCommand,
- SshInfo sshInfo) {
+ SshInfo sshInfo,
+ Change change) {
this.projectState = projectState;
this.canonicalWebUrl = canonicalWebUrl;
this.installCommitMsgHookCommand = installCommitMsgHookCommand;
this.sshInfo = sshInfo;
this.user = user;
+ this.change = change;
}
@Override
@@ -289,7 +317,12 @@
messages.add(getMissingChangeIdErrorMsg(errMsg, receiveEvent.commit));
throw new CommitValidationException(errMsg, messages);
}
+ if (change != null && !v.equals(change.getKey().get())) {
+ String errMsg = String.format(CHANGE_ID_MISMATCH_MSG, sha1);
+ throw new CommitValidationException(errMsg);
+ }
}
+
return Collections.emptyList();
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 38a9489..f7ca2f2 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -75,9 +75,11 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.receive.ReceiveConstants;
+import com.google.gerrit.server.git.validators.CommitValidators.ChangeIdValidator;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.project.testing.Util;
@@ -1321,6 +1323,28 @@
}
@Test
+ public void testPushWithChangedChangeId() throws Exception {
+ PushOneCommit.Result r = pushTo("refs/for/master");
+ r.assertOkStatus();
+ PushOneCommit push =
+ pushFactory.create(
+ db,
+ admin.getIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT
+ + "\n\n"
+ + "Change-Id: I55eab7c7a76e95005fa9cc469aa8f9fc16da9eba\n",
+ "b.txt",
+ "anotherContent",
+ r.getChangeId());
+ r = push.to("refs/changes/" + r.getChange().change().getId().get());
+ r.assertErrorStatus(
+ String.format(
+ ChangeIdValidator.CHANGE_ID_MISMATCH_MSG,
+ r.getCommit().abbreviate(RevId.ABBREV_LEN).name()));
+ }
+
+ @Test
public void pushWithMultipleChangeIds() throws Exception {
testPushWithMultipleChangeIds();
}
