[JGIT PATCH] index-pack: Avoid disk corruption in objects appended to thin packs

[JGIT PATCH] index-pack: Avoid disk corruption in objects appended to thin packs

[Shawn O. Pearce]

When we are appending onto a thin pack we need to make sure the whole
object we wrote into the pack is what is there when we re-read the
pack to compute the final footer.

Nicolas Pitre pointed out the need to perform this sort of compare
in C Git.  We missed it in my first round of index-pack improvements.

We also now take advantage of "roughly" block aligned reads during
the fixup operation.  4k is a popular filesystem block size and by
arranging the code a little differently we can always perform reads
on 4k alignments within the file.  This should make it easier for
the OS to move data to us.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/transport/IndexPack.java  |   47 ++++++++++++++-----
 1 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 7b1f7ee..d8e6548 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -76,7 +76,7 @@
 	/** Progress message when computing names of delta compressed objects. */
 	public static final String PROGRESS_RESOLVE_DELTA = "Resolving deltas";
 
-	private static final int BUFFER_SIZE = 2048;
+	private static final int BUFFER_SIZE = 4096;
 
 	/**
 	 * Create an index pack instance to load a new pack into a repository.
@@ -403,6 +403,7 @@ while (bi < b.size()) {
 	private void fixThinPack(final ProgressMonitor progress) throws IOException {
 		growEntries();
 
+		packDigest.reset();
 		originalEOF = packOut.length() - 20;
 		final Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false);
 		long end = originalEOF;
@@ -432,7 +433,7 @@ if (!baseById.isEmpty()) {
 			throw new MissingObjectException(need, "delta base");
 		}
 
-		fixHeaderFooter();
+		fixHeaderFooter(packcsum, packDigest.digest());
 	}
 
 	private void writeWhole(final Deflater def, final int typeCode,
@@ -446,6 +447,7 @@ while (sz > 0) {
 			buf[hdrlen++] = (byte) (sz & 0x7f);
 			sz >>>= 7;
 		}
+		packDigest.update(buf, 0, hdrlen);
 		crc.update(buf, 0, hdrlen);
 		packOut.write(buf, 0, hdrlen);
 		def.reset();
@@ -453,37 +455,56 @@ while (sz > 0) {
 		def.finish();
 		while (!def.finished()) {
 			final int datlen = def.deflate(buf);
+			packDigest.update(buf, 0, datlen);
 			crc.update(buf, 0, datlen);
 			packOut.write(buf, 0, datlen);
 		}
 	}
 
-	private void fixHeaderFooter() throws IOException {
+	private void fixHeaderFooter(final byte[] origcsum, final byte[] tailcsum)
+			throws IOException {
 		final MessageDigest origDigest = Constants.newMessageDigest();
-		long origRemaining = originalEOF - 12;
+		final MessageDigest tailDigest = Constants.newMessageDigest();
+		long origRemaining = originalEOF;
 
 		packOut.seek(0);
-		if (packOut.read(buf, 0, 12) != 12)
-			throw new IOException("Cannot re-read pack header to fix count");
-		origDigest.update(buf, 0, 12);
+		bAvail = 0;
+		bOffset = 0;
+		fillFromFile(12);
+
+		{
+			final int origCnt = (int) Math.min(bAvail, origRemaining);
+			origDigest.update(buf, 0, origCnt);
+			origRemaining -= origCnt;
+			if (origRemaining == 0)
+				tailDigest.update(buf, origCnt, bAvail - origCnt);
+		}
+
 		NB.encodeInt32(buf, 8, entryCount);
 		packOut.seek(0);
 		packOut.write(buf, 0, 12);
+		packOut.seek(bAvail);
 
 		packDigest.reset();
-		packDigest.update(buf, 0, 12);
+		packDigest.update(buf, 0, bAvail);
 		for (;;) {
 			final int n = packOut.read(buf);
 			if (n < 0)
 				break;
-			if (origRemaining > 0) {
-				origDigest.update(buf, 0, (int) Math.min(n, origRemaining));
-				origRemaining -= n;
-			}
+			if (origRemaining != 0) {
+				final int origCnt = (int) Math.min(n, origRemaining);
+				origDigest.update(buf, 0, origCnt);
+				origRemaining -= origCnt;
+				if (origRemaining == 0)
+					tailDigest.update(buf, origCnt, n - origCnt);
+			} else
+				tailDigest.update(buf, 0, n);
+
 			packDigest.update(buf, 0, n);
 		}
 
-		if (!Arrays.equals(origDigest.digest(), packcsum))
+		if (!Arrays.equals(origDigest.digest(), origcsum)
+				|| !Arrays.equals(tailDigest.digest(), tailcsum))
 			throw new IOException("Pack corrupted while writing to filesystem");
 
 		packcsum = packDigest.digest();
-- 
1.6.0.272.g9ab4