Do not execute rejected commands
If a command has already been rejected by prior processing,
do not invoke its execute() method.
Bug: issue 1420
Change-Id: I044d2ceba05f0765e28e3130ed9d58c9c107c852
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 04224d9..37fbdb2 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
@@ -15,6 +15,11 @@
package com.google.gerrit.server.git;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_MISSING_OBJECT;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_NONFASTFORWARD;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.PageLinks;
@@ -80,7 +85,6 @@
import org.eclipse.jgit.transport.AdvertiseRefsHook;
import org.eclipse.jgit.transport.AdvertiseRefsHookChain;
import org.eclipse.jgit.transport.ReceiveCommand;
-import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.eclipse.jgit.transport.ReceivePack;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -426,8 +430,7 @@
commandProgress = progress.beginSubTask("refs", UNKNOWN);
parseCommands(commands);
- if (newChange != null
- && newChange.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED) {
+ if (newChange != null && newChange.getResult() == NOT_ATTEMPTED) {
createNewChanges();
}
newProgress.end();
@@ -436,7 +439,7 @@
replaceProgress.end();
for (final ReceiveCommand c : commands) {
- if (c.getResult() == Result.OK) {
+ if (c.getResult() == OK) {
switch (c.getType()) {
case CREATE:
if (isHead(c)) {
@@ -511,7 +514,7 @@
private void parseCommands(final Collection<ReceiveCommand> commands) {
for (final ReceiveCommand cmd : commands) {
- if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) {
+ if (cmd.getResult() != NOT_ATTEMPTED) {
// Already rejected by the core receive process.
//
continue;
@@ -559,7 +562,7 @@
continue;
}
- if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) {
+ if (cmd.getResult() != NOT_ATTEMPTED) {
continue;
}
@@ -625,7 +628,9 @@
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (ctl.canCreate(rp.getRevWalk(), obj)) {
validateNewCommits(ctl, cmd);
- cmd.execute(rp);
+ if (cmd.getResult() == NOT_ATTEMPTED) {
+ cmd.execute(rp);
+ }
} else {
reject(cmd, "can not create new references");
}
@@ -639,7 +644,9 @@
}
validateNewCommits(ctl, cmd);
- cmd.execute(rp);
+ if (cmd.getResult() == NOT_ATTEMPTED) {
+ cmd.execute(rp);
+ }
} else {
reject(cmd, "can not update the reference as a fast forward");
}
@@ -667,7 +674,9 @@
private void parseDelete(final ReceiveCommand cmd) {
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (ctl.canDelete()) {
- cmd.execute(rp);
+ if (cmd.getResult() == NOT_ATTEMPTED) {
+ cmd.execute(rp);
+ }
} else {
reject(cmd, "can not delete references");
}
@@ -689,15 +698,17 @@
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
if (newObject != null) {
validateNewCommits(ctl, cmd);
- if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) {
+ if (cmd.getResult() != NOT_ATTEMPTED) {
return;
}
}
if (ctl.canForceUpdate()) {
- cmd.execute(rp);
+ if (cmd.getResult() == NOT_ATTEMPTED) {
+ cmd.execute(rp);
+ }
} else {
- cmd.setResult(ReceiveCommand.Result.REJECTED_NONFASTFORWARD, " need '"
+ cmd.setResult(REJECTED_NONFASTFORWARD, " need '"
+ PermissionRule.FORCE_PUSH + "' privilege.");
}
}
@@ -803,7 +814,7 @@
walk.setRevFilter(oldRevFilter);
}
} catch (IOException e) {
- newChange.setResult(Result.REJECTED_MISSING_OBJECT);
+ newChange.setResult(REJECTED_MISSING_OBJECT);
log.error("Invalid pack upload; one or more objects weren't sent", e);
return;
}
@@ -978,7 +989,7 @@
// Should never happen, the core receive process would have
// identified the missing object earlier before we got control.
//
- newChange.setResult(Result.REJECTED_MISSING_OBJECT);
+ newChange.setResult(REJECTED_MISSING_OBJECT);
log.error("Invalid pack upload; one or more objects weren't sent", e);
return;
} catch (OrmException e) {
@@ -1006,7 +1017,7 @@
}
newProgress.update(1);
}
- newChange.setResult(ReceiveCommand.Result.OK);
+ newChange.setResult(OK);
}
private static boolean isValidChangeId(String idStr) {
@@ -1156,7 +1167,7 @@
+ request.ontoChange + ", commit " + request.newCommit.name(), err);
reject(request.cmd, "database error");
}
- if (request.cmd.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED) {
+ if (request.cmd.getResult() == NOT_ATTEMPTED) {
log.error("Replacement patch for change " + request.ontoChange
+ ", commit " + request.newCommit.name() + " wasn't attempted."
+ " This is a bug in the receive process implementation.");
@@ -1455,7 +1466,7 @@
}
replication.scheduleUpdate(project.getNameKey(), ru.getName());
hooks.doPatchsetCreatedHook(result.change, ps, db);
- request.cmd.setResult(ReceiveCommand.Result.OK);
+ request.cmd.setResult(OK);
workQueue.getDefaultQueue()
.submit(requestScopePropagator.wrap(new Runnable() {
@@ -1611,7 +1622,7 @@
}
}
} catch (IOException err) {
- cmd.setResult(Result.REJECTED_MISSING_OBJECT);
+ cmd.setResult(REJECTED_MISSING_OBJECT);
log.error("Invalid pack upload; one or more objects weren't sent", err);
}
}
@@ -2055,7 +2066,7 @@
}
private void reject(final ReceiveCommand cmd, final String why) {
- cmd.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, why);
+ cmd.setResult(REJECTED_OTHER_REASON, why);
commandProgress.update(1);
}
