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>

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.ZZZZ b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.ZZZZ
new file mode 100644 (file)
index 0000000..c931c06
Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.ZZZZ differ

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.aaaa b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.aaaa
new file mode 100644 (file)
index 0000000..3f13d61
Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.aaaa differ

diff --git a/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.badchecksum b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.badchecksum
new file mode 100644 (file)
index 0000000..ab0382d
Binary files /dev/null and b/org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.badchecksum differ

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheCGitCompatabilityTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheCGitCompatabilityTest.java
index 688fa73598587d444a521a4c273cada4b176901d..fa5fea8633fc6092453e1e32485f9051926329af 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheCGitCompatabilityTest.java
+++ b/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);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java
index ec0df23374fbca26b3d2b28f62a919e6166163e2..189787dd9e0166a8517dc81dc31025050731d00e 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java
+++ b/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;