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 824eecff08719c992a1ed86d81caea8cdcbccce9..2ec0acaa3aa6a2fde89e52ba177c173bcf02bc6d 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 @@ ReceivePack createReceivePack(final Repository db) { 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 void testCreateBranchAtHiddenCommitFails() throws Exception { 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 void testUsingHiddenDeltaBaseFails() throws Exception { 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 void testUsingHiddenCommonBlobFails() throws Exception { 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 void testUsingUnknownBlobFails() throws Exception { 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 void testUsingUnknownTreeFails() throws Exception { 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 7c113d65f67cf9c9520fd797cb0d7b522dfe4643..4e62d7427f4cc036684863eec6b738fb8e9c2d54 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 final Map<String, Ref> getAdvertisedRefs() { } /** - * 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. * <p> - * 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()}. * <p> - * 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}. + * <p> + * 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 @@ private void service() throws IOException { if (needPack()) { try { receivePack(); - if (isCheckReceivedObjects()) + if (needCheckConnectivity()) checkConnectivity(); ip = null; unpackError = null; @@ -761,8 +774,8 @@ private void receivePack() throws IOException { 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 @@ private void receivePack() throws IOException { timeoutIn.setTimeout(timeout * 1000); } + private boolean needCheckConnectivity() { + return isCheckReceivedObjects() + || isCheckReferencedObjectsAreReachable(); + } + private void checkConnectivity() throws IOException { ObjectIdSubclassMap<ObjectId> baseObjects = null; ObjectIdSubclassMap<ObjectId> providedObjects = null; - if (ensureObjectsProvidedVisible) { + if (checkReferencedIsReachable) { baseObjects = ip.getBaseObjectIds(); providedObjects = ip.getNewObjectIds(); } @@ -797,7 +815,7 @@ private void checkConnectivity() throws IOException { 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 @@ private void checkConnectivity() throws IOException { } } - 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 @@ private void checkConnectivity() throws IOException { 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