[JGIT PATCH 1/2] Check object connectivity during fetch if fsck is enabled

[JGIT PATCH 1/2] Check object connectivity during fetch if fsck is enabled

[Shawn O. Pearce]

If we are fetching over a pack oriented connection and we are doing
object-level fsck validation we need to also verify the graph is
fully connected after the fetch is complete.  This additional check
is necessary to ensure the peer didn't omit objects that we don't
have, but which are listed as needing to be present.

On the walk style fetch connection we can bypass this check, as the
connectivity was implicitly verified by the walker as it downloaded
objects and built its queue of things to fetch.  Native pack and
bundle transports however do not have this check built into them,
and require that we execute the work ourselves.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 This is a follow-on to my fsck during fetch series, which is already
 in master.  Making calls to ObjectChecker isn't enough as it does
 not check the DAG's connectivity.

 .../jgit/transport/BasePackFetchConnection.java    |    4 +++
 .../spearce/jgit/transport/FetchConnection.java    |   22 ++++++++++++++++++++
 .../org/spearce/jgit/transport/FetchProcess.java   |   13 ++++++++++-
 .../spearce/jgit/transport/TransportBundle.java    |    4 +++
 .../jgit/transport/WalkFetchConnection.java        |    4 +++
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index a542eb7..542a8a9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -146,6 +146,10 @@ public boolean didFetchIncludeTags() {
 		return false;
 	}
 
+	public boolean didFetchTestConnectivity() {
+		return false;
+	}
+
 	protected void doFetch(final ProgressMonitor monitor,
 			final Collection<Ref> want) throws TransportException {
 		try {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
index 9d25b0d..d93972d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchConnection.java
@@ -111,4 +111,26 @@ public void fetch(final ProgressMonitor monitor, final Collection<Ref> want)
 	 *         false if tags were not implicitly obtained.
 	 */
 	public boolean didFetchIncludeTags();
+
+	/**
+	 * Did the last {@link #fetch(ProgressMonitor, Collection)} validate graph?
+	 * <p>
+	 * Some transports walk the object graph on the client side, with the client
+	 * looking for what objects it is missing and requesting them individually
+	 * from the remote peer. By virtue of completing the fetch call the client
+	 * implicitly tested the object connectivity, as every object in the graph
+	 * was either already local or was requested successfully from the peer. In
+	 * such transports this method returns true.
+	 * <p>
+	 * Some transports assume the remote peer knows the Git object graph and is
+	 * able to supply a fully connected graph to the client (although it may
+	 * only be transferring the parts the client does not yet have). Its faster
+	 * to assume such remote peers are well behaved and send the correct
+	 * response to the client. In such tranports this method returns false.
+	 * 
+	 * @return true if the last fetch had to perform a connectivity check on the
+	 *         client side in order to succeed; false if the last fetch assumed
+	 *         the remote peer supplied a complete graph.
+	 */
+	public boolean didFetchTestConnectivity();
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
index 654572d..bb2d051 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
@@ -118,7 +118,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 
 			final boolean includedTags;
 			if (!askFor.isEmpty() && !askForIsComplete()) {
-				conn.fetch(monitor, askFor.values());
+				fetchObjects(monitor);
 				includedTags = conn.didFetchIncludeTags();
 
 				// Connection was used for object transfer. If we
@@ -143,7 +143,7 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 				if (!askFor.isEmpty() && (!includedTags || !askForIsComplete())) {
 					reopenConnection();
 					if (!askFor.isEmpty())
-						conn.fetch(monitor, askFor.values());
+						fetchObjects(monitor);
 				}
 			}
 		} finally {
@@ -171,6 +171,15 @@ else if (tagopt == TagOpt.FETCH_TAGS)
 		}
 	}
 
+	private void fetchObjects(final ProgressMonitor monitor)
+			throws TransportException {
+		conn.fetch(monitor, askFor.values());
+		if (transport.isCheckFetchedObjects()
+				&& !conn.didFetchTestConnectivity() && !askForIsComplete())
+			throw new TransportException(transport.getURI(),
+					"peer did not supply a complete object graph");
+	}
+
 	private void closeConnection() {
 		if (conn != null) {
 			conn.close();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
index 5b321a0..7d38b02 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportBundle.java
@@ -165,6 +165,10 @@ private String readLine(final byte[] hdrbuf) throws IOException {
 			return RawParseUtils.decode(Constants.CHARSET, hdrbuf, 0, lf);
 		}
 
+		public boolean didFetchTestConnectivity() {
+			return false;
+		}
+
 		@Override
 		protected void doFetch(final ProgressMonitor monitor,
 				final Collection<Ref> want) throws TransportException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 5638454..d089f7b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -189,6 +189,10 @@ WalkFetchConnection(final WalkTransport wt, final WalkRemoteObjectDatabase w) {
 		workQueue = new LinkedList<ObjectId>();
 	}
 
+	public boolean didFetchTestConnectivity() {
+		return true;
+	}
+
 	@Override
 	protected void doFetch(final ProgressMonitor monitor,
 			final Collection<Ref> want) throws TransportException {
-- 
1.6.0.2.706.g340fc

[JGIT PATCH 2/2] Add --[no-]thin and --[no-]fsck optiosn to fetch command line tool

[Shawn O. Pearce]

This way users can force verification on the fly, such as when
fetching from an untrusted URL pasted on the command line.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/pgm/Fetch.java            |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
index e14e213..ad7e08f 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Fetch.java
@@ -54,6 +54,22 @@
 	@Option(name = "--verbose", aliases = { "-v" }, usage = "be more verbose")
 	private boolean verbose;
 
+	@Option(name = "--fsck", usage = "perform fsck style checks on receive")
+	private Boolean fsck;
+
+	@Option(name = "--no-fsck")
+	void nofsck(final boolean ignored) {
+		fsck = Boolean.FALSE;
+	}
+
+	@Option(name = "--thin", usage = "fetch thin pack")
+	private Boolean thin;
+
+	@Option(name = "--no-thin")
+	void nothin(final boolean ignored) {
+		thin = Boolean.FALSE;
+	}
+
 	@Argument(index = 0, metaVar = "uri-ish")
 	private String remote = "origin";
 
@@ -63,6 +79,10 @@
 	@Override
 	protected void run() throws Exception {
 		final Transport tn = Transport.open(db, remote);
+		if (fsck != null)
+			tn.setCheckFetchedObjects(fsck.booleanValue());
+		if (thin != null)
+			tn.setFetchThin(thin.booleanValue());
 		final FetchResult r;
 		try {
 			r = tn.fetch(new TextProgressMonitor(), toget);
-- 
1.6.0.2.706.g340fc