From 604cdb290a831411c7b58cb2636bb30df4e7a84b Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 22 Mar 2017 13:28:28 +0900 Subject: Be more consistent about object ids used in ref operation validation 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 Change-Id: If1f3a6179fa789077731a16e4b731227a73be7f2 --- .../google/gerrit/server/git/validators/RefOperationValidators.java | 2 +- .../main/java/com/google/gerrit/server/project/DeleteBranch.java | 6 +++++- .../main/java/com/google/gerrit/server/project/DeleteBranches.java | 2 ++ 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 { 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); -- cgit v1.2.3