Use bitmaps for non-commit reachability checks

Currently, unless RequestPolicy#ANY is used, UploadPack rejects all
non-commit "want" lines unless they were advertized. This is fine,
except when "uploadpack.allowreachablesha1inwant" is true
(corresponding to RequestPolicy#REACHABLE_COMMIT), in which case one
would expect that "want"-ing anything reachable would work.

(There is no restriction that "want" lines must only contain commits -
it is allowed for refs to directly point to trees and blobs, and
requesting for them using "want" lines works.)

This commit has been written to avoid performance regressions as much
as possible. In the usual (and currently working) case where the only
unadvertized things requested are commits, we do a standard RevWalk in
order to avoid incurring the cost of loading bitmaps. However, if
unadvertized non-commits are requested, bitmaps are used instead, and
if there are no bitmaps, a WantNotValidException is thrown (as is
currently done).

Change-Id: I68ed4abd0e477ff415c696c7544ccaa234df7f99
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>

1 files changed, 33 insertions, 2 deletions

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 cf070c6348..cab532288b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -77,14 +77,18 @@ import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.PackWriter;
+import org.eclipse.jgit.lib.BitmapIndex;
+import org.eclipse.jgit.lib.BitmapIndex.BitmapBuilder;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.AsyncRevObjectQueue;
+import org.eclipse.jgit.revwalk.BitmapWalker;
 import org.eclipse.jgit.revwalk.DepthWalk;
 import org.eclipse.jgit.revwalk.ObjectWalk;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -1315,6 +1319,18 @@ public class UploadPack {
 		}
 	}
 
+	private static void checkNotAdvertisedWantsUsingBitmap(ObjectReader reader,
+			BitmapIndex bitmapIndex, List<ObjectId> notAdvertisedWants,
+			Set<ObjectId> reachableFrom) throws IOException {
+		BitmapWalker bitmapWalker = new BitmapWalker(new ObjectWalk(reader), bitmapIndex, null);
+		BitmapBuilder reachables = bitmapWalker.findObjects(reachableFrom, null, false);
+		for (ObjectId oid : notAdvertisedWants) {
+			if (!reachables.contains(oid)) {
+				throw new WantNotValidException(oid);
+			}
+		}
+	}
+
 	private static void checkNotAdvertisedWants(UploadPack up,
 			List<ObjectId> notAdvertisedWants, Set<ObjectId> reachableFrom)
 			throws MissingObjectException, IncorrectObjectTypeException, IOException {
@@ -1324,13 +1340,28 @@ public class UploadPack {
 		// into an advertised branch it will be marked UNINTERESTING and no commits
 		// return.
 
-		try (RevWalk walk = new RevWalk(up.getRevWalk().getObjectReader())) {
+		ObjectReader reader = up.getRevWalk().getObjectReader();
+		try (RevWalk walk = new RevWalk(reader)) {
 			AsyncRevObjectQueue q = walk.parseAny(notAdvertisedWants, true);
 			try {
 				RevObject obj;
 				while ((obj = q.next()) != null) {
-					if (!(obj instanceof RevCommit))
+					if (!(obj instanceof RevCommit)) {
+						// If unadvertized non-commits are requested, use
+						// bitmaps. If there are no bitmaps, instead of
+						// incurring the expense of a manual walk, reject
+						// the request.
+						BitmapIndex bitmapIndex = reader.getBitmapIndex();
+						if (bitmapIndex != null) {
+							checkNotAdvertisedWantsUsingBitmap(
+									reader,
+									bitmapIndex,
+									notAdvertisedWants,
+									reachableFrom);
+							return;
+						}
 						throw new WantNotValidException(obj);
+					}
 					walk.markStart((RevCommit) obj);
 				}
 			} catch (MissingObjectException notFound) {