From 7a91b180c193fa9fc168e4ccdf2215a41db73353 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 14 Apr 2010 16:53:43 -0700 Subject: [PATCH 1/7] ReceivePack: fix ensureProvidedObjectsVisible on thin packs If ensureProvidedObjectsVisible is enabled we expected any trees or blobs directly reachable from an advertised reference to be marked with UNINTERESTING. Unfortunately ObjectWalk doesn't bother setting this until the traversal is complete. Even then it won't necessarily set it on every tree if the corresponding commit wasn't popped. When we are going to check the base objects for the received pack, ensure the UNINTERESTING flag gets carried into every immediately reachable tree or blob, because these are the ones that the client might try to use as delta bases in a thin pack. Change-Id: I5d5fdcf07e25ac9fc360e79a25dff491925e4101 Signed-off-by: Shawn O. Pearce --- .../transport/ReceivePackRefFilterTest.java | 477 ++++++++++++++++++ .../eclipse/jgit/transport/ReceivePack.java | 26 +- .../jgit/transport/TransportLocal.java | 12 +- 3 files changed, 510 insertions(+), 5 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java new file mode 100644 index 000000000..824eecff0 --- /dev/null +++ b/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 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.setEnsureProvidedObjectsVisible(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.setEnsureProvidedObjectsVisible(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.setEnsureProvidedObjectsVisible(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.setEnsureProvidedObjectsVisible(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.setEnsureProvidedObjectsVisible(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.setEnsureProvidedObjectsVisible(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 filter(Map refs) { + Map r = new HashMap(refs); + assertNotNull(r.remove(R_PRIVATE)); + return r; + } + } + + private static URIish uriOf(Repository r) throws URISyntaxException { + return new URIish(r.getDirectory().getAbsolutePath()); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 6ba326c00..8be9ff46a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/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; @@ -817,6 +819,13 @@ public class ReceivePack { } private void checkConnectivity() throws IOException { + final Set baseObjects; + + if (ensureObjectsProvidedVisible) + baseObjects = getBaseObjectIds(); + else + baseObjects = Collections.emptySet(); + final ObjectWalk ow = new ObjectWalk(db); for (final ReceiveCommand cmd : commands) { if (cmd.getResult() != Result.NOT_ATTEMPTED) @@ -825,13 +834,24 @@ public class ReceivePack { continue; ow.markStart(ow.parseAny(cmd.getNewId())); } - for (final Ref ref : refs.values()) - ow.markUninteresting(ow.parseAny(ref.getObjectId())); + for (final Ref ref : refs.values()) { + RevObject o = ow.parseAny(ref.getObjectId()); + ow.markUninteresting(o); + + if (ensureObjectsProvidedVisible && !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 provided = new ObjectIdSubclassMap(); if (ensureObjectsProvidedVisible) { - for (ObjectId id : getBaseObjectIds()) { + for (ObjectId id : baseObjects) { RevObject b = ow.lookupAny(id, Constants.OBJ_BLOB); if (!b.has(RevFlag.UNINTERESTING)) throw new MissingObjectException(b, b.getType()); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java index b9b9dbd00..22c436de3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java +++ b/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. From 8279361de8ca9a216c6dbfcc02591c011308223b Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 08:16:23 -0700 Subject: [PATCH 2/7] ReceivePack: Discard IndexPack as soon as possible The IndexPack object carries a good bit of state within itself about the objects received over the wire. The earlier we can discard it, the sooner the GC is able to reclaim this chunk of memory for other uses. So drop it as soon as we are certain the pack is valid and we have no connectivity concerns. Change-Id: I1e8bc87c2e9183733043622237a064e55957891f Signed-off-by: Shawn O. Pearce --- org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java | 1 + 1 file changed, 1 insertion(+) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 8be9ff46a..46912847d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -656,6 +656,7 @@ public class ReceivePack { receivePack(); if (isCheckReceivedObjects()) checkConnectivity(); + ip = null; unpackError = null; } catch (IOException err) { unpackError = err; From 329a0e1689f2493b7034d15185f4fbf59c9e49bb Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 08:11:30 -0700 Subject: [PATCH 3/7] ReceivePack: Remove need new,base object id properties These are more like internal implementation details of how IndexPack works with ReceivePack to validate the incoming object stream. Callers who are embedding the ReceivePack logic in their own application don't really need to know the details of which objects were used for delta bases in the incoming thin pack, or exactly which objects were newly transmitted. Hide these from the API, as exposing them through ReceivePack was an early mistake. Change-Id: I7ee44a314fa19e6a8520472ce05de92c324ad43e Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/transport/ReceivePack.java | 53 ++----------------- 1 file changed, 4 insertions(+), 49 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 46912847d..8dbeb9598 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -184,10 +184,6 @@ public class ReceivePack { /** Lock around the received pack file, while updating refs. */ private PackLock packLock; - private boolean needNewObjectIds; - - private boolean needBaseObjectIds; - private boolean ensureObjectsProvidedVisible; /** @@ -255,45 +251,6 @@ public class ReceivePack { return refs; } - /** - * Configure this receive pack instance to keep track of the objects assumed - * for delta bases. - *

- * 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 getBaseObjectIds() { - return ip.getBaseObjectIds(); - } - - /** - * Configure this receive pack instance to keep track of new objects. - *

- * By default a receive pack doesn't save the new objects that were created - * when it was instantiated. Setting this flag to {@code true} allows the - * caller to use {@link #getNewObjectIds()} to retrieve that list. - * - * @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 getNewObjectIds() { - return ip.getNewObjectIds(); - } - /** * Configure this receive pack instance to ensure that the provided * objects are visible to the user. @@ -804,9 +761,8 @@ public class ReceivePack { ip = IndexPack.create(db, rawIn); ip.setFixThin(true); - ip.setNeedNewObjectIds(needNewObjectIds || ensureObjectsProvidedVisible); - ip.setNeedBaseObjectIds(needBaseObjectIds - || ensureObjectsProvidedVisible); + ip.setNeedNewObjectIds(ensureObjectsProvidedVisible); + ip.setNeedBaseObjectIds(ensureObjectsProvidedVisible); ip.setObjectChecking(isCheckReceivedObjects()); ip.index(NullProgressMonitor.INSTANCE); @@ -823,7 +779,7 @@ public class ReceivePack { final Set baseObjects; if (ensureObjectsProvidedVisible) - baseObjects = getBaseObjectIds(); + baseObjects = ip.getBaseObjectIds(); else baseObjects = Collections.emptySet(); @@ -857,9 +813,8 @@ public class ReceivePack { if (!b.has(RevFlag.UNINTERESTING)) throw new MissingObjectException(b, b.getType()); } - for (ObjectId id : getNewObjectIds()) { + for (ObjectId id : ip.getNewObjectIds()) provided.add(id); - } } RevCommit c; From 2bb8defa5447c15a4fcbf23aaace9f11995f3ad0 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 08:52:43 -0700 Subject: [PATCH 4/7] IndexPack: Tighten up new and base object bookkeeping The only current consumer of these collections is ReceivePack, where it needs to test ObjectId equality between a RevObject and an ObjectId. There we were copying from a traditional HashSet into an ObjectIdSubclassMap, as the latter can perform hashing using ObjectId's native value support, bypassing RevObject's override on hashCode() and equals(). Instead of doing that copy, directly create ObjectIdSubclassMap instances inside of ReceivePack. We also only need to record the objects that do not appear in the incoming pack, and were therefore copied from the local repositiory in order to complete delta resolution. Instead of listing everything that used an OBJ_REF_DELTA format, list only the objects that we pulled from the destination repository via a normal ObjectLoader. ReceivePack can now discard the IndexPack object, and all of its other data, as soon as these collections are held by the check connectivity method. This frees up memory for the ObjectWalk's own RevObject pool. Change-Id: I22ef71b45c2045a0202e7fd550a770ee1f6f38a6 Signed-off-by: Shawn O. Pearce --- .../org/eclipse/jgit/transport/IndexPack.java | 52 +++++++++++-------- .../eclipse/jgit/transport/ReceivePack.java | 18 +++---- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java index 29f200c52..6eeccea84 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java +++ b/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 newObjectIds; + /** + * Every object contained within the incoming pack. + *

+ * 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 newObjectIds; private int deltaCount; @@ -183,7 +187,14 @@ public class IndexPack { private ObjectIdSubclassMap baseById; - private Set baseIds; + /** + * Objects referenced by their name from deltas, that aren't in this pack. + *

+ * 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 baseObjectIds; private LongMap baseByPos; @@ -287,7 +298,7 @@ public class IndexPack { */ public void setNeedNewObjectIds(boolean b) { if (b) - newObjectIds = new HashSet(); + newObjectIds = new ObjectIdSubclassMap(); else newObjectIds = null; } @@ -311,17 +322,17 @@ public class IndexPack { } /** @return the new objects that were sent by the user */ - public Set getNewObjectIds() { - return newObjectIds == null ? - Collections.emptySet() : newObjectIds; + public ObjectIdSubclassMap getNewObjectIds() { + if (newObjectIds != null) + return newObjectIds; + return new ObjectIdSubclassMap(); } - /** - * @return the set of objects the incoming pack assumed for delta purposes - */ - public Set getBaseObjectIds() { - return baseIds == null ? - Collections.emptySet() : baseIds; + /** @return set of objects the incoming pack assumed for delta purposes */ + public ObjectIdSubclassMap getBaseObjectIds() { + if (baseObjectIds != null) + return baseObjectIds; + return new ObjectIdSubclassMap(); } /** @@ -390,12 +401,6 @@ public class IndexPack { if (packOut == null) throw new IOException("need packOut"); resolveDeltas(progress); - if (needBaseObjectIds) { - baseIds = new HashSet(); - 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(); + 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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 8dbeb9598..cce0a17d0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -776,12 +776,14 @@ public class ReceivePack { } private void checkConnectivity() throws IOException { - final Set baseObjects; + ObjectIdSubclassMap baseObjects = null; + ObjectIdSubclassMap providedObjects = null; - if (ensureObjectsProvidedVisible) + if (ensureObjectsProvidedVisible) { baseObjects = ip.getBaseObjectIds(); - else - baseObjects = Collections.emptySet(); + providedObjects = ip.getNewObjectIds(); + } + ip = null; final ObjectWalk ow = new ObjectWalk(db); for (final ReceiveCommand cmd : commands) { @@ -805,21 +807,17 @@ public class ReceivePack { } } - ObjectIdSubclassMap provided = - new ObjectIdSubclassMap(); if (ensureObjectsProvidedVisible) { for (ObjectId id : baseObjects) { RevObject b = ow.lookupAny(id, Constants.OBJ_BLOB); if (!b.has(RevFlag.UNINTERESTING)) throw new MissingObjectException(b, b.getType()); } - for (ObjectId id : ip.getNewObjectIds()) - provided.add(id); } RevCommit c; while ((c = ow.next()) != null) { - if (ensureObjectsProvidedVisible && !provided.contains(c)) + if (ensureObjectsProvidedVisible && !providedObjects.contains(c)) throw new MissingObjectException(c, Constants.TYPE_COMMIT); } @@ -828,7 +826,7 @@ public class ReceivePack { if (o instanceof RevBlob && !db.hasObject(o)) throw new MissingObjectException(o, Constants.TYPE_BLOB); - if (ensureObjectsProvidedVisible && !provided.contains(o)) + if (ensureObjectsProvidedVisible && !providedObjects.contains(o)) throw new MissingObjectException(o, Constants.TYPE_BLOB); } } From 6029bb24adafd4434f362c58e2f00c0057502f45 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 08:55:44 -0700 Subject: [PATCH 5/7] ReceivePack: Correct type of not provided object If a tree was referenced but not provided in the pack, report it as a missing tree and not as a missing blob. Change-Id: Iab05705349cdf0d30cc3f8afc6698a8d2a941343 Signed-off-by: Shawn O. Pearce --- .../src/org/eclipse/jgit/transport/ReceivePack.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index cce0a17d0..85522edc4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -827,7 +827,7 @@ public class ReceivePack { throw new MissingObjectException(o, Constants.TYPE_BLOB); if (ensureObjectsProvidedVisible && !providedObjects.contains(o)) - throw new MissingObjectException(o, Constants.TYPE_BLOB); + throw new MissingObjectException(o, o.getType()); } } From a770205070b52199e5c561f407ee0b0168dd8b9f Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 16:37:29 -0700 Subject: [PATCH 6/7] ReceivePack: Micro-optimize object lookup when checking connectivity If we are checking the visibility of everything referenced in the pack that isn't already reachable by a reference, it needs to be in the provided set. Since the provided set lists everything that is in this pack, we can avoid checking to see if the blob exists on disk, because we know it should be there, it was found in the pack we just consumed. Change-Id: Ie3c7746f734d13077242100a68e048f1ac18c34a Signed-off-by: Shawn O. Pearce --- .../src/org/eclipse/jgit/transport/ReceivePack.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 85522edc4..7c113d65f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -823,11 +823,15 @@ public class ReceivePack { RevObject o; while ((o = ow.nextObject()) != null) { + if (ensureObjectsProvidedVisible) { + if (providedObjects.contains(o)) + continue; + else + throw new MissingObjectException(o, o.getType()); + } + if (o instanceof RevBlob && !db.hasObject(o)) throw new MissingObjectException(o, Constants.TYPE_BLOB); - - if (ensureObjectsProvidedVisible && !providedObjects.contains(o)) - throw new MissingObjectException(o, o.getType()); } } From 585dcb7a1ce7fd9e9cbd8f61f8bc6ab9afcb329c Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 17:03:54 -0700 Subject: [PATCH 7/7] ReceivePack: Clarify the check reachable option This option was mis-named from day 1. Its not checking that the objects provided by the client are reachable, its actually doing a scan to prove that objects referenced by the client are already reachable through another reference on the server, or were sent as part of the pack from the client. Rename it checkReferencedObjectsAreReachable, since we really are trying to validate that objects referenced by the client's actions are reachable to the client. We also need to ensure we run checkConnectivity() anytime this is enabled, even if the caller didn't turn on fsck for object formats. Otherwise the check would be completely bypassed. Change-Id: Ic352ddb0ca8464d407c6da5c83573093e018af19 Signed-off-by: Shawn O. Pearce --- .../transport/ReceivePackRefFilterTest.java | 12 ++-- .../eclipse/jgit/transport/ReceivePack.java | 62 ++++++++++++------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java index 824eecff0..2ec0acaa3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java @@ -186,7 +186,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final ReceivePack rp = super.createReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); return rp; } @@ -229,7 +229,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024); final ReceivePack rp = new ReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); @@ -264,7 +264,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024); final ReceivePack rp = new ReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); @@ -305,7 +305,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024); final ReceivePack rp = new ReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); @@ -347,7 +347,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024); final ReceivePack rp = new ReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); @@ -386,7 +386,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase { final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024); final ReceivePack rp = new ReceivePack(dst); rp.setCheckReceivedObjects(true); - rp.setEnsureProvidedObjectsVisible(true); + rp.setCheckReferencedObjectsAreReachable(true); rp.setRefFilter(new HidePrivateFilter()); rp.receive(new ByteArrayInputStream(inBuf.toByteArray()), outBuf, null); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index 7c113d65f..4e62d7427 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -184,7 +184,7 @@ public class ReceivePack { /** Lock around the received pack file, while updating refs. */ private PackLock packLock; - private boolean ensureObjectsProvidedVisible; + private boolean checkReferencedIsReachable; /** * Create a new pack receive for an open repository. @@ -252,23 +252,36 @@ public class ReceivePack { } /** - * Configure this receive pack instance to ensure that the provided - * objects are visible to the user. + * @return true if this instance will validate all referenced, but not + * supplied by the client, objects are reachable from another + * reference. + */ + public boolean isCheckReferencedObjectsAreReachable() { + return checkReferencedIsReachable; + } + + /** + * Validate all referenced but not supplied objects are reachable. *

- * By default, a receive pack assumes that its user will only provide - * references to objects that it can see. Setting this flag to {@code true} - * will add an additional check that verifies that the objects that were - * provided are reachable by a tree or a commit that the user can see. + * If enabled, this instance will verify that references to objects not + * contained within the received pack are already reachable through at least + * one other reference selected by the {@link #getRefFilter()} and displayed + * as part of {@link #getAdvertisedRefs()}. *

- * This option is useful when the code doesn't trust the client not to - * provide a forged SHA-1 reference to an object in an attempt to access - * parts of the DAG that they aren't allowed to see, via the configured - * {@link RefFilter}. + * This feature is useful when the application doesn't trust the client to + * not provide a forged SHA-1 reference to an object, in an attempt to + * access parts of the DAG that they aren't allowed to see and which have + * been hidden from them via the configured {@link RefFilter}. + *

+ * Enabling this feature may imply at least some, if not all, of the same + * functionality performed by {@link #setCheckReceivedObjects(boolean)}. + * Applications are encouraged to enable both features, if desired. * - * @param b {@code true} to enable the additional check. + * @param b + * {@code true} to enable the additional check. */ - public void setEnsureProvidedObjectsVisible(boolean b) { - this.ensureObjectsProvidedVisible = b; + public void setCheckReferencedObjectsAreReachable(boolean b) { + this.checkReferencedIsReachable = b; } /** @@ -611,7 +624,7 @@ public class ReceivePack { if (needPack()) { try { receivePack(); - if (isCheckReceivedObjects()) + if (needCheckConnectivity()) checkConnectivity(); ip = null; unpackError = null; @@ -761,8 +774,8 @@ public class ReceivePack { ip = IndexPack.create(db, rawIn); ip.setFixThin(true); - ip.setNeedNewObjectIds(ensureObjectsProvidedVisible); - ip.setNeedBaseObjectIds(ensureObjectsProvidedVisible); + ip.setNeedNewObjectIds(checkReferencedIsReachable); + ip.setNeedBaseObjectIds(checkReferencedIsReachable); ip.setObjectChecking(isCheckReceivedObjects()); ip.index(NullProgressMonitor.INSTANCE); @@ -775,11 +788,16 @@ public class ReceivePack { timeoutIn.setTimeout(timeout * 1000); } + private boolean needCheckConnectivity() { + return isCheckReceivedObjects() + || isCheckReferencedObjectsAreReachable(); + } + private void checkConnectivity() throws IOException { ObjectIdSubclassMap baseObjects = null; ObjectIdSubclassMap providedObjects = null; - if (ensureObjectsProvidedVisible) { + if (checkReferencedIsReachable) { baseObjects = ip.getBaseObjectIds(); providedObjects = ip.getNewObjectIds(); } @@ -797,7 +815,7 @@ public class ReceivePack { RevObject o = ow.parseAny(ref.getObjectId()); ow.markUninteresting(o); - if (ensureObjectsProvidedVisible && !baseObjects.isEmpty()) { + if (checkReferencedIsReachable && !baseObjects.isEmpty()) { while (o instanceof RevTag) o = ((RevTag) o).getObject(); if (o instanceof RevCommit) @@ -807,7 +825,7 @@ public class ReceivePack { } } - if (ensureObjectsProvidedVisible) { + if (checkReferencedIsReachable) { for (ObjectId id : baseObjects) { RevObject b = ow.lookupAny(id, Constants.OBJ_BLOB); if (!b.has(RevFlag.UNINTERESTING)) @@ -817,13 +835,13 @@ public class ReceivePack { RevCommit c; while ((c = ow.next()) != null) { - if (ensureObjectsProvidedVisible && !providedObjects.contains(c)) + if (checkReferencedIsReachable && !providedObjects.contains(c)) throw new MissingObjectException(c, Constants.TYPE_COMMIT); } RevObject o; while ((o = ow.nextObject()) != null) { - if (ensureObjectsProvidedVisible) { + if (checkReferencedIsReachable) { if (providedObjects.contains(o)) continue; else