diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/ObjectWalkTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/ObjectWalkTest.java index 582406c01..0ff4d512e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/ObjectWalkTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/ObjectWalkTest.java @@ -43,6 +43,11 @@ package org.eclipse.jgit.revwalk; +import org.eclipse.jgit.lib.FileTreeEntry; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectWriter; +import org.eclipse.jgit.lib.Tree; + public class ObjectWalkTest extends RevWalkTestCase { protected ObjectWalk objw; @@ -194,4 +199,43 @@ public void testCull() throws Exception { assertSame(f2, objw.nextObject()); assertNull(objw.nextObject()); } + + public void testEmptyTreeCorruption() throws Exception { + ObjectId bId = ObjectId + .fromString("abbbfafe3129f85747aba7bfac992af77134c607"); + final RevTree tree_root, tree_A, tree_AB; + final RevCommit b; + { + Tree root = new Tree(db); + Tree A = root.addTree("A"); + FileTreeEntry B = root.addFile("B"); + B.setId(bId); + + Tree A_A = A.addTree("A"); + Tree A_B = A.addTree("B"); + + final ObjectWriter ow = new ObjectWriter(db); + A_A.setId(ow.writeTree(A_A)); + A_B.setId(ow.writeTree(A_B)); + A.setId(ow.writeTree(A)); + root.setId(ow.writeTree(root)); + + tree_root = rw.parseTree(root.getId()); + tree_A = rw.parseTree(A.getId()); + tree_AB = rw.parseTree(A_A.getId()); + assertSame(tree_AB, rw.parseTree(A_B.getId())); + b = commit(rw.parseTree(root.getId())); + } + + markStart(b); + + assertCommit(b, objw.next()); + assertNull(objw.next()); + + assertSame(tree_root, objw.nextObject()); + assertSame(tree_A, objw.nextObject()); + assertSame(tree_AB, objw.nextObject()); + assertSame(rw.lookupBlob(bId), objw.nextObject()); + assertNull(objw.nextObject()); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java index b3acf518c..ddf40ac10 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/ObjectWalk.java @@ -86,9 +86,7 @@ public class ObjectWalk extends RevWalk { private RevTree currentTree; - private boolean fromTreeWalk; - - private RevTree nextSubtree; + private RevObject last; /** * Create a new revision and object walker for a given repository. @@ -243,18 +241,12 @@ public RevCommit next() throws MissingObjectException, */ public RevObject nextObject() throws MissingObjectException, IncorrectObjectTypeException, IOException { - fromTreeWalk = false; - - if (nextSubtree != null) { - treeWalk = treeWalk.createSubtreeIterator0(db, nextSubtree, curs); - nextSubtree = null; - } + if (last != null) + treeWalk = last instanceof RevTree ? enter(last) : treeWalk.next(); while (!treeWalk.eof()) { final FileMode mode = treeWalk.getEntryFileMode(); - final int sType = mode.getObjectType(); - - switch (sType) { + switch (mode.getObjectType()) { case Constants.OBJ_BLOB: { treeWalk.getEntryObjectId(idBuffer); final RevBlob o = lookupBlob(idBuffer); @@ -263,7 +255,7 @@ public RevObject nextObject() throws MissingObjectException, o.flags |= SEEN; if (shouldSkipObject(o)) break; - fromTreeWalk = true; + last = o; return o; } case Constants.OBJ_TREE: { @@ -274,8 +266,7 @@ public RevObject nextObject() throws MissingObjectException, o.flags |= SEEN; if (shouldSkipObject(o)) break; - nextSubtree = o; - fromTreeWalk = true; + last = o; return o; } default: @@ -291,6 +282,7 @@ public RevObject nextObject() throws MissingObjectException, treeWalk = treeWalk.next(); } + last = null; for (;;) { final RevObject o = pendingObjects.next(); if (o == null) @@ -308,6 +300,18 @@ public RevObject nextObject() throws MissingObjectException, } } + private CanonicalTreeParser enter(RevObject tree) throws IOException { + CanonicalTreeParser p = treeWalk.createSubtreeIterator0(db, tree, curs); + if (p.eof()) { + // We can't tolerate the subtree being an empty tree, as + // that will break us out early before we visit all names. + // If it is, advance to the parent's next record. + // + return treeWalk.next(); + } + return p; + } + private final boolean shouldSkipObject(final RevObject o) { return (o.flags & UNINTERESTING) != 0 && !hasRevSort(RevSort.BOUNDARY); } @@ -364,7 +368,7 @@ public void checkConnectivity() throws MissingObjectException, * has no path, such as for annotated tags or root level trees. */ public String getPathString() { - return fromTreeWalk ? treeWalk.getEntryPathString() : null; + return last != null ? treeWalk.getEntryPathString() : null; } @Override @@ -372,8 +376,8 @@ public void dispose() { super.dispose(); pendingObjects = new BlockObjQueue(); treeWalk = new CanonicalTreeParser(); - nextSubtree = null; currentTree = null; + last = null; } @Override @@ -381,7 +385,8 @@ protected void reset(final int retainFlags) { super.reset(retainFlags); pendingObjects = new BlockObjQueue(); treeWalk = new CanonicalTreeParser(); - nextSubtree = null; + currentTree = null; + last = null; } private void addObject(final RevObject o) {