ReceivePack: Prevent pointing a branch to a non-commit object

Since commit c3b0dec509fe136c5417422f31898b5a4e2d5e02, Git has
disallowed writing a non-commit to refs/heads/* refs.  JGit still
allows that, which can put users in a bad situation.  For example,

	git push origin v1.0:master

pushes the tag object v1.0 to refs/heads/master, instead of the
intended commit object v1.0^{commit}.

Prevent that by validating that the target of the ref points to a
commit object when pushing to refs/heads/*.

Git performs the same check at a lower level (in the RefDatabase).  We
could do the same here, but for now let's start conservatively by
handling it in pushes first.

[jn: fleshed out commit message]

Change-Id: I8f98ae6d8acbcd5ef7553ec732bc096cb6eb7c4e
Signed-off-by: Yunjie Li <yunjieli@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>

2 files changed, 24 insertions, 24 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/AtomicPushTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/AtomicPushTest.java
index c1e078d10d..d6c7a6199d 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/AtomicPushTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/AtomicPushTest.java
@@ -55,10 +55,9 @@ import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
-import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.resolver.ReceivePackFactory;
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
@@ -73,8 +72,8 @@ public class AtomicPushTest {
 	private Object ctx = new Object();
 	private InMemoryRepository server;
 	private InMemoryRepository client;
-	private ObjectId obj1;
-	private ObjectId obj2;
+	private ObjectId commit1;
+	private ObjectId commit2;
 
 	@Before
 	public void setUp() throws Exception {
@@ -92,10 +91,11 @@ public class AtomicPushTest {
 				});
 		uri = testProtocol.register(ctx, server);
 
-		try (ObjectInserter ins = client.newObjectInserter()) {
-			obj1 = ins.insert(Constants.OBJ_BLOB, Constants.encode("test"));
-			obj2 = ins.insert(Constants.OBJ_BLOB, Constants.encode("file"));
-			ins.flush();
+		try (TestRepository<?> clientRepo = new TestRepository<>(client)) {
+			commit1 = clientRepo.commit().noFiles().message("test commit 1")
+					.create();
+			commit2 = clientRepo.commit().noFiles().message("test commit 2")
+					.create();
 		}
 	}
 
@@ -149,13 +149,13 @@ public class AtomicPushTest {
 		List<RemoteRefUpdate> cmds = new ArrayList<>();
 		cmds.add(new RemoteRefUpdate(
 				null, null,
-				obj1, "refs/heads/one",
+				commit1, "refs/heads/one",
 				true /* force update */,
 				null /* no local tracking ref */,
 				ObjectId.zeroId()));
 		cmds.add(new RemoteRefUpdate(
 				null, null,
-				obj2, "refs/heads/two",
+				commit2, "refs/heads/two",
 				true /* force update */,
 				null /* no local tracking ref */,
 				ObjectId.zeroId()));
@@ -176,16 +176,16 @@ public class AtomicPushTest {
 		List<RemoteRefUpdate> cmds = new ArrayList<>();
 		cmds.add(new RemoteRefUpdate(
 				null, null,
-				obj1, "refs/heads/one",
+				commit1, "refs/heads/one",
 				true /* force update */,
 				null /* no local tracking ref */,
 				ObjectId.zeroId()));
 		cmds.add(new RemoteRefUpdate(
 				null, null,
-				obj2, "refs/heads/two",
+				commit2, "refs/heads/two",
 				true /* force update */,
 				null /* no local tracking ref */,
-				obj1));
+				commit1));
 		return cmds;
 	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
index fd1c3bf8b8..18946e0d50 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
@@ -62,10 +62,9 @@ import org.eclipse.jgit.api.errors.TransportException;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.RepositoryTestCase;
-import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -79,8 +78,8 @@ public class PushOptionsTest extends RepositoryTestCase {
 	private Object ctx = new Object();
 	private InMemoryRepository server;
 	private InMemoryRepository client;
-	private ObjectId obj1;
-	private ObjectId obj2;
+	private ObjectId commit1;
+	private ObjectId commit2;
 	private ReceivePack receivePack;
 
 	@Override
@@ -101,10 +100,11 @@ public class PushOptionsTest extends RepositoryTestCase {
 
 		uri = testProtocol.register(ctx, server);
 
-		try (ObjectInserter ins = client.newObjectInserter()) {
-			obj1 = ins.insert(Constants.OBJ_BLOB, Constants.encode("test"));
-			obj2 = ins.insert(Constants.OBJ_BLOB, Constants.encode("file"));
-			ins.flush();
+		try (TestRepository<?> clientRepo = new TestRepository<>(client)) {
+			commit1 = clientRepo.commit().noFiles().message("test commit 1")
+					.create();
+			commit2 = clientRepo.commit().noFiles().message("test commit 2")
+					.create();
 		}
 	}
 
@@ -121,12 +121,12 @@ public class PushOptionsTest extends RepositoryTestCase {
 	private List<RemoteRefUpdate> commands(boolean atomicSafe)
 			throws IOException {
 		List<RemoteRefUpdate> cmds = new ArrayList<>();
-		cmds.add(new RemoteRefUpdate(null, null, obj1, "refs/heads/one",
+		cmds.add(new RemoteRefUpdate(null, null, commit1, "refs/heads/one",
 				true /* force update */, null /* no local tracking ref */,
 				ObjectId.zeroId()));
-		cmds.add(new RemoteRefUpdate(null, null, obj2, "refs/heads/two",
+		cmds.add(new RemoteRefUpdate(null, null, commit2, "refs/heads/two",
 				true /* force update */, null /* no local tracking ref */,
-				atomicSafe ? ObjectId.zeroId() : obj1));
+				atomicSafe ? ObjectId.zeroId() : commit1));
 		return cmds;
 	}