From 784b24dde151a967229c2f8c2f08b0827709bd4d Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 2 Feb 2010 09:09:26 -0800 Subject: [PATCH] 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 --- .../jgit/test/resources/gitgit.index.ZZZZ | Bin 0 -> 187 bytes .../jgit/test/resources/gitgit.index.aaaa | Bin 0 -> 187 bytes .../test/resources/gitgit.index.badchecksum | Bin 0 -> 187 bytes .../DirCacheCGitCompatabilityTest.java | 29 ++++++++++ .../org/eclipse/jgit/dircache/DirCache.java | 54 ++++++++++++++---- 5 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.ZZZZ create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.aaaa create mode 100644 org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/gitgit.index.badchecksum 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 index 0000000000000000000000000000000000000000..c931c06fd60d981bcc612efb7da51ff5fc7cb51e GIT binary patch literal 187 zcmZ?q402{*U|<4bM(+&2(?FU5Ml&${V_?#3^J8FWT*AN@_=$l*xC1Dy@-9(Ia^ZKO zO;uN1-gzV#cD8J>e!{@$$PfhtK>1)t215e_GZPau>N~%{liQMP{gr#1Tf99#zdPr8 zX+9`tn*BitSiiZOaA@lz3B4U#NuO%nnmO*y1CdSR;^p# RsF1?$Ej3eU)}}}0jsWsiL1O>_ literal 0 HcmV?d00001 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 index 0000000000000000000000000000000000000000..3f13d61216593625ba190cde54c6c59c7d3c8145 GIT binary patch literal 187 zcmZ?q402{*U|<4bM(+&2(?FU5Ml&${V_?#3^J8FWT*AN@_=$l*xC1Dy@-9(Ia^ZKO zO;uN1-gzV#cD8J>e!{@$$dCvGK>1)t215e_GZPau>N~%{liQMP{gr#1Tf99#zdPr8 zX+9`tn*BitSiiZOaA@lz3B4U#NuO%nnmO*HqKdHGVkUG St8eRb#OI2&qe!{@$$PfhtK>1)t215e_GZPau>N~%{liQMP{gr#1Tf99#zdPr8 zX+9`tn*BitSiiZOaA@lz3B4U#NuO%nnmO*HqKdHGVkUG St8eRb#1q9@Qa&EqvJ(I#&P7`Q literal 0 HcmV?d00001 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 688fa7359..fa5fea863 100644 --- 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.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 void testTreeWalk_LsFiles() throws Exception { } } + 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 ec0df2337..189787dd9 100644 --- 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 * and other copyright owners as documented in the project's IP log. * @@ -46,12 +46,14 @@ 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 @@ private void readFrom(final FileInputStream inStream) throws IOException, // break; } - in.reset(); + 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 @@ private void readFrom(final FileInputStream inStream) throws IOException, 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 @@ private void readFrom(final FileInputStream inStream) throws IOException, } } + 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;