UploadPack: Fix NPE when traversing a tag chain

Always parse RevTags including their body before getting their object
to ensure that non-cached objects are handled correctly when traversing
a tag chain. An NPE in UploadPack#addTagChain will occur on a depth=1
fetch of a branch containing a tag chain and the ref to one of the
middle tags in the chain is deleted.

Change-Id: Ifd8fe868869070b365df926fec5dcd8e64d4f521
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
index 7131905850007285bd518eb8394e7ea32bbc9f4d..2b05decb45bc8a1c5376ee441a3ec647776786a9 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java
@@ -2538,6 +2538,75 @@ public class UploadPackTest {
                }
        }
 
+       @Test
+       public void testSingleBranchShallowCloneTagChainWithReflessTag() throws Exception {
+               RevCommit one = remote.commit().message("1").create();
+               remote.update("master", one);
+               RevTag tag1 = remote.tag("t1", one);
+               remote.lightweightTag("t1", tag1);
+               RevTag tag2 = remote.tag("t2", tag1);
+               RevTag tag3 = remote.tag("t3", tag2);
+               remote.lightweightTag("t3", tag3);
+
+               UploadPack uploadPack = new UploadPack(remote.getRepository());
+
+               ByteArrayOutputStream cli = new ByteArrayOutputStream();
+               PacketLineOut clientWant = new PacketLineOut(cli);
+               clientWant.writeString("want " + one.name() + " include-tag");
+               clientWant.writeString("deepen 1\n");
+               clientWant.end();
+               clientWant.writeString("done\n");
+
+               try (ByteArrayOutputStream serverResponse = new ByteArrayOutputStream()) {
+
+                       uploadPack.setPreUploadHook(new PreUploadHook() {
+                               @Override
+                               public void onBeginNegotiateRound(UploadPack up,
+                                               Collection<? extends ObjectId> wants, int cntOffered)
+                                               throws ServiceMayNotContinueException {
+                                       // Do nothing.
+                               }
+
+                               @Override
+                               public void onEndNegotiateRound(UploadPack up,
+                                               Collection<? extends ObjectId> wants, int cntCommon,
+                                               int cntNotFound, boolean ready)
+                                               throws ServiceMayNotContinueException {
+                                       // Do nothing.
+                               }
+
+                               @Override
+                               public void onSendPack(UploadPack up,
+                                               Collection<? extends ObjectId> wants,
+                                               Collection<? extends ObjectId> haves)
+                                               throws ServiceMayNotContinueException {
+                                       // collect pack data
+                                       serverResponse.reset();
+                               }
+                       });
+                       uploadPack.upload(new ByteArrayInputStream(cli.toByteArray()),
+                                       serverResponse, System.err);
+                       ByteArrayInputStream packReceived = new ByteArrayInputStream(
+                                       serverResponse.toByteArray());
+                       PackLock lock = null;
+                       try (ObjectInserter ins = client.newObjectInserter()) {
+                               PackParser parser = ins.newPackParser(packReceived);
+                               parser.setAllowThin(true);
+                               parser.setLockMessage("receive-tag-chain");
+                               ProgressMonitor mlc = NullProgressMonitor.INSTANCE;
+                               lock = parser.parse(mlc, mlc);
+                               ins.flush();
+                       } finally {
+                               if (lock != null) {
+                                       lock.unlock();
+                               }
+                       }
+                       InMemoryRepository.MemObjDatabase objDb = client
+                                       .getObjectDatabase();
+                       assertTrue(objDb.has(one.toObjectId()));
+               }
+       }
+
        @Test
        public void testSafeToClearRefsInFetchV0() throws Exception {
                server =

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 6cb8fe43c0b59cd454da577119f03594fc6c47fd..0c23c8cadf31d092c700d0fc7b4187c7691a4ca0 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -2405,11 +2405,11 @@ public class UploadPack implements Closeable {
                                        if (peeledId == null || objectId == null)
                                                continue;
 
-                                       objectId = ref.getObjectId();
-                                       if (pw.willInclude(peeledId) && !pw.willInclude(objectId)) {
-                                               RevObject o = rw.parseAny(objectId);
-                                               addTagChain(o, pw);
-                                               pw.addObject(o);
+                                       if (pw.willInclude(peeledId)) {
+                                               // We don't need to handle parseTag throwing an
+                                               // IncorrectObjectTypeException as we only reach
+                                               // here when ref is an annotated tag
+                                               addTagChain(rw.parseTag(objectId), pw);
                                        }
                                }
                        }
@@ -2459,15 +2459,16 @@ public class UploadPack implements Closeable {
        }
 
        private void addTagChain(
-                       RevObject o, PackWriter pw) throws IOException {
-               while (Constants.OBJ_TAG == o.getType()) {
-                       RevTag t = (RevTag) o;
-                       o = t.getObject();
-                       if (o.getType() == Constants.OBJ_TAG && !pw.willInclude(o.getId())) {
-                               walk.parseBody(o);
-                               pw.addObject(o);
+                       RevTag tag, PackWriter pw) throws IOException {
+               RevObject o = tag;
+               do {
+                       tag = (RevTag) o;
+                       walk.parseBody(tag);
+                       if (!pw.willInclude(tag.getId())) {
+                               pw.addObject(tag);
                        }
-               }
+                       o = tag.getObject();
+               } while (Constants.OBJ_TAG == o.getType());
        }
 
        private static class ResponseBufferedOutputStream extends OutputStream {