From 06ee913c8d5cd1afaa3c5573430f07e5655e1edf Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 16 Apr 2010 15:55:18 -0700 Subject: [PATCH] IndexPack: Correct thin pack fix using less than 20 bytes If we need to append less than 20 bytes in order to fix a thin pack and make it complete, we need to set the length of our file back to the actual number of bytes used because the original SHA-1 footer was not completely overwritten. That extra data will confuse the header and footer fixup logic when it tries to read to the end of the file. This isn't a very common case to occur, which is why we've never seen it before. Getting a delta that requires a whole object which uses less than 20 bytes in pack representation is really hard. Generally a delta generator won't make these, because the delta would be bigger than simply deflating the whole object. I only managed to do this with a hand-crafted pack file where a 1 byte delta was pointed to a 1 byte whole object. Normally we try really hard to avoid truncating, because its typically not safe across network filesystems. But the odds of this occurring are very low. This truncation is done on a file we have open for writing, will append more content onto, and is a temporary file that we won't move into position for others to see until we've validated its SHA-1 is sane. I don't think the truncate on NFS issue is something we need to worry about here. Change-Id: I102b9637dfd048dc833c050890d142f43c1e75ae Signed-off-by: Shawn O. Pearce --- .../eclipse/jgit/transport/IndexPackTest.java | 59 +++++++++++++++++++ .../org/eclipse/jgit/transport/IndexPack.java | 8 +++ 2 files changed, 67 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/IndexPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/IndexPackTest.java index 78f4393c3..e18f741ac 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/IndexPackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/IndexPackTest.java @@ -46,16 +46,25 @@ package org.eclipse.jgit.transport; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.security.MessageDigest; +import java.util.zip.Deflater; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PackFile; import org.eclipse.jgit.lib.RepositoryTestCase; import org.eclipse.jgit.lib.TextProgressMonitor; +import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.util.JGitTestUtil; +import org.eclipse.jgit.util.NB; +import org.eclipse.jgit.util.TemporaryBuffer; /** * Test indexing of git packs. A pack is read from a stream, copied @@ -120,4 +129,54 @@ public class IndexPackTest extends RepositoryTestCase { is.close(); } } + + public void testTinyThinPack() throws Exception { + TestRepository d = new TestRepository(db); + RevBlob a = d.blob("a"); + + TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024); + + packHeader(pack, 1); + + pack.write((Constants.OBJ_REF_DELTA) << 4 | 4); + a.copyRawTo(pack); + deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' }); + + digest(pack); + + final byte[] raw = pack.toByteArray(); + IndexPack ip = IndexPack.create(db, new ByteArrayInputStream(raw)); + ip.setFixThin(true); + ip.index(NullProgressMonitor.INSTANCE); + ip.renameAndOpenPack(); + } + + private void packHeader(TemporaryBuffer.Heap tinyPack, int cnt) + throws IOException { + final byte[] hdr = new byte[8]; + NB.encodeInt32(hdr, 0, 2); + NB.encodeInt32(hdr, 4, cnt); + + tinyPack.write(Constants.PACK_SIGNATURE); + tinyPack.write(hdr, 0, 8); + } + + private void deflate(TemporaryBuffer.Heap tinyPack, final byte[] content) + throws IOException { + final Deflater deflater = new Deflater(); + final byte[] buf = new byte[128]; + deflater.setInput(content, 0, content.length); + deflater.finish(); + do { + final int n = deflater.deflate(buf, 0, buf.length); + if (n > 0) + tinyPack.write(buf, 0, n); + } while (!deflater.finished()); + } + + private void digest(TemporaryBuffer.Heap buf) throws IOException { + MessageDigest md = Constants.newMessageDigest(); + md.update(buf.toByteArray()); + buf.write(md.digest()); + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java index 23faa42a2..29f200c52 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java @@ -601,6 +601,14 @@ public class IndexPack { throw new MissingObjectException(base, "delta base"); } + if (end - originalEOF < 20) { + // Ugly corner case; if what we appended on to complete deltas + // doesn't completely cover the SHA-1 we have to truncate off + // we need to shorten the file, otherwise we will include part + // of the old footer as object content. + packOut.setLength(end); + } + fixHeaderFooter(packcsum, packDigest.digest()); }