diff options
author | David Pursehouse <dpursehouse@collab.net> | 2017-03-22 13:28:28 +0900 |
---|---|---|
committer | David Pursehouse <dpursehouse@collab.net> | 2017-03-22 13:28:28 +0900 |
commit | 604cdb290a831411c7b58cb2636bb30df4e7a84b (patch) | |
tree | 4134c90286399b8b6def95ef817f09f8f3de181c | |
parent | 1b435832b2c464053f7b47ced16722043567edf1 (diff) |
Be more consistent about object ids used in ref operation validationv2.13.7
In CreateBranch, a RefUpdate is created with expectedOldObjectId set
but not oldObjectId. In DeleteBranch[es], none of expectedOldObjectId,
oldObjectId, and newObjectId is set.
As a result, when RefOperationValidators uses oldObjectId and
newObjectId to create a ReceiveCommand, they can be null, resulting in
NullPointerException when a listener dereferences them.
So use expectedOldObjectId instead of oldObjectId in
RefOperationValidators. Before a ref update has been performed, only
the former is meant to be set.
In DeleteBranch[es], set the expectedOldObjectId and the newObjectId
(which is zero). This way, in RefOperationValidators we know both values
will always be set.
Bug: Issue 5817
Helped-By: Jonathan Nieder <jrn@google.com>
Change-Id: If1f3a6179fa789077731a16e4b731227a73be7f2
3 files changed, 8 insertions, 2 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java index 769c7d27ac..580de95e62 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java @@ -41,7 +41,7 @@ public class RefOperationValidators { } public static ReceiveCommand getCommand(RefUpdate update, ReceiveCommand.Type type) { - return new ReceiveCommand(update.getOldObjectId(), update.getNewObjectId(), + return new ReceiveCommand(update.getExpectedOldObjectId(), update.getNewObjectId(), update.getName(), type); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java index 091cba364f..732d47bb14 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java @@ -29,6 +29,7 @@ import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.errors.LockFailedException; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceiveCommand; @@ -80,7 +81,10 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input> { try (Repository r = repoManager.openRepository(rsrc.getNameKey())) { RefUpdate.Result result; - RefUpdate u = r.updateRef(rsrc.getRef()); + String ref = rsrc.getRef(); + RefUpdate u = r.updateRef(ref); + u.setExpectedOldObjectId(r.exactRef(ref).getObjectId()); + u.setNewObjectId(ObjectId.zeroId()); u.setForceUpdate(true); refDeletionValidator.validateRefOperation( rsrc.getName(), identifiedUser.get(), u); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java index f4fa4468ee..7d53fecc8b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java @@ -127,6 +127,8 @@ public class DeleteBranches command.setResult(Result.REJECTED_OTHER_REASON, "it has open changes"); } RefUpdate u = r.updateRef(branch); + u.setExpectedOldObjectId(r.exactRef(branch).getObjectId()); + u.setNewObjectId(ObjectId.zeroId()); u.setForceUpdate(true); refDeletionValidator.validateRefOperation( project.getName(), identifiedUser.get(), u); |