From ab3c68c512035a87071da3269ce8810b55b52b3f Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 6 Jul 2010 17:29:27 -0700 Subject: [PATCH] amend commit: Support large loose objects as streams We need to validate the stream state after the InflaterInputStream thinks the stream is done. Git expects a higher level of service from the Inflater than the InflaterInputStream usually gives, we need to ensure the embedded CRC is valid, and that there isn't trailing garbage at the end of the file. Change-Id: I1c9642a82dbd76b69e607dceccf8b85dc869a3c1 Signed-off-by: Shawn O. Pearce --- .../jgit/storage/file/UnpackedObjectTest.java | 104 ++++++++++++++++++ .../jgit/storage/file/UnpackedObject.java | 76 ++++++++++--- 2 files changed, 167 insertions(+), 13 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/UnpackedObjectTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/UnpackedObjectTest.java index d77a96220..25dfe4c23 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/UnpackedObjectTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/UnpackedObjectTest.java @@ -227,6 +227,42 @@ public class UnpackedObjectTest extends LocalDiskRepositoryTestCase { } } + public void testStandardFormat_SmallObject_TruncatedZLibStream() + throws Exception { + ObjectId id = ObjectId.zeroId(); + byte[] data = rng.nextBytes(300); + + try { + byte[] gz = compressStandardFormat(Constants.OBJ_BLOB, data); + byte[] tr = new byte[gz.length - 1]; + System.arraycopy(gz, 0, tr, 0, tr.length); + UnpackedObject.open(new ByteArrayInputStream(tr), path(id), id, wc); + fail("Did not throw CorruptObjectException"); + } catch (CorruptObjectException coe) { + assertEquals(MessageFormat.format(JGitText.get().objectIsCorrupt, + id.name(), JGitText.get().corruptObjectBadStream), coe + .getMessage()); + } + } + + public void testStandardFormat_SmallObject_TrailingGarbage() + throws Exception { + ObjectId id = ObjectId.zeroId(); + byte[] data = rng.nextBytes(300); + + try { + byte[] gz = compressStandardFormat(Constants.OBJ_BLOB, data); + byte[] tr = new byte[gz.length + 1]; + System.arraycopy(gz, 0, tr, 0, gz.length); + UnpackedObject.open(new ByteArrayInputStream(tr), path(id), id, wc); + fail("Did not throw CorruptObjectException"); + } catch (CorruptObjectException coe) { + assertEquals(MessageFormat.format(JGitText.get().objectIsCorrupt, + id.name(), JGitText.get().corruptObjectBadStream), coe + .getMessage()); + } + } + public void testStandardFormat_LargeObject_CorruptZLibStream() throws Exception { final int type = Constants.OBJ_BLOB; @@ -264,6 +300,74 @@ public class UnpackedObjectTest extends LocalDiskRepositoryTestCase { } } + public void testStandardFormat_LargeObject_TruncatedZLibStream() + throws Exception { + final int type = Constants.OBJ_BLOB; + byte[] data = rng.nextBytes(ObjectLoader.STREAM_THRESHOLD + 5); + ObjectId id = new ObjectInserter.Formatter().idFor(type, data); + byte[] gz = compressStandardFormat(type, data); + byte[] tr = new byte[gz.length - 1]; + System.arraycopy(gz, 0, tr, 0, tr.length); + + write(id, tr); + + ObjectLoader ol; + { + FileInputStream fs = new FileInputStream(path(id)); + try { + ol = UnpackedObject.open(fs, path(id), id, wc); + } finally { + fs.close(); + } + } + + byte[] tmp = new byte[data.length]; + InputStream in = ol.openStream(); + IO.readFully(in, tmp, 0, tmp.length); + try { + in.close(); + fail("close did not throw CorruptObjectException"); + } catch (CorruptObjectException coe) { + assertEquals(MessageFormat.format(JGitText.get().objectIsCorrupt, + id.name(), JGitText.get().corruptObjectBadStream), coe + .getMessage()); + } + } + + public void testStandardFormat_LargeObject_TrailingGarbage() + throws Exception { + final int type = Constants.OBJ_BLOB; + byte[] data = rng.nextBytes(ObjectLoader.STREAM_THRESHOLD + 5); + ObjectId id = new ObjectInserter.Formatter().idFor(type, data); + byte[] gz = compressStandardFormat(type, data); + byte[] tr = new byte[gz.length + 1]; + System.arraycopy(gz, 0, tr, 0, gz.length); + + write(id, tr); + + ObjectLoader ol; + { + FileInputStream fs = new FileInputStream(path(id)); + try { + ol = UnpackedObject.open(fs, path(id), id, wc); + } finally { + fs.close(); + } + } + + byte[] tmp = new byte[data.length]; + InputStream in = ol.openStream(); + IO.readFully(in, tmp, 0, tmp.length); + try { + in.close(); + fail("close did not throw CorruptObjectException"); + } catch (CorruptObjectException coe) { + assertEquals(MessageFormat.format(JGitText.get().objectIsCorrupt, + id.name(), JGitText.get().corruptObjectBadStream), coe + .getMessage()); + } + } + public void testPackFormat_SmallObject() throws Exception { final int type = Constants.OBJ_BLOB; byte[] data = rng.nextBytes(300); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UnpackedObject.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UnpackedObject.java index 55ee3b78b..0325c5711 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UnpackedObject.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UnpackedObject.java @@ -52,6 +52,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.util.zip.DataFormatException; import java.util.zip.Inflater; import java.util.zip.InflaterInputStream; import java.util.zip.ZipException; @@ -108,8 +109,9 @@ public class UnpackedObject { if (isStandardFormat(hdr)) { in.reset(); - in = inflate(in, wc); - int avail = readSome(in, hdr, 0, 64); + Inflater inf = wc.inflater(); + InputStream zIn = inflate(in, inf); + int avail = readSome(zIn, hdr, 0, 64); if (avail < 5) throw new CorruptObjectException(id, JGitText.get().corruptObjectNoHeader); @@ -130,7 +132,8 @@ public class UnpackedObject { int n = avail - p.value; if (n > 0) System.arraycopy(hdr, p.value, data, 0, n); - IO.readFully(in, data, n, data.length - n); + IO.readFully(zIn, data, n, data.length - n); + checkValidEndOfStream(in, inf, id, hdr); return new ObjectLoader.SmallObject(type, data); } return new LargeObject(type, size, path, id, wc.db); @@ -165,9 +168,11 @@ public class UnpackedObject { if (size < wc.getStreamFileThreshold() || path == null) { in.reset(); IO.skipFully(in, p); - in = inflate(in, wc); + Inflater inf = wc.inflater(); + InputStream zIn = inflate(in, inf); byte[] data = new byte[(int) size]; - IO.readFully(in, data, 0, data.length); + IO.readFully(zIn, data, 0, data.length); + checkValidEndOfStream(in, inf, id, hdr); return new ObjectLoader.SmallObject(type, data); } return new LargeObject(type, size, path, id, wc.db); @@ -178,6 +183,40 @@ public class UnpackedObject { } } + private static void checkValidEndOfStream(InputStream in, Inflater inf, + AnyObjectId id, final byte[] buf) throws IOException, + CorruptObjectException { + for (;;) { + int r; + try { + r = inf.inflate(buf); + } catch (DataFormatException e) { + throw new CorruptObjectException(id, + JGitText.get().corruptObjectBadStream); + } + if (r != 0) + throw new CorruptObjectException(id, + JGitText.get().corruptObjectIncorrectLength); + + if (inf.finished()) { + if (inf.getRemaining() != 0 || in.read() != -1) + throw new CorruptObjectException(id, + JGitText.get().corruptObjectBadStream); + break; + } + + if (!inf.needsInput()) + throw new CorruptObjectException(id, + JGitText.get().corruptObjectBadStream); + + r = in.read(buf); + if (r <= 0) + throw new CorruptObjectException(id, + JGitText.get().corruptObjectBadStream); + inf.setInput(buf, 0, r); + } + } + private static boolean isStandardFormat(final byte[] hdr) { // Try to determine if this is a standard format loose object or // a pack style loose object. The standard format is completely @@ -189,13 +228,19 @@ public class UnpackedObject { return fb == 0x78 && (((fb << 8) | hdr[1] & 0xff) % 31) == 0; } - private static InputStream inflate(InputStream in, final ObjectId id) { + private static InputStream inflate(final InputStream in, final long size, + final ObjectId id) { final Inflater inf = InflaterCache.get(); return new InflaterInputStream(in, inf) { + private long remaining = size; + @Override public int read(byte[] b, int off, int cnt) throws IOException { try { - return super.read(b, off, cnt); + int r = super.read(b, off, cnt); + if (r > 0) + remaining -= r; + return r; } catch (ZipException badStream) { throw new CorruptObjectException(id, JGitText.get().corruptObjectBadStream); @@ -204,14 +249,19 @@ public class UnpackedObject { @Override public void close() throws IOException { - super.close(); - InflaterCache.release(inf); + try { + if (remaining <= 0) + checkValidEndOfStream(in, inf, id, new byte[64]); + super.close(); + } finally { + InflaterCache.release(inf); + } } }; } - private static InputStream inflate(InputStream in, WindowCursor wc) { - return new InflaterInputStream(in, wc.inflater(), BUFFER_SIZE); + private static InflaterInputStream inflate(InputStream in, Inflater inf) { + return new InflaterInputStream(in, inf, BUFFER_SIZE); } private static BufferedInputStream buffer(InputStream in) { @@ -293,7 +343,7 @@ public class UnpackedObject { if (isStandardFormat(hdr)) { in.reset(); - in = buffer(inflate(in, id)); + in = buffer(inflate(in, size, id)); while (0 < in.read()) continue; } else { @@ -305,7 +355,7 @@ public class UnpackedObject { in.reset(); IO.skipFully(in, p); - in = buffer(inflate(in, id)); + in = buffer(inflate(in, size, id)); } ok = true;