Correctly skip over unrecognized optional dircache extensions

We didn't skip the correct number of bytes when we skipped over an
unrecognized but optional dircache extension.  We missed skipping
the 8 byte header that makes up the extension's name and length.

We also didn't include the skipped extension's payload as part of
our index checksum, resuting in a checksum failure when the index
was done reading.  So ensure we always scan through a skipped
section and include it in the checksum computation.

Add a test case for a currently unsupported index extension, 'ZZZZ',
to verify we can still read the DirCache object even though we
don't know what 'ZZZZ' is supposed to mean.

Bug: 301287
Change-Id: I4bdde94576fffe826d0782483fd98cab1ea628fa
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheCGitCompatabilityTest.java

@@ -52,6 +52,7 @@ import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
@@ -100,6 +101,34 @@ public class DirCacheCGitCompatabilityTest extends LocalDiskRepositoryTestCase {
 		}
 	}
 
+	public void testUnsupportedOptionalExtension() throws Exception {
+		final DirCache dc = new DirCache(pathOf("gitgit.index.ZZZZ"));
+		dc.read();
+		assertEquals(1, dc.getEntryCount());
+		assertEquals("A", dc.getEntry(0).getPathString());
+	}
+
+	public void testUnsupportedRequiredExtension() throws Exception {
+		final DirCache dc = new DirCache(pathOf("gitgit.index.aaaa"));
+		try {
+			dc.read();
+			fail("Cache loaded an unsupported extension");
+		} catch (CorruptObjectException err) {
+			assertEquals("DIRC extension 'aaaa'"
+					+ " not supported by this version.", err.getMessage());
+		}
+	}
+
+	public void testCorruptChecksumAtFooter() throws Exception {
+		final DirCache dc = new DirCache(pathOf("gitgit.index.badchecksum"));
+		try {
+			dc.read();
+			fail("Cache loaded despite corrupt checksum");
+		} catch (CorruptObjectException err) {
+			assertEquals("DIRC checksum mismatch", err.getMessage());
+		}
+	}
+
 	private static void assertEqual(final CGitIndexRecord c,
 			final DirCacheEntry j) {
 		assertNotNull(c);

org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2008-2010, Google Inc.
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
  * and other copyright owners as documented in the project's IP log.
  *
@@ -46,12 +46,14 @@ package org.eclipse.jgit.dircache;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
+import java.io.EOFException;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
-import java.nio.ByteBuffer;
+import java.io.UnsupportedEncodingException;
 import java.security.DigestOutputStream;
 import java.security.MessageDigest;
 import java.util.Arrays;
@@ -387,13 +389,20 @@ public class DirCache {
 				//
 				break;
 			}
+
 			in.reset();
+			md.update(hdr, 0, 8);
+			IO.skipFully(in, 8);
 
+			long sz = NB.decodeUInt32(hdr, 4);
 			switch (NB.decodeInt32(hdr, 0)) {
 			case EXT_TREE: {
-				final byte[] raw = new byte[NB.decodeInt32(hdr, 4)];
-				md.update(hdr, 0, 8);
-				IO.skipFully(in, 8);
+				if (Integer.MAX_VALUE < sz) {
+					throw new CorruptObjectException("DIRC extension "
+							+ formatExtensionName(hdr) + " is too large at "
+							+ sz + " bytes.");
+				}
+				final byte[] raw = new byte[(int) sz];
 				IO.readFully(in, raw, 0, raw.length);
 				md.update(raw, 0, raw.length);
 				tree = new DirCacheTree(raw, new MutableInteger(), null);
@@ -403,18 +412,18 @@ public class DirCache {
 				if (hdr[0] >= 'A' && hdr[0] <= 'Z') {
 					// The extension is optional and is here only as
 					// a performance optimization. Since we do not
-					// understand it, we can safely skip past it.
+					// understand it, we can safely skip past it, after
+					// we include its data in our checksum.
 					//
-					IO.skipFully(in, NB.decodeUInt32(hdr, 4));
+					skipOptionalExtension(in, md, hdr, sz);
 				} else {
 					// The extension is not an optimization and is
 					// _required_ to understand this index format.
 					// Since we did not trap it above we must abort.
 					//
-					throw new CorruptObjectException("DIRC extension '"
-							+ Constants.CHARSET.decode(
-									ByteBuffer.wrap(hdr, 0, 4)).toString()
-							+ "' not supported by this version.");
+					throw new CorruptObjectException("DIRC extension "
+							+ formatExtensionName(hdr)
+							+ " not supported by this version.");
 				}
 			}
 		}
@@ -425,6 +434,27 @@ public class DirCache {
 		}
 	}
 
+	private void skipOptionalExtension(final InputStream in,
+			final MessageDigest md, final byte[] hdr, long sz)
+			throws IOException {
+		final byte[] b = new byte[4096];
+		while (0 < sz) {
+			int n = in.read(b, 0, (int) Math.min(b.length, sz));
+			if (n < 0) {
+				throw new EOFException("Short read of optional DIRC extension "
+						+ formatExtensionName(hdr) + "; expected another " + sz
+						+ " bytes within the section.");
+			}
+			md.update(b, 0, n);
+			sz -= n;
+		}
+	}
+
+	private static String formatExtensionName(final byte[] hdr)
+			throws UnsupportedEncodingException {
+		return "'" + new String(hdr, 0, 4, "ISO-8859-1") + "'";
+	}
+
 	private static boolean is_DIRC(final byte[] hdr) {
 		if (hdr.length < SIG_DIRC.length)
 			return false;