Merge changes I8c60d970,I09bdd4b8,I87ff3933

* changes:
  Pack: open reverse index from file if present
  PackReverseIndex: open file if present otherwise compute
  PackReverseIndex: verify checksums

7 files changed, 104 insertions, 14 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index bf8a1ef5db..c73d85f078 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -223,6 +223,7 @@ corruptObjectTruncatedInMode=truncated in mode
 corruptObjectTruncatedInName=truncated in name
 corruptObjectTruncatedInObjectId=truncated in object id
 corruptObjectZeroId=entry points to null SHA-1
+corruptReverseIndexChecksumIncorrect=Reverse index checksum incorrect: written as {0} but digest was {1}
 corruptUseCnt=close() called when useCnt is already zero for {0}
 couldNotGetAdvertisedRef=Remote {0} did not advertise Ref for branch {1}. This Ref may not exist in the remote or may be hidden by permission settings.
 couldNotGetRepoStatistics=Could not get repository statistics
@@ -566,7 +567,7 @@ openingConnection=Opening connection
 operationCanceled=Operation {0} was canceled
 outputHasAlreadyBeenStarted=Output has already been started.
 overflowedReftableBlock=Overflowed reftable block
-packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: .pack has {1} whilst .idx has {2}
+packChecksumMismatch=Pack checksum mismatch detected for pack file {0}: {1} has {2} whilst {3} has {4}
 packCorruptedWhileWritingToFilesystem=Pack corrupted while writing to filesystem
 packedRefsHandleIsStale=packed-refs handle is stale, {0}. retry
 packetSizeMustBeAtLeast=packet size {0} must be >= {1}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index a032d24c71..91d53220a9 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -252,6 +252,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String corruptObjectTruncatedInName;
 	/***/ public String corruptObjectTruncatedInObjectId;
 	/***/ public String corruptObjectZeroId;
+	/***/ public String corruptReverseIndexChecksumIncorrect;
 	/***/ public String corruptPack;
 	/***/ public String corruptUseCnt;
 	/***/ public String couldNotFindTabInLine;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
index 782cbea054..2b5586a2ce 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
@@ -14,6 +14,7 @@ package org.eclipse.jgit.internal.storage.file;
 
 import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
 import static org.eclipse.jgit.internal.storage.pack.PackExt.KEEP;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.REVERSE_INDEX;
 
 import java.io.EOFException;
 import java.io.File;
@@ -50,12 +51,14 @@ import org.eclipse.jgit.errors.UnsupportedPackIndexVersionException;
 import org.eclipse.jgit.errors.UnsupportedPackVersionException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.pack.BinaryDelta;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.internal.storage.pack.PackOutputStream;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.util.Hex;
 import org.eclipse.jgit.util.LongList;
 import org.eclipse.jgit.util.NB;
 import org.eclipse.jgit.util.RawParseUtils;
@@ -177,10 +180,10 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 							throw new PackMismatchException(MessageFormat
 									.format(JGitText.get().packChecksumMismatch,
 											packFile.getPath(),
-											ObjectId.fromRaw(packChecksum)
-													.name(),
-											ObjectId.fromRaw(idx.packChecksum)
-													.name()));
+											PackExt.PACK.getExtension(),
+											Hex.toHexString(packChecksum),
+											PackExt.INDEX.getExtension(),
+											Hex.toHexString(idx.packChecksum)));
 						}
 						loadedIdx = idx;
 					} catch (InterruptedIOException e) {
@@ -765,11 +768,11 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 		fd.seek(length - 20);
 		fd.readFully(buf, 0, 20);
 		if (!Arrays.equals(buf, packChecksum)) {
-			throw new PackMismatchException(MessageFormat.format(
-					JGitText.get().packChecksumMismatch,
-					getPackFile(),
-					ObjectId.fromRaw(buf).name(),
-					ObjectId.fromRaw(idx.packChecksum).name()));
+			throw new PackMismatchException(
+					MessageFormat.format(JGitText.get().packChecksumMismatch,
+							getPackFile(), PackExt.PACK.getExtension(),
+							Hex.toHexString(buf), PackExt.INDEX.getExtension(),
+							Hex.toHexString(idx.packChecksum)));
 		}
 	}
 
@@ -1148,8 +1151,15 @@ public class Pack implements Iterable<PackIndex.MutableEntry> {
 	}
 
 	private synchronized PackReverseIndex getReverseIdx() throws IOException {
-		if (reverseIdx == null)
-			reverseIdx = PackReverseIndexFactory.computeFromIndex(idx());
+		if (invalid) {
+			throw new PackInvalidException(packFile, invalidatingCause);
+		}
+		if (reverseIdx == null) {
+			PackFile reverseIndexFile = packFile.create(REVERSE_INDEX);
+			reverseIdx = PackReverseIndexFactory.openOrCompute(reverseIndexFile,
+					getObjectCount(), () -> getIndex());
+			reverseIdx.verifyPackChecksum(getPackFile().getPath());
+		}
 		return reverseIdx;
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java
index 74ea89025a..ef9753cd79 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java
@@ -11,6 +11,7 @@
 package org.eclipse.jgit.internal.storage.file;
 
 import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.lib.ObjectId;
 
 /**
@@ -35,6 +36,17 @@ public interface PackReverseIndex {
 	int VERSION_1 = 1;
 
 	/**
+	 * Verify that the pack checksum found in the reverse index matches that
+	 * from the pack file.
+	 *
+	 * @param packFilePath
+	 *            the path to display in event of a mismatch
+	 * @throws PackMismatchException
+	 *             if the checksums do not match
+	 */
+	void verifyPackChecksum(String packFilePath) throws PackMismatchException;
+
+	/**
 	 * Search for object id with the specified start offset in this pack
 	 * (reverse) index.
 	 *
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java
index d6eaa965d4..0b487a2819 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java
@@ -12,6 +12,7 @@ package org.eclipse.jgit.internal.storage.file;
 import java.text.MessageFormat;
 
 import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
 import org.eclipse.jgit.lib.ObjectId;
@@ -144,6 +145,12 @@ final class PackReverseIndexComputed implements PackReverseIndex {
 	}
 
 	@Override
+	public void verifyPackChecksum(String packFilePath)
+			throws PackMismatchException {
+		// There is no file with a checksum.
+	}
+
+	@Override
 	public ObjectId findObject(long offset) {
 		final int ith = binarySearch(offset);
 		if (ith < 0) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexFactory.java
index b16da5ae83..32830c3cf0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexFactory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexFactory.java
@@ -14,6 +14,8 @@ import static org.eclipse.jgit.internal.storage.file.PackReverseIndex.MAGIC;
 import static org.eclipse.jgit.internal.storage.file.PackReverseIndex.VERSION_1;
 
 import java.io.DataInput;
+import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.security.DigestInputStream;
@@ -23,12 +25,43 @@ import java.util.Arrays;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.util.IO;
+import org.eclipse.jgit.util.io.SilentFileInputStream;
 
 /**
  * Factory for creating instances of {@link PackReverseIndex}.
  */
 public final class PackReverseIndexFactory {
 	/**
+	 * Create an in-memory pack reverse index by reading it from the given file
+	 * if the file exists, or computing it from the given pack index if the file
+	 * doesn't exist.
+	 *
+	 * @param idxFile
+	 *            the file to read the pack file from, if it exists
+	 * @param objectCount
+	 *            the number of objects in the corresponding pack
+	 * @param packIndexSupplier
+	 *            a function to lazily get the corresponding forward index
+	 * @return the reverse index instance
+	 * @throws IOException
+	 *             if reading from the file fails
+	 */
+	static PackReverseIndex openOrCompute(File idxFile, long objectCount,
+			PackBitmapIndex.SupplierWithIOException<PackIndex> packIndexSupplier)
+			throws IOException {
+		try (SilentFileInputStream fd = new SilentFileInputStream(idxFile)) {
+			return readFromFile(fd, objectCount, packIndexSupplier);
+		} catch (FileNotFoundException e) {
+			return computeFromIndex(packIndexSupplier.get());
+		} catch (IOException e) {
+			throw new IOException(
+					MessageFormat.format(JGitText.get().unreadablePackIndex,
+							idxFile.getAbsolutePath()),
+					e);
+		}
+	}
+
+	/**
 	 * Compute an in-memory pack reverse index from the in-memory pack forward
 	 * index. This computation uses insertion sort, which has a quadratic
 	 * runtime on average.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java
index 76f0793cc8..c77a8eb761 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java
@@ -16,10 +16,14 @@ import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.security.DigestInputStream;
 import java.text.MessageFormat;
+import java.util.Arrays;
 
 import org.eclipse.jgit.errors.CorruptObjectException;
+import org.eclipse.jgit.errors.PackMismatchException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.pack.PackExt;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.util.Hex;
 import org.eclipse.jgit.util.IO;
 
 /**
@@ -89,7 +93,20 @@ final class PackReverseIndexV1 implements PackReverseIndex {
 		parseChecksums();
 	}
 
-	private void parseBody() throws IOException {
+	@Override
+	public void verifyPackChecksum(String packFilePath)
+			throws PackMismatchException {
+		if (!Arrays.equals(packChecksum, getPackIndex().getChecksum())) {
+			throw new PackMismatchException(
+					MessageFormat.format(JGitText.get().packChecksumMismatch,
+							packFilePath, PackExt.INDEX.getExtension(),
+							Hex.toHexString(getPackIndex().getChecksum()),
+							PackExt.REVERSE_INDEX.getExtension(),
+							Hex.toHexString(packChecksum)));
+		}
+	}
+
+		private void parseBody() throws IOException {
 		for (int i = 0; i < objectCount; i++) {
 			indexPositionsSortedByOffset[i] = dataIn.readInt();
 		}
@@ -98,10 +115,19 @@ final class PackReverseIndexV1 implements PackReverseIndex {
 	private void parseChecksums() throws IOException {
 		packChecksum = new byte[SHA1_BYTES];
 		IO.readFully(inputStream, packChecksum);
-		// TODO: verify checksum
+
+		// Take digest before reading the self checksum changes it.
+		byte[] observedSelfChecksum = inputStream.getMessageDigest().digest();
 
 		byte[] readSelfChecksum = new byte[SHA1_BYTES];
 		IO.readFully(inputStream, readSelfChecksum);
+
+		if (!Arrays.equals(readSelfChecksum, observedSelfChecksum)) {
+			throw new CorruptObjectException(MessageFormat.format(
+					JGitText.get().corruptReverseIndexChecksumIncorrect,
+					Hex.toHexString(readSelfChecksum),
+					Hex.toHexString(observedSelfChecksum)));
+		}
 	}
 
 	@Override