motiejus
/
jgit
1
Fork 0
Code Releases Activity

Merge changes I8c60d970,I09bdd4b8,I87ff3933 

* changes:
  Pack: open reverse index from file if present
  PackReverseIndex: open file if present otherwise compute
  PackReverseIndex: verify checksums
This commit is contained in:
Jonathan Tan 2023-07-26 16:39:13 -04:00 committed by Gerrit Code Review @ Eclipse.org
parent c8f5a3f99d f196c7a0e8
commit ec11129b1d
12 changed files with 198 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

BIN
org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/pack-cbdeda40019ae0e6e789088ea0f51f164f489d14.corrupt.rev Normal file
View File

Binary file not shown.

BIN
org.eclipse.jgit.test/tst-rsrc/org/eclipse/jgit/test/resources/pack-cbdeda40019ae0e6e789088ea0f51f164f489d14.rev Normal file
View File

Binary file not shown.

7
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputedTest.java
View File

@ -17,6 +17,7 @@
import static org.junit.Assert.fail;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.PackMismatchException;
import org.eclipse.jgit.internal.storage.file.PackIndex.MutableEntry;
import org.eclipse.jgit.junit.JGitTestUtil;
import org.eclipse.jgit.junit.RepositoryTestCase;
@ -92,6 +93,12 @@ public void testFindNextOffsetWrongOffset() {
}
}
@Test
public void testVerifyChecksum() throws PackMismatchException {
// ComputedReverseIndex doesn't have a file containing a checksum.
reverseIdx.verifyPackChecksum(null);
}
private long findFirstOffset() {
long min = Long.MAX_VALUE;
for (MutableEntry me : idx)

44
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexTest.java
View File

@ -10,13 +10,52 @@
package org.eclipse.jgit.internal.storage.file;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import org.eclipse.jgit.internal.storage.pack.PackExt;
import org.eclipse.jgit.junit.JGitTestUtil;
import org.junit.Test;
public class PackReverseIndexTest {
@Test
public void open_fallbackToComputed() throws IOException {
String noRevFilePrefix = "pack-3280af9c07ee18a87705ef50b0cc4cd20266cf12.";
PackReverseIndex computed = PackReverseIndexFactory.openOrCompute(
getResourceFileFor(noRevFilePrefix, PackExt.REVERSE_INDEX), 7,
() -> PackIndex.open(
getResourceFileFor(noRevFilePrefix, PackExt.INDEX)));
assertTrue(computed instanceof PackReverseIndexComputed);
}
@Test
public void open_readGoodFile() throws IOException {
String hasRevFilePrefix = "pack-cbdeda40019ae0e6e789088ea0f51f164f489d14.";
PackReverseIndex version1 = PackReverseIndexFactory.openOrCompute(
getResourceFileFor(hasRevFilePrefix, PackExt.REVERSE_INDEX), 6,
() -> PackIndex.open(
getResourceFileFor(hasRevFilePrefix, PackExt.INDEX)));
assertTrue(version1 instanceof PackReverseIndexV1);
}
@Test
public void open_readCorruptFile() {
String hasRevFilePrefix = "pack-cbdeda40019ae0e6e789088ea0f51f164f489d14.";
assertThrows(IOException.class,
() -> PackReverseIndexFactory.openOrCompute(
getResourceFileFor(hasRevFilePrefix + "corrupt.",
PackExt.REVERSE_INDEX),
6, () -> PackIndex.open(getResourceFileFor(
hasRevFilePrefix, PackExt.INDEX))));
}
@Test
public void read_badMagic() {
byte[] badMagic = new byte[] { 'R', 'B', 'A', 'D', // magic
@ -53,4 +92,9 @@ public void read_unsupportedVersion2() {
assertThrows(IOException.class,
() -> PackReverseIndexFactory.readFromFile(in, 0, () -> null));
}
private File getResourceFileFor(String packFilePrefix, PackExt ext) {
return JGitTestUtil
.getTestResourceFile(packFilePrefix + ext.getExtension());
}
}

43
org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1Test.java
View File

@ -15,11 +15,14 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.PackMismatchException;
import org.eclipse.jgit.junit.JGitTestUtil;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.PackedObjectInfo;
@ -146,6 +149,26 @@ public void read_objectCountTooLarge() {
() -> null));
}
@Test
public void read_incorrectChecksum() {
byte[] badChecksum = new byte[] { 'R', 'I', 'D', 'X', // magic
0x00, 0x00, 0x00, 0x01, // file version
0x00, 0x00, 0x00, 0x01, // oid version
// pack checksum to copy into at byte 12
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
// checksum
(byte) 0xf2, 0x1a, 0x1a, (byte) 0xaa, 0x32, 0x2d, (byte) 0xb9,
(byte) 0xfd, 0x0f, (byte) 0xa5, 0x4c, (byte) 0xea, (byte) 0xcf,
(byte) 0xbb, (byte) 0x99, (byte) 0xde, (byte) 0xd3, 0x4e,
(byte) 0xb1, (byte) 0xee, // would be 0x74 if correct
};
System.arraycopy(FAKE_PACK_CHECKSUM, 0, badChecksum, 12,
FAKE_PACK_CHECKSUM.length);
ByteArrayInputStream in = new ByteArrayInputStream(badChecksum);
assertThrows(CorruptObjectException.class,
() -> PackReverseIndexFactory.readFromFile(in, 0, () -> null));
}
@Test
public void findObject_noObjects() {
assertNull(emptyReverseIndex.findObject(0));
@ -235,6 +258,26 @@ public void findObjectByPosition_badOffset() {
() -> smallReverseIndex.findObjectByPosition(10));
}
@Test
public void verifyChecksum_match() throws IOException {
smallReverseIndex.verifyPackChecksum("smallPackFilePath");
}
@Test
public void verifyChecksum_mismatch() throws IOException {
ByteArrayInputStream in = new ByteArrayInputStream(NO_OBJECTS);
PackIndex mockForwardIndex = mock(PackIndex.class);
when(mockForwardIndex.getChecksum()).thenReturn(
new byte[] { 'D', 'I', 'F', 'F', 'P', 'A', 'C', 'K', 'C', 'H',
'E', 'C', 'K', 'S', 'U', 'M', '7', '8', '9', '0', });
PackReverseIndex reverseIndex = PackReverseIndexFactory.readFromFile(in,
0,
() -> mockForwardIndex);
assertThrows(PackMismatchException.class,
() -> reverseIndex.verifyPackChecksum("packFilePath"));
}
private static PackedObjectInfo objectInfo(String objectId, int type,
long offset) {
PackedObjectInfo objectInfo = new PackedObjectInfo(

3
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
View File

@ -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}

1
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
View File

@ -252,6 +252,7 @@ public static JGitText get() {
/***/ public String corruptObjectTruncatedInName;
/***/ public String corruptObjectTruncatedInObjectId;
/***/ public String corruptObjectZeroId;
/***/ public String corruptReverseIndexChecksumIncorrect;
/***/ public String corruptPack;
/***/ public String corruptUseCnt;
/***/ public String couldNotFindTabInLine;

32
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
View File

@ -14,6 +14,7 @@
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.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 @@ private PackIndex idx() throws IOException {
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 @@ private void onOpenPack() throws IOException {
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 @@ synchronized PackBitmapIndex getBitmapIndex() throws IOException {
}
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;
}

12
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndex.java
View File

@ -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;
/**
@ -34,6 +35,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.

7
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexComputed.java
View File

@ -12,6 +12,7 @@
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;
@ -143,6 +144,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);

33
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexFactory.java
View File

@ -14,6 +14,8 @@
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,11 +25,42 @@
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

30
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackReverseIndexV1.java
View File

@ -16,10 +16,14 @@
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 @@ private void parseBody() throws IOException {
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