git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 2/5] Implement CRC32 computation during PackWriter

From
Shawn O. Pearce <spearce@spearce.org>
Date
Mar 26, 2009, 01:21 UTC
Message-ID
<1238030515-31768-2-git-send-email-spearce@spearce.org>
In-Reply-To
<1238030515-31768-1-git-send-email-spearce@spearce.org>

To correctly create a v2 index file for a pack we must compute the CRC32 code for each object entry as they are written out, otherwise the CRC32 table will be full of 0's, and fail verification if the resulting pack were to be reused.

Reported-by: Daniel Cheng (aka SDiZ) <j16sdiz+freenet@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../tst/org/spearce/jgit/lib/PackWriterTest.java   |   17 +++---
 .../PackOutputStream.java}                         |   65 +++++++++++++-------
 .../src/org/spearce/jgit/lib/PackWriter.java       |   16 ++---
 3 files changed, 57 insertions(+), 41 deletions(-)
 rename org.spearce.jgit/src/org/spearce/jgit/{util/CountingOutputStream.java => lib/PackOutputStream.java} (61%)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
index f7139fc..46616e3 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/PackWriterTest.java
@@ -56,7 +56,6 @@
 import org.spearce.jgit.revwalk.RevObject;
 import org.spearce.jgit.revwalk.RevWalk;
 import org.spearce.jgit.transport.IndexPack;
-import org.spearce.jgit.util.CountingOutputStream;
 import org.spearce.jgit.util.JGitTestUtil;
 
 public class PackWriterTest extends RepositoryTestCase {
@@ -71,7 +70,7 @@
 
 	private ByteArrayOutputStream os;
 
-	private CountingOutputStream cos;
+	private PackOutputStream cos;
 
 	private File packBase;
 
@@ -84,7 +83,7 @@
 	public void setUp() throws Exception {
 		super.setUp();
 		os = new ByteArrayOutputStream();
-		cos = new CountingOutputStream(os);
+		cos = new PackOutputStream(os);
 		packBase = new File(trash, "tmp_pack");
 		packFile = new File(trash, "tmp_pack.pack");
 		indexFile = new File(trash, "tmp_pack.idx");
@@ -308,11 +307,11 @@ public void testWritePack4ThinPack() throws IOException {
 	 */
 	public void testWritePack2SizeDeltasVsNoDeltas() throws Exception {
 		testWritePack2();
-		final long sizePack2NoDeltas = cos.getCount();
+		final long sizePack2NoDeltas = cos.length();
 		tearDown();
 		setUp();
 		testWritePack2DeltasReuseRefs();
-		final long sizePack2DeltasRefs = cos.getCount();
+		final long sizePack2DeltasRefs = cos.length();
 
 		assertTrue(sizePack2NoDeltas > sizePack2DeltasRefs);
 	}
@@ -327,11 +326,11 @@ public void testWritePack2SizeDeltasVsNoDeltas() throws Exception {
 	 */
 	public void testWritePack2SizeOffsetsVsRefs() throws Exception {
 		testWritePack2DeltasReuseRefs();
-		final long sizePack2DeltasRefs = cos.getCount();
+		final long sizePack2DeltasRefs = cos.length();
 		tearDown();
 		setUp();
 		testWritePack2DeltasReuseOffsets();
-		final long sizePack2DeltasOffsets = cos.getCount();
+		final long sizePack2DeltasOffsets = cos.length();
 
 		assertTrue(sizePack2DeltasRefs > sizePack2DeltasOffsets);
 	}
@@ -345,11 +344,11 @@ public void testWritePack2SizeOffsetsVsRefs() throws Exception {
 	 */
 	public void testWritePack4SizeThinVsNoThin() throws Exception {
 		testWritePack4();
-		final long sizePack4 = cos.getCount();
+		final long sizePack4 = cos.length();
 		tearDown();
 		setUp();
 		testWritePack4ThinPack();
-		final long sizePack4Thin = cos.getCount();
+		final long sizePack4Thin = cos.length();
 
 		assertTrue(sizePack4 > sizePack4Thin);
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/CountingOutputStream.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackOutputStream.java
similarity index 61%
rename from org.spearce.jgit/src/org/spearce/jgit/util/CountingOutputStream.java
rename to org.spearce.jgit/src/org/spearce/jgit/lib/PackOutputStream.java
index 5f333f5..403b892 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/CountingOutputStream.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackOutputStream.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2009, Google Inc.
  * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  *
  * All rights reserved.
@@ -35,46 +36,66 @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-package org.spearce.jgit.util;
+package org.spearce.jgit.lib;
 
-import java.io.FilterOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.security.MessageDigest;
+import java.util.zip.CRC32;
+
+/** Custom output stream to support {@link PackWriter}. */
+final class PackOutputStream extends OutputStream {
+	private final OutputStream out;
+
+	private final CRC32 crc = new CRC32();
+
+	private final MessageDigest md = Constants.newMessageDigest();
 
-/**
- * Counting output stream decoration. Counts bytes written to stream.
- */
-public class CountingOutputStream extends FilterOutputStream {
 	private long count;
 
-	/**
-	 * Create counting stream being decorated to provided real output stream.
-	 *
-	 * @param out
-	 *            output stream where data should be written
-	 */
-	public CountingOutputStream(OutputStream out) {
-		super(out);
+	PackOutputStream(final OutputStream out) {
+		this.out = out;
 	}
 
 	@Override
-	public void write(int b) throws IOException {
+	public void write(final int b) throws IOException {
 		out.write(b);
+		crc.update(b);
+		md.update((byte) b);
 		count++;
 	}
 
 	@Override
-	public void write(byte[] b, int off, int len) throws IOException {
+	public void write(final byte[] b, final int off, final int len)
+			throws IOException {
 		out.write(b, off, len);
+		crc.update(b, off, len);
+		md.update(b, off, len);
 		count += len;
 	}
 
-	/**
-	 * Return number of already written bytes.
-	 *
-	 * @return number of written bytes since stream start.
-	 */
-	public long getCount() {
+	@Override
+	public void flush() throws IOException {
+		out.flush();
+	}
+
+	/** @return total number of bytes written since stream start. */
+	long length() {
 		return count;
 	}
+
+	/** @return obtain the current CRC32 register. */
+	int getCRC32() {
+		return (int) crc.getValue();
+	}
+
+	/** Reinitialize the CRC32 register for a new region. */
+	void resetCRC32() {
+		crc.reset();
+	}
+
+	/** @return obtain the current SHA-1 digest. */
+	byte[] getDigest() {
+		return md.digest();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index 601ce71..cfec35c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -40,7 +40,6 @@
 import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.security.DigestOutputStream;
 import java.security.MessageDigest;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -57,7 +56,6 @@
 import org.spearce.jgit.revwalk.RevObject;
 import org.spearce.jgit.revwalk.RevSort;
 import org.spearce.jgit.transport.PackedObjectInfo;
-import org.spearce.jgit.util.CountingOutputStream;
 import org.spearce.jgit.util.NB;
 
 /**
@@ -166,9 +164,7 @@
 
 	private final Repository db;
 
-	private DigestOutputStream out;
-
-	private CountingOutputStream countingOut;
+	private PackOutputStream out;
 
 	private final Deflater deflater;
 
@@ -563,8 +559,7 @@ public void writePack(OutputStream packStream) throws IOException {
 
 		if (!(packStream instanceof BufferedOutputStream))
 			packStream = new BufferedOutputStream(packStream);
-		countingOut = new CountingOutputStream(packStream);
-		out = new DigestOutputStream(countingOut, Constants.newMessageDigest());
+		out = new PackOutputStream(packStream);
 
 		writeMonitor.beginTask(WRITING_OBJECTS_PROGRESS, getObjectsNumber());
 		writeHeader();
@@ -687,11 +682,13 @@ private void writeObject(final ObjectToPack otp) throws IOException {
 
 		assert !otp.isWritten();
 
-		otp.setOffset(countingOut.getCount());
+		out.resetCRC32();
+		otp.setOffset(out.length());
 		if (otp.isDeltaRepresentation())
 			writeDeltaObject(otp);
 		else
 			writeWholeObject(otp);
+		otp.setCRC(out.getCRC32());
 
 		writeMonitor.update(1);
 	}
@@ -753,8 +750,7 @@ private void writeObjectHeader(final int objectType, long dataLength)
 	}
 
 	private void writeChecksum() throws IOException {
-		out.on(false);
-		packcsum = out.getMessageDigest().digest();
+		packcsum = out.getDigest();
 		out.write(packcsum);
 	}
 

-- 
1.6.2.1.471.g682837
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 2 of 11 in “Remove dead/unused reset method from CountingOutputStream”
  1. 1/5 Remove dead/unused reset method from CountingOutputStreamShawn O. Pearce, Mar 26, 2009
  2. 2/5 Implement CRC32 computation during PackWriterShawn O. Pearce, Mar 26, 2009
  3. 3/5 Test case for pack index CRC32 when written by PackWriterShawn O. Pearce, Mar 26, 2009
  4. 4/5 Write the pack header in one shotShawn O. Pearce, Mar 26, 2009
  5. 5/5 Use Deflater directly in PackWriterShawn O. Pearce, Mar 26, 2009
  6. Daniel ChengMar 26, 2009
  7. Daniel ChengMar 26, 2009
  8. Robin RosenbergMar 27, 2009
  9. Shawn O. PearceMar 27, 2009
  10. Shawn O. PearceMar 27, 2009
  11. Robin RosenbergMar 27, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.