From 43ef5dabf12fee8877c6d8fa07b26be766dca1ee Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 20 Dec 2017 12:36:03 -0500
Subject: PackInserter: Ensure objects are written at the end of the pack

When interleaving reads and writes from an unflushed pack, we forgot to
reset the file pointer back to the end of the file before writing more
new objects. This had at least two unfortunate effects:
  * The pack data was potentially corrupt, since we could overwrite
    previous portions of the file willy-nilly.
  * The CountingOutputStream would report more bytes read than the size
    of the file, which stored the wrong PackedObjectInfo, which would
    cause EOFs during reading.

We already had a test in PackInserterTest which was supposed to catch
bugs like this, by interleaving reads and writes. Unfortunately, it
didn't catch the bug, since as an implementation detail we always read a
full buffer's worth of data from the file when inflating during
readback. If the size of the file was less than the offset of the object
we were reading back plus one buffer (8192 bytes), we would completely
accidentally end up back in the right place in the file.

So, add another test for this case where we read back a small object
positioned before a large object. Before the fix, this test exhibited
exactly the "Unexpected EOF" error reported at crbug.com/gerrit/7668.

Change-Id: I74f08f3d5d9046781d59e5bd7c84916ff8225c3b
---
 .../internal/storage/file/PackInserterTest.java    | 52 ++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

(limited to 'org.eclipse.jgit.test/tst/org/eclipse/jgit')

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackInserterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackInserterTest.java
index 17f04c8548..8596f74f81 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackInserterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackInserterTest.java
@@ -67,6 +67,7 @@ import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Random;
 import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -440,6 +441,53 @@ public class PackInserterTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void readBackSmallObjectBeforeLargeObject() throws Exception {
+		WindowCacheConfig wcc = new WindowCacheConfig();
+		wcc.setStreamFileThreshold(1024);
+		wcc.install();
+
+		ObjectId blobId1;
+		ObjectId blobId2;
+		ObjectId largeId;
+		byte[] blob1 = Constants.encode("blob1");
+		byte[] blob2 = Constants.encode("blob2");
+		byte[] largeBlob = newLargeBlob();
+		try (PackInserter ins = newInserter()) {
+			assertThat(blob1.length, lessThan(ins.getBufferSize()));
+			assertThat(largeBlob.length, greaterThan(ins.getBufferSize()));
+
+			blobId1 = ins.insert(OBJ_BLOB, blob1);
+			largeId = ins.insert(OBJ_BLOB, largeBlob);
+
+			try (ObjectReader reader = ins.newReader()) {
+				// A previous bug did not reset the file pointer to EOF after reading
+				// back. We need to seek to something further back than a full buffer,
+				// since the read-back code eagerly reads a full buffer's worth of data
+				// from the file to pass to the inflater. If we seeked back just a small
+				// amount, this step would consume the rest of the file, so the file
+				// pointer would coincidentally end up back at EOF, hiding the bug.
+				assertBlob(reader, blobId1, blob1);
+			}
+
+			blobId2 = ins.insert(OBJ_BLOB, blob2);
+
+			try (ObjectReader reader = ins.newReader()) {
+				assertBlob(reader, blobId1, blob1);
+				assertBlob(reader, blobId2, blob2);
+				assertBlob(reader, largeId, largeBlob);
+			}
+
+			ins.flush();
+		}
+
+		try (ObjectReader reader = db.newObjectReader()) {
+				assertBlob(reader, blobId1, blob1);
+				assertBlob(reader, blobId2, blob2);
+				assertBlob(reader, largeId, largeBlob);
+		}
+	}
+
 	private List<PackFile> listPacks() throws Exception {
 		List<PackFile> fromOpenDb = listPacks(db);
 		List<PackFile> reopened;
@@ -470,9 +518,7 @@ public class PackInserterTest extends RepositoryTestCase {
 
 	private static byte[] newLargeBlob() {
 		byte[] blob = new byte[10240];
-		for (int i = 0; i < blob.length; i++) {
-			blob[i] = (byte) ('0' + (i % 10));
-		}
+		new Random(0).nextBytes(blob);
 		return blob;
 	}
 
-- 
cgit v1.2.3