Merge branch 'receive-pack-filter'

* receive-pack-filter:
  ReceivePack: Clarify the check reachable option
  ReceivePack: Micro-optimize object lookup when checking connectivity
  ReceivePack: Correct type of not provided object
  IndexPack: Tighten up new and base object bookkeeping
  ReceivePack: Remove need new,base object id properties
  ReceivePack: Discard IndexPack as soon as possible
  ReceivePack: fix ensureProvidedObjectsVisible on thin packs

Change-Id: I4ef2fcb931f3219872e0519abfcee220191d5133

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java

@@ -0,0 +1,477 @@
+/*
+ * Copyright (C) 2010, Google Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.transport;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.security.MessageDigest;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.zip.Deflater;
+
+import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectDirectory;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevBlob;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.util.NB;
+import org.eclipse.jgit.util.TemporaryBuffer;
+
+public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase {
+	private static final NullProgressMonitor PM = NullProgressMonitor.INSTANCE;
+
+	private static final String R_MASTER = Constants.R_HEADS + Constants.MASTER;
+
+	private static final String R_PRIVATE = Constants.R_HEADS + "private";
+
+	private Repository src;
+
+	private Repository dst;
+
+	private RevCommit A, B, P;
+
+	private RevBlob a, b;
+
+	@Override
+	protected void setUp() throws Exception {
+		super.setUp();
+
+		src = createBareRepository();
+		dst = createBareRepository();
+
+		// Fill dst with a some common history.
+		//
+		TestRepository d = new TestRepository(dst);
+		a = d.blob("a");
+		A = d.commit(d.tree(d.file("a", a)));
+		B = d.commit().parent(A).create();
+		d.update(R_MASTER, B);
+
+		// Clone from dst into src
+		//
+		Transport t = Transport.open(src, uriOf(dst));
+		try {
+			t.fetch(PM, Collections.singleton(new RefSpec("+refs/*:refs/*")));
+			assertEquals(B.copy(), src.resolve(R_MASTER));
+		} finally {
+			t.close();
+		}
+
+		// Now put private stuff into dst.
+		//
+		b = d.blob("b");
+		P = d.commit(d.tree(d.file("b", b)), A);
+		d.update(R_PRIVATE, P);
+	}
+
+	public void testFilterHidesPrivate() throws Exception {
+		Map<String, Ref> refs;
+		TransportLocal t = new TransportLocal(src, uriOf(dst)) {
+			@Override
+			ReceivePack createReceivePack(final Repository db) {
+				db.close();
+				dst.incrementOpen();
+
+				final ReceivePack rp = super.createReceivePack(dst);
+				rp.setRefFilter(new HidePrivateFilter());
+				return rp;
+			}
+		};
+		try {
+			PushConnection c = t.openPush();
+			try {
+				refs = c.getRefsMap();
+			} finally {
+				c.close();
+			}
+		} finally {
+			t.close();
+		}
+
+		assertNotNull(refs);
+		assertNull("no private", refs.get(R_PRIVATE));
+		assertNull("no HEAD", refs.get(Constants.HEAD));
+		assertEquals(1, refs.size());
+
+		Ref master = refs.get(R_MASTER);
+		assertNotNull("has master", master);
+		assertEquals(B.copy(), master.getObjectId());
+	}
+
+	public void testSuccess() throws Exception {
+		// Manually force a delta of an object so we reuse it later.
+		//
+		TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024);
+
+		packHeader(pack, 2);
+		pack.write((Constants.OBJ_BLOB) << 4 | 1);
+		deflate(pack, new byte[] { 'a' });
+
+		pack.write((Constants.OBJ_REF_DELTA) << 4 | 4);
+		a.copyRawTo(pack);
+		deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' });
+
+		digest(pack);
+		openPack(pack);
+
+		// Verify the only storage of b is our packed delta above.
+		//
+		ObjectDirectory od = (ObjectDirectory) src.getObjectDatabase();
+		assertTrue("has b", od.hasObject(b));
+		assertFalse("b not loose", od.fileFor(b).exists());
+
+		// Now use b but in a different commit than what is hidden.
+		//
+		TestRepository s = new TestRepository(src);
+		RevCommit N = s.commit().parent(B).add("q", b).create();
+		s.update(R_MASTER, N);
+
+		// Push this new content to the remote, doing strict validation.
+		//
+		TransportLocal t = new TransportLocal(src, uriOf(dst)) {
+			@Override
+			ReceivePack createReceivePack(final Repository db) {
+				db.close();
+				dst.incrementOpen();
+
+				final ReceivePack rp = super.createReceivePack(dst);
+				rp.setCheckReceivedObjects(true);
+				rp.setCheckReferencedObjectsAreReachable(true);
+				rp.setRefFilter(new HidePrivateFilter());
+				return rp;
+			}
+		};
+		RemoteRefUpdate u = new RemoteRefUpdate( //
+				src, //
+				R_MASTER, // src name
+				R_MASTER, // dst name
+				false, // do not force update
+				null, // local tracking branch
+				null // expected id
+		);
+		PushResult r;
+		try {
+			t.setPushThin(true);
+			r = t.push(PM, Collections.singleton(u));
+		} finally {
+			t.close();
+		}
+
+		assertNotNull("have result", r);
+		assertNull("private not advertised", r.getAdvertisedRef(R_PRIVATE));
+		assertSame("master updated", RemoteRefUpdate.Status.OK, u.getStatus());
+		assertEquals(N.copy(), dst.resolve(R_MASTER));
+	}
+
+	public void testCreateBranchAtHiddenCommitFails() throws Exception {
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(64);
+		packHeader(pack, 0);
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(256);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + P.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setRefFilter(new HidePrivateFilter());
+		rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null);
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		assertEquals("unpack error Missing commit " + P.name(), r.readString());
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	public void testUsingHiddenDeltaBaseFails() throws Exception {
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(64);
+		packHeader(pack, 1);
+		pack.write((Constants.OBJ_REF_DELTA) << 4 | 4);
+		b.copyRawTo(pack);
+		deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' });
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(256);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + P.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setRefFilter(new HidePrivateFilter());
+		rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null);
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		assertEquals("unpack error Missing blob " + b.name(), r.readString());
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	public void testUsingHiddenCommonBlobFails() throws Exception {
+		// Try to use the 'b' blob that is hidden.
+		//
+		TestRepository s = new TestRepository(src);
+		RevCommit N = s.commit().parent(B).add("q", s.blob("b")).create();
+
+		// But don't include it in the pack.
+		//
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(64);
+		packHeader(pack, 2);
+		copy(pack, src.openObject(N));
+		copy(pack,src.openObject(s.parseBody(N).getTree()));
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(256);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + N.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setRefFilter(new HidePrivateFilter());
+		rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null);
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		assertEquals("unpack error Missing blob " + b.name(), r.readString());
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	public void testUsingUnknownBlobFails() throws Exception {
+		// Try to use the 'n' blob that is not on the server.
+		//
+		TestRepository s = new TestRepository(src);
+		RevBlob n = s.blob("n");
+		RevCommit N = s.commit().parent(B).add("q", n).create();
+
+		// But don't include it in the pack.
+		//
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(64);
+		packHeader(pack, 2);
+		copy(pack, src.openObject(N));
+		copy(pack,src.openObject(s.parseBody(N).getTree()));
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(256);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + N.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setRefFilter(new HidePrivateFilter());
+		rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null);
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		assertEquals("unpack error Missing blob " + n.name(), r.readString());
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	public void testUsingUnknownTreeFails() throws Exception {
+		TestRepository s = new TestRepository(src);
+		RevCommit N = s.commit().parent(B).add("q", s.blob("a")).create();
+		RevTree t = s.parseBody(N).getTree();
+
+		// Don't include the tree in the pack.
+		//
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(64);
+		packHeader(pack, 1);
+		copy(pack, src.openObject(N));
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(256);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + N.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setRefFilter(new HidePrivateFilter());
+		rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null);
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		assertEquals("unpack error Missing tree " + t.name(), r.readString());
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	private void packHeader(TemporaryBuffer.Heap tinyPack, int cnt)
+			throws IOException {
+		final byte[] hdr = new byte[8];
+		NB.encodeInt32(hdr, 0, 2);
+		NB.encodeInt32(hdr, 4, cnt);
+
+		tinyPack.write(Constants.PACK_SIGNATURE);
+		tinyPack.write(hdr, 0, 8);
+	}
+
+	private void copy(TemporaryBuffer.Heap tinyPack, ObjectLoader ldr)
+			throws IOException {
+		final byte[] buf = new byte[64];
+		final byte[] content = ldr.getCachedBytes();
+		int dataLength = content.length;
+		int nextLength = dataLength >>> 4;
+		int size = 0;
+		buf[size++] = (byte) ((nextLength > 0 ? 0x80 : 0x00)
+				| (ldr.getType() << 4) | (dataLength & 0x0F));
+		dataLength = nextLength;
+		while (dataLength > 0) {
+			nextLength >>>= 7;
+			buf[size++] = (byte) ((nextLength > 0 ? 0x80 : 0x00) | (dataLength & 0x7F));
+			dataLength = nextLength;
+		}
+		tinyPack.write(buf, 0, size);
+		deflate(tinyPack, content);
+	}
+
+	private void deflate(TemporaryBuffer.Heap tinyPack, final byte[] content)
+			throws IOException {
+		final Deflater deflater = new Deflater();
+		final byte[] buf = new byte[128];
+		deflater.setInput(content, 0, content.length);
+		deflater.finish();
+		do {
+			final int n = deflater.deflate(buf, 0, buf.length);
+			if (n > 0)
+				tinyPack.write(buf, 0, n);
+		} while (!deflater.finished());
+	}
+
+	private void digest(TemporaryBuffer.Heap buf) throws IOException {
+		MessageDigest md = Constants.newMessageDigest();
+		md.update(buf.toByteArray());
+		buf.write(md.digest());
+	}
+
+	private void openPack(TemporaryBuffer.Heap buf) throws IOException {
+		final byte[] raw = buf.toByteArray();
+		IndexPack ip = IndexPack.create(src, new ByteArrayInputStream(raw));
+		ip.setFixThin(true);
+		ip.index(PM);
+		ip.renameAndOpenPack();
+	}
+
+	private static PacketLineIn asPacketLineIn(TemporaryBuffer.Heap buf)
+			throws IOException {
+		return new PacketLineIn(new ByteArrayInputStream(buf.toByteArray()));
+	}
+
+	private static final class HidePrivateFilter implements RefFilter {
+		public Map<String, Ref> filter(Map<String, Ref> refs) {
+			Map<String, Ref> r = new HashMap<String, Ref>(refs);
+			assertNotNull(r.remove(R_PRIVATE));
+			return r;
+		}
+	}
+
+	private static URIish uriOf(Repository r) throws URISyntaxException {
+		return new URIish(r.getDirectory().getAbsolutePath());
+	}
+}

org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java

@@ -54,10 +54,7 @@ import java.io.RandomAccessFile;
 import java.security.MessageDigest;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.zip.CRC32;
 import java.util.zip.DataFormatException;
 import java.util.zip.Deflater;
@@ -173,7 +170,14 @@ public class IndexPack {
 
 	private PackedObjectInfo[] entries;
 
-	private Set<ObjectId> newObjectIds;
+	/**
+	 * Every object contained within the incoming pack.
+	 * <p>
+	 * This is a subset of {@link #entries}, as thin packs can add additional
+	 * objects to {@code entries} by copying already existing objects from the
+	 * repository onto the end of the thin pack to make it self-contained.
+	 */
+	private ObjectIdSubclassMap<ObjectId> newObjectIds;
 
 	private int deltaCount;
 
@@ -183,7 +187,14 @@ public class IndexPack {
 
 	private ObjectIdSubclassMap<DeltaChain> baseById;
 
-	private Set<ObjectId> baseIds;
+	/**
+	 * Objects referenced by their name from deltas, that aren't in this pack.
+	 * <p>
+	 * This is the set of objects that were copied onto the end of this pack to
+	 * make it complete. These objects were not transmitted by the remote peer,
+	 * but instead were assumed to already exist in the local repository.
+	 */
+	private ObjectIdSubclassMap<ObjectId> baseObjectIds;
 
 	private LongMap<UnresolvedDelta> baseByPos;
 
@@ -287,7 +298,7 @@ public class IndexPack {
 	 */
 	public void setNeedNewObjectIds(boolean b) {
 		if (b)
-			newObjectIds = new HashSet<ObjectId>();
+			newObjectIds = new ObjectIdSubclassMap<ObjectId>();
 		else
 			newObjectIds = null;
 	}
@@ -311,17 +322,17 @@ public class IndexPack {
 	}
 
 	/** @return the new objects that were sent by the user */
-	public Set<ObjectId> getNewObjectIds() {
+	public ObjectIdSubclassMap<ObjectId> getNewObjectIds() {
-		return newObjectIds == null ?
+		if (newObjectIds != null)
-				Collections.<ObjectId>emptySet() : newObjectIds;
+			return newObjectIds;
+		return new ObjectIdSubclassMap<ObjectId>();
 	}
 
-	/**
+	/** @return set of objects the incoming pack assumed for delta purposes */
-	 *  @return the set of objects the incoming pack assumed for delta purposes
+	public ObjectIdSubclassMap<ObjectId> getBaseObjectIds() {
-	 */
+		if (baseObjectIds != null)
-	public Set<ObjectId> getBaseObjectIds() {
+			return baseObjectIds;
-		return baseIds == null ?
+		return new ObjectIdSubclassMap<ObjectId>();
-				Collections.<ObjectId>emptySet() : baseIds;
 	}
 
 	/**
@@ -390,12 +401,6 @@ public class IndexPack {
 					if (packOut == null)
 						throw new IOException("need packOut");
 					resolveDeltas(progress);
-					if (needBaseObjectIds) {
-						baseIds = new HashSet<ObjectId>();
-						for (DeltaChain c : baseById) {
-							baseIds.add(c);
-						}
-					}
 					if (entryCount < objectCount) {
 						if (!fixThin) {
 							throw new IOException("pack has "
@@ -566,6 +571,9 @@ public class IndexPack {
 	private void fixThinPack(final ProgressMonitor progress) throws IOException {
 		growEntries();
 
+		if (needBaseObjectIds)
+			baseObjectIds = new ObjectIdSubclassMap<ObjectId>();
+
 		packDigest.reset();
 		originalEOF = packOut.length() - 20;
 		final Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false);
@@ -574,6 +582,8 @@ public class IndexPack {
 		for (final DeltaChain baseId : baseById) {
 			if (baseId.head == null)
 				continue;
+			if (needBaseObjectIds)
+				baseObjectIds.add(baseId);
 			final ObjectLoader ldr = repo.openObject(readCurs, baseId);
 			if (ldr == null) {
 				missing.add(baseId);

org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java

@@ -82,6 +82,8 @@ import org.eclipse.jgit.revwalk.RevBlob;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevFlag;
 import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.revwalk.RevTag;
+import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand.Result;
 import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
@@ -182,11 +184,7 @@ public class ReceivePack {
 	/** Lock around the received pack file, while updating refs. */
 	private PackLock packLock;
 
-	private boolean needNewObjectIds;
+	private boolean checkReferencedIsReachable;
-
-	private boolean needBaseObjectIds;
-
-	private boolean ensureObjectsProvidedVisible;
 
 	/**
 	 * Create a new pack receive for an open repository.
@@ -254,62 +252,36 @@ public class ReceivePack {
 	}
 
 	/**
-	 * Configure this receive pack instance to keep track of the objects assumed
+	 * @return true if this instance will validate all referenced, but not
-	 * for delta bases.
+	 *         supplied by the client, objects are reachable from another
-	 * <p>
+	 *         reference.
-	 * By default a receive pack doesn't save the objects that were used as
-	 * delta bases. Setting this flag to {@code true} will allow the caller to
-	 * use {@link #getBaseObjectIds()} to retrieve that list.
-	 *
-	 * @param b {@code true} to enable keeping track of delta bases.
-	 */
-	public void setNeedBaseObjectIds(boolean b) {
-		this.needBaseObjectIds = b;
-	}
-
-	/**
-	 *  @return the set of objects the incoming pack assumed for delta purposes
 	 */
-	public final Set<ObjectId> getBaseObjectIds() {
+	public boolean isCheckReferencedObjectsAreReachable() {
-		return ip.getBaseObjectIds();
+		return checkReferencedIsReachable;
 	}
 
 	/**
-	 * Configure this receive pack instance to keep track of new objects.
+	 * Validate all referenced but not supplied objects are reachable.
 	 * <p>
-	 * By default a receive pack doesn't save the new objects that were created
+	 * If enabled, this instance will verify that references to objects not
-	 * when it was instantiated. Setting this flag to {@code true} allows the
+	 * contained within the received pack are already reachable through at least
-	 * caller to use {@link #getNewObjectIds()} to retrieve that list.
+	 * one other reference selected by the {@link #getRefFilter()} and displayed
-	 *
+	 * as part of {@link #getAdvertisedRefs()}.
-	 * @param b {@code true} to enable keeping track of new objects.
-	 */
-	public void setNeedNewObjectIds(boolean b) {
-		this.needNewObjectIds = b;
-	}
-
-	/** @return the new objects that were sent by the user */
-	public final Set<ObjectId> getNewObjectIds() {
-		return ip.getNewObjectIds();
-	}
-
-	/**
-	 * Configure this receive pack instance to ensure that the provided
-	 * objects are visible to the user.
 	 * <p>
-	 * By default, a receive pack assumes that its user will only provide
+	 * This feature is useful when the application doesn't trust the client to
-	 * references to objects that it can see. Setting this flag to {@code true}
+	 * not provide a forged SHA-1 reference to an object, in an attempt to
-	 * will add an additional check that verifies that the objects that were
+	 * access parts of the DAG that they aren't allowed to see and which have
-	 * provided are reachable by a tree or a commit that the user can see.
+	 * been hidden from them via the configured {@link RefFilter}.
 	 * <p>
-	 * This option is useful when the code doesn't trust the client not to
+	 * Enabling this feature may imply at least some, if not all, of the same
-	 * provide a forged SHA-1 reference to an object in an attempt to access
+	 * functionality performed by {@link #setCheckReceivedObjects(boolean)}.
-	 * parts of the DAG that they aren't allowed to see, via the configured
+	 * Applications are encouraged to enable both features, if desired.
-	 * {@link RefFilter}.
 	 *
-	 * @param b {@code true} to enable the additional check.
+	 * @param b
+	 *            {@code true} to enable the additional check.
 	 */
-	public void setEnsureProvidedObjectsVisible(boolean b) {
+	public void setCheckReferencedObjectsAreReachable(boolean b) {
-		this.ensureObjectsProvidedVisible = b;
+		this.checkReferencedIsReachable = b;
 	}
 
 	/**
@@ -652,8 +624,9 @@ public class ReceivePack {
 			if (needPack()) {
 				try {
 					receivePack();
-					if (isCheckReceivedObjects())
+					if (needCheckConnectivity())
 						checkConnectivity();
+					ip = null;
 					unpackError = null;
 				} catch (IOException err) {
 					unpackError = err;
@@ -801,9 +774,8 @@ public class ReceivePack {
 
 		ip = IndexPack.create(db, rawIn);
 		ip.setFixThin(true);
-		ip.setNeedNewObjectIds(needNewObjectIds || ensureObjectsProvidedVisible);
+		ip.setNeedNewObjectIds(checkReferencedIsReachable);
-		ip.setNeedBaseObjectIds(needBaseObjectIds
+		ip.setNeedBaseObjectIds(checkReferencedIsReachable);
-				|| ensureObjectsProvidedVisible);
 		ip.setObjectChecking(isCheckReceivedObjects());
 		ip.index(NullProgressMonitor.INSTANCE);
 
@@ -816,7 +788,21 @@ public class ReceivePack {
 			timeoutIn.setTimeout(timeout * 1000);
 	}
 
+	private boolean needCheckConnectivity() {
+		return isCheckReceivedObjects()
+				|| isCheckReferencedObjectsAreReachable();
+	}
+
 	private void checkConnectivity() throws IOException {
+		ObjectIdSubclassMap<ObjectId> baseObjects = null;
+		ObjectIdSubclassMap<ObjectId> providedObjects = null;
+
+		if (checkReferencedIsReachable) {
+			baseObjects = ip.getBaseObjectIds();
+			providedObjects = ip.getNewObjectIds();
+		}
+		ip = null;
+
 		final ObjectWalk ow = new ObjectWalk(db);
 		for (final ReceiveCommand cmd : commands) {
 			if (cmd.getResult() != Result.NOT_ATTEMPTED)
@@ -825,34 +811,44 @@ public class ReceivePack {
 				continue;
 			ow.markStart(ow.parseAny(cmd.getNewId()));
 		}
-		for (final Ref ref : refs.values())
+		for (final Ref ref : refs.values()) {
-			ow.markUninteresting(ow.parseAny(ref.getObjectId()));
+			RevObject o = ow.parseAny(ref.getObjectId());
+			ow.markUninteresting(o);
+
+			if (checkReferencedIsReachable && !baseObjects.isEmpty()) {
+				while (o instanceof RevTag)
+					o = ((RevTag) o).getObject();
+				if (o instanceof RevCommit)
+					o = ((RevCommit) o).getTree();
+				if (o instanceof RevTree)
+					ow.markUninteresting(o);
+			}
+		}
 
-		ObjectIdSubclassMap<ObjectId> provided =
+		if (checkReferencedIsReachable) {
-			new ObjectIdSubclassMap<ObjectId>();
+			for (ObjectId id : baseObjects) {
-		if (ensureObjectsProvidedVisible) {
-			for (ObjectId id : getBaseObjectIds()) {
 				   RevObject b = ow.lookupAny(id, Constants.OBJ_BLOB);
 				   if (!b.has(RevFlag.UNINTERESTING))
 				     throw new MissingObjectException(b, b.getType());
 			}
-			for (ObjectId id : getNewObjectIds()) {
-				provided.add(id);
-			}
 		}
 
 		RevCommit c;
 		while ((c = ow.next()) != null) {
-			if (ensureObjectsProvidedVisible && !provided.contains(c))
+			if (checkReferencedIsReachable && !providedObjects.contains(c))
 				throw new MissingObjectException(c, Constants.TYPE_COMMIT);
 		}
 
 		RevObject o;
 		while ((o = ow.nextObject()) != null) {
-			if (o instanceof RevBlob && !db.hasObject(o))
+			if (checkReferencedIsReachable) {
-				throw new MissingObjectException(o, Constants.TYPE_BLOB);
+				if (providedObjects.contains(o))
+					continue;
+				else
+					throw new MissingObjectException(o, o.getType());
+			}
 
-			if (ensureObjectsProvidedVisible && !provided.contains(o))
+			if (o instanceof RevBlob && !db.hasObject(o))
 				throw new MissingObjectException(o, Constants.TYPE_BLOB);
 		}
 	}

org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java

@@ -111,6 +111,14 @@ class TransportLocal extends Transport implements PackTransport {
 		remoteGitDir = d;
 	}
 
+	UploadPack createUploadPack(final Repository dst) {
+		return new UploadPack(dst);
+	}
+
+	ReceivePack createReceivePack(final Repository dst) {
+		return new ReceivePack(dst);
+	}
+
 	@Override
 	public FetchConnection openFetch() throws TransportException {
 		final String up = getOptionUploadPack();
@@ -197,7 +205,7 @@ class TransportLocal extends Transport implements PackTransport {
 			worker = new Thread("JGit-Upload-Pack") {
 				public void run() {
 					try {
-						final UploadPack rp = new UploadPack(dst);
+						final UploadPack rp = createUploadPack(dst);
 						rp.upload(out_r, in_w, null);
 					} catch (IOException err) {
 						// Client side of the pipes should report the problem.
@@ -329,7 +337,7 @@ class TransportLocal extends Transport implements PackTransport {
 			worker = new Thread("JGit-Receive-Pack") {
 				public void run() {
 					try {
-						final ReceivePack rp = new ReceivePack(dst);
+						final ReceivePack rp = createReceivePack(dst);
 						rp.receive(out_r, in_w, System.err);
 					} catch (IOException err) {
 						// Client side of the pipes should report the problem.