Remove validate support when reusing cached pack
Cached packs are only used when writing over the network or to a bundle file and reuse validation is always disabled in these two contexts. The client/consumer of the stream will be SHA-1 checksumming every object. Reuse validation is most critical during local GC to avoid silently ignoring corruption by stopping as soon as a problem is found and leaving everything alone for the end-user to debug and salvage. Cached packs are not supported during local GC as the bitmap rebuild logic does not support including a cached pack in the result. Strip out the validation and force PackWriter to always disable the cached pack feature if reuseValidation is enabled. Change-Id: If0d7baf2ae1bf1f7e71bf773151302c9f7887039
This commit is contained in:
parent
a3476ced1f
commit
6e5c71b358
|
@ -46,7 +46,6 @@
|
|||
package org.eclipse.jgit.internal.storage.dfs;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.security.MessageDigest;
|
||||
import java.util.zip.CRC32;
|
||||
import java.util.zip.DataFormatException;
|
||||
import java.util.zip.Inflater;
|
||||
|
@ -101,12 +100,9 @@ void crc32(CRC32 out, long pos, int cnt) {
|
|||
out.update(block, ptr, cnt);
|
||||
}
|
||||
|
||||
void write(PackOutputStream out, long pos, int cnt, MessageDigest digest)
|
||||
void write(PackOutputStream out, long pos, int cnt)
|
||||
throws IOException {
|
||||
int ptr = (int) (pos - start);
|
||||
out.write(block, ptr, cnt);
|
||||
if (digest != null)
|
||||
digest.update(block, ptr, cnt);
|
||||
out.write(block, (int) (pos - start), cnt);
|
||||
}
|
||||
|
||||
void check(Inflater inf, byte[] tmp, long pos, int cnt)
|
||||
|
|
|
@ -78,8 +78,7 @@ public boolean hasObject(ObjectToPack obj, StoredObjectRepresentation rep) {
|
|||
return ((DfsObjectRepresentation) rep).pack == pack;
|
||||
}
|
||||
|
||||
void copyAsIs(PackOutputStream out, boolean validate, DfsReader ctx)
|
||||
throws IOException {
|
||||
pack.copyPackAsIs(out, validate, ctx);
|
||||
void copyAsIs(PackOutputStream out, DfsReader ctx) throws IOException {
|
||||
pack.copyPackAsIs(out, ctx);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -56,9 +56,7 @@
|
|||
import java.io.InputStream;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.nio.channels.Channels;
|
||||
import java.security.MessageDigest;
|
||||
import java.text.MessageFormat;
|
||||
import java.util.Arrays;
|
||||
import java.util.Set;
|
||||
import java.util.zip.CRC32;
|
||||
import java.util.zip.DataFormatException;
|
||||
|
@ -466,55 +464,35 @@ private byte[] decompress(long position, int sz, DfsReader ctx)
|
|||
return dstbuf;
|
||||
}
|
||||
|
||||
void copyPackAsIs(PackOutputStream out, boolean validate, DfsReader ctx)
|
||||
void copyPackAsIs(PackOutputStream out, DfsReader ctx)
|
||||
throws IOException {
|
||||
MessageDigest md = initCopyPack(out, validate, ctx);
|
||||
long p;
|
||||
if (cache.shouldCopyThroughCache(length))
|
||||
p = copyPackThroughCache(out, ctx, md);
|
||||
else
|
||||
p = copyPackBypassCache(out, ctx, md);
|
||||
verifyPackChecksum(p, md, ctx);
|
||||
}
|
||||
|
||||
private MessageDigest initCopyPack(PackOutputStream out, boolean validate,
|
||||
DfsReader ctx) throws IOException {
|
||||
// If the length hasn't been determined yet, pin to set it.
|
||||
if (length == -1)
|
||||
if (length == -1) {
|
||||
ctx.pin(this, 0);
|
||||
if (!validate) {
|
||||
ctx.unpin();
|
||||
return null;
|
||||
}
|
||||
|
||||
int hdrlen = 12;
|
||||
byte[] buf = out.getCopyBuffer();
|
||||
int n = ctx.copy(this, 0, buf, 0, hdrlen);
|
||||
ctx.unpin();
|
||||
if (n != hdrlen)
|
||||
throw packfileIsTruncated();
|
||||
MessageDigest md = Constants.newMessageDigest();
|
||||
md.update(buf, 0, hdrlen);
|
||||
return md;
|
||||
if (cache.shouldCopyThroughCache(length))
|
||||
copyPackThroughCache(out, ctx);
|
||||
else
|
||||
copyPackBypassCache(out, ctx);
|
||||
}
|
||||
|
||||
private long copyPackThroughCache(PackOutputStream out, DfsReader ctx,
|
||||
MessageDigest md) throws IOException {
|
||||
private void copyPackThroughCache(PackOutputStream out, DfsReader ctx)
|
||||
throws IOException {
|
||||
long position = 12;
|
||||
long remaining = length - (12 + 20);
|
||||
while (0 < remaining) {
|
||||
DfsBlock b = cache.getOrLoad(this, position, ctx);
|
||||
int ptr = (int) (position - b.start);
|
||||
int n = (int) Math.min(b.size() - ptr, remaining);
|
||||
b.write(out, position, n, md);
|
||||
b.write(out, position, n);
|
||||
position += n;
|
||||
remaining -= n;
|
||||
}
|
||||
return position;
|
||||
}
|
||||
|
||||
private long copyPackBypassCache(PackOutputStream out, DfsReader ctx,
|
||||
MessageDigest md) throws IOException {
|
||||
private long copyPackBypassCache(PackOutputStream out, DfsReader ctx)
|
||||
throws IOException {
|
||||
try (ReadableChannel rc = ctx.db.openFile(packDesc, PACK)) {
|
||||
ByteBuffer buf = newCopyBuffer(out, rc);
|
||||
if (ctx.getOptions().getStreamPackBufferSize() > 0)
|
||||
|
@ -526,7 +504,7 @@ private long copyPackBypassCache(PackOutputStream out, DfsReader ctx,
|
|||
if (b != null) {
|
||||
int ptr = (int) (position - b.start);
|
||||
int n = (int) Math.min(b.size() - ptr, remaining);
|
||||
b.write(out, position, n, md);
|
||||
b.write(out, position, n);
|
||||
position += n;
|
||||
remaining -= n;
|
||||
rc.position(position);
|
||||
|
@ -540,8 +518,6 @@ private long copyPackBypassCache(PackOutputStream out, DfsReader ctx,
|
|||
else if (n > remaining)
|
||||
n = (int) remaining;
|
||||
out.write(buf.array(), 0, n);
|
||||
if (md != null)
|
||||
md.update(buf.array(), 0, n);
|
||||
position += n;
|
||||
remaining -= n;
|
||||
}
|
||||
|
@ -557,22 +533,6 @@ private ByteBuffer newCopyBuffer(PackOutputStream out, ReadableChannel rc) {
|
|||
return ByteBuffer.wrap(copyBuf, 0, bs);
|
||||
}
|
||||
|
||||
private void verifyPackChecksum(long position, MessageDigest md,
|
||||
DfsReader ctx) throws IOException {
|
||||
if (md != null) {
|
||||
byte[] buf = new byte[20];
|
||||
byte[] actHash = md.digest();
|
||||
if (ctx.copy(this, position, buf, 0, 20) != 20)
|
||||
throw packfileIsTruncated();
|
||||
if (!Arrays.equals(actHash, buf)) {
|
||||
invalid = true;
|
||||
throw new IOException(MessageFormat.format(
|
||||
JGitText.get().packfileCorruptionDetected,
|
||||
getPackName()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void copyAsIs(PackOutputStream out, DfsObjectToPack src,
|
||||
boolean validate, DfsReader ctx) throws IOException,
|
||||
StoredObjectRepresentationNotAvailableException {
|
||||
|
@ -719,7 +679,7 @@ void copyAsIs(PackOutputStream out, DfsObjectToPack src,
|
|||
// and we have it pinned. Write this out without copying.
|
||||
//
|
||||
out.writeHeader(src, inflatedLength);
|
||||
quickCopy.write(out, dataOffset, (int) dataLength, null);
|
||||
quickCopy.write(out, dataOffset, (int) dataLength);
|
||||
|
||||
} else if (dataLength <= buf.length) {
|
||||
// Tiny optimization: Lots of objects are very small deltas or
|
||||
|
|
|
@ -492,9 +492,9 @@ public void writeObjects(PackOutputStream out, List<ObjectToPack> list)
|
|||
out.writeObject(otp);
|
||||
}
|
||||
|
||||
public void copyPackAsIs(PackOutputStream out, CachedPack pack,
|
||||
boolean validate) throws IOException {
|
||||
((DfsCachedPack) pack).copyAsIs(out, validate, this);
|
||||
public void copyPackAsIs(PackOutputStream out, CachedPack pack)
|
||||
throws IOException {
|
||||
((DfsCachedPack) pack).copyAsIs(out, this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -46,7 +46,6 @@
|
|||
package org.eclipse.jgit.internal.storage.file;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.security.MessageDigest;
|
||||
import java.util.zip.CRC32;
|
||||
import java.util.zip.DataFormatException;
|
||||
import java.util.zip.Inflater;
|
||||
|
@ -84,12 +83,10 @@ void crc32(CRC32 out, long pos, int cnt) {
|
|||
}
|
||||
|
||||
@Override
|
||||
void write(PackOutputStream out, long pos, int cnt, MessageDigest digest)
|
||||
void write(PackOutputStream out, long pos, int cnt)
|
||||
throws IOException {
|
||||
int ptr = (int) (pos - start);
|
||||
out.write(array, ptr, cnt);
|
||||
if (digest != null)
|
||||
digest.update(array, ptr, cnt);
|
||||
}
|
||||
|
||||
void check(Inflater inf, byte[] tmp, long pos, int cnt)
|
||||
|
|
|
@ -47,7 +47,6 @@
|
|||
|
||||
import java.io.IOException;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.security.MessageDigest;
|
||||
import java.util.zip.DataFormatException;
|
||||
import java.util.zip.Inflater;
|
||||
|
||||
|
@ -76,7 +75,7 @@ protected int copy(final int p, final byte[] b, final int o, int n) {
|
|||
}
|
||||
|
||||
@Override
|
||||
void write(PackOutputStream out, long pos, int cnt, MessageDigest digest)
|
||||
void write(PackOutputStream out, long pos, int cnt)
|
||||
throws IOException {
|
||||
final ByteBuffer s = buffer.slice();
|
||||
s.position((int) (pos - start));
|
||||
|
@ -86,8 +85,6 @@ void write(PackOutputStream out, long pos, int cnt, MessageDigest digest)
|
|||
int n = Math.min(cnt, buf.length);
|
||||
s.get(buf, 0, n);
|
||||
out.write(buf, 0, n);
|
||||
if (digest != null)
|
||||
digest.update(buf, 0, n);
|
||||
cnt -= n;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -45,7 +45,6 @@
|
|||
package org.eclipse.jgit.internal.storage.file;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.security.MessageDigest;
|
||||
import java.util.zip.DataFormatException;
|
||||
import java.util.zip.Inflater;
|
||||
|
||||
|
@ -121,8 +120,8 @@ final int copy(long pos, byte[] dstbuf, int dstoff, int cnt) {
|
|||
*/
|
||||
protected abstract int copy(int pos, byte[] dstbuf, int dstoff, int cnt);
|
||||
|
||||
abstract void write(PackOutputStream out, long pos, int cnt,
|
||||
MessageDigest md) throws IOException;
|
||||
abstract void write(PackOutputStream out, long pos, int cnt)
|
||||
throws IOException;
|
||||
|
||||
final int setInput(long pos, Inflater inf) throws DataFormatException {
|
||||
return setInput((int) (pos - start), inf);
|
||||
|
|
|
@ -79,10 +79,10 @@ public long getObjectCount() throws IOException {
|
|||
return cnt;
|
||||
}
|
||||
|
||||
void copyAsIs(PackOutputStream out, boolean validate, WindowCursor wc)
|
||||
void copyAsIs(PackOutputStream out, WindowCursor wc)
|
||||
throws IOException {
|
||||
for (PackFile pack : getPacks())
|
||||
pack.copyPackAsIs(out, validate, wc);
|
||||
pack.copyPackAsIs(out, wc);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -344,11 +344,11 @@ private final byte[] decompress(final long position, final int sz,
|
|||
return dstbuf;
|
||||
}
|
||||
|
||||
void copyPackAsIs(PackOutputStream out, boolean validate, WindowCursor curs)
|
||||
void copyPackAsIs(PackOutputStream out, WindowCursor curs)
|
||||
throws IOException {
|
||||
// Pin the first window, this ensures the length is accurate.
|
||||
curs.pin(this, 0);
|
||||
curs.copyPackAsIs(this, length, validate, out);
|
||||
curs.copyPackAsIs(this, length, out);
|
||||
}
|
||||
|
||||
final void copyAsIs(PackOutputStream out, LocalObjectToPack src,
|
||||
|
@ -502,7 +502,7 @@ private void copyAsIs2(PackOutputStream out, LocalObjectToPack src,
|
|||
// and we have it pinned. Write this out without copying.
|
||||
//
|
||||
out.writeHeader(src, inflatedLength);
|
||||
quickCopy.write(out, dataOffset, (int) dataLength, null);
|
||||
quickCopy.write(out, dataOffset, (int) dataLength);
|
||||
|
||||
} else if (dataLength <= buf.length) {
|
||||
// Tiny optimization: Lots of objects are very small deltas or
|
||||
|
|
|
@ -45,9 +45,6 @@
|
|||
package org.eclipse.jgit.internal.storage.file;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.security.MessageDigest;
|
||||
import java.text.MessageFormat;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
|
@ -59,7 +56,6 @@
|
|||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||
import org.eclipse.jgit.errors.MissingObjectException;
|
||||
import org.eclipse.jgit.errors.StoredObjectRepresentationNotAvailableException;
|
||||
import org.eclipse.jgit.internal.JGitText;
|
||||
import org.eclipse.jgit.internal.storage.pack.CachedPack;
|
||||
import org.eclipse.jgit.internal.storage.pack.ObjectReuseAsIs;
|
||||
import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
|
||||
|
@ -232,27 +228,13 @@ int copy(final PackFile pack, long position, final byte[] dstbuf,
|
|||
return cnt - need;
|
||||
}
|
||||
|
||||
public void copyPackAsIs(PackOutputStream out, CachedPack pack,
|
||||
boolean validate) throws IOException {
|
||||
((LocalCachedPack) pack).copyAsIs(out, validate, this);
|
||||
public void copyPackAsIs(PackOutputStream out, CachedPack pack)
|
||||
throws IOException {
|
||||
((LocalCachedPack) pack).copyAsIs(out, this);
|
||||
}
|
||||
|
||||
void copyPackAsIs(final PackFile pack, final long length, boolean validate,
|
||||
void copyPackAsIs(final PackFile pack, final long length,
|
||||
final PackOutputStream out) throws IOException {
|
||||
MessageDigest md = null;
|
||||
if (validate) {
|
||||
md = Constants.newMessageDigest();
|
||||
byte[] buf = out.getCopyBuffer();
|
||||
pin(pack, 0);
|
||||
if (window.copy(0, buf, 0, 12) != 12) {
|
||||
pack.setInvalid();
|
||||
throw new IOException(MessageFormat.format(
|
||||
JGitText.get().packfileIsTruncated, pack.getPackFile()
|
||||
.getPath()));
|
||||
}
|
||||
md.update(buf, 0, 12);
|
||||
}
|
||||
|
||||
long position = 12;
|
||||
long remaining = length - (12 + 20);
|
||||
while (0 < remaining) {
|
||||
|
@ -260,29 +242,10 @@ void copyPackAsIs(final PackFile pack, final long length, boolean validate,
|
|||
|
||||
int ptr = (int) (position - window.start);
|
||||
int n = (int) Math.min(window.size() - ptr, remaining);
|
||||
window.write(out, position, n, md);
|
||||
window.write(out, position, n);
|
||||
position += n;
|
||||
remaining -= n;
|
||||
}
|
||||
|
||||
if (md != null) {
|
||||
byte[] buf = new byte[20];
|
||||
byte[] actHash = md.digest();
|
||||
|
||||
pin(pack, position);
|
||||
if (window.copy(position, buf, 0, 20) != 20) {
|
||||
pack.setInvalid();
|
||||
throw new IOException(MessageFormat.format(
|
||||
JGitText.get().packfileIsTruncated, pack.getPackFile()
|
||||
.getPath()));
|
||||
}
|
||||
if (!Arrays.equals(actHash, buf)) {
|
||||
pack.setInvalid();
|
||||
throw new IOException(MessageFormat.format(
|
||||
JGitText.get().packfileCorruptionDetected, pack
|
||||
.getPackFile().getPath()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -209,16 +209,11 @@ public void copyObjectAsIs(PackOutputStream out, ObjectToPack otp,
|
|||
* stream to append the pack onto.
|
||||
* @param pack
|
||||
* the cached pack to send.
|
||||
* @param validate
|
||||
* if true the representation must be validated and not be
|
||||
* corrupt before being reused. If false, validation may be
|
||||
* skipped as it will be performed elsewhere in the processing
|
||||
* pipeline.
|
||||
* @throws IOException
|
||||
* the pack cannot be read, or stream did not accept a write.
|
||||
*/
|
||||
public abstract void copyPackAsIs(PackOutputStream out, CachedPack pack,
|
||||
boolean validate) throws IOException;
|
||||
public abstract void copyPackAsIs(PackOutputStream out, CachedPack pack)
|
||||
throws IOException;
|
||||
|
||||
/**
|
||||
* Obtain the available cached packs that match the bitmap and update
|
||||
|
|
|
@ -1048,7 +1048,7 @@ public void writePack(ProgressMonitor compressMonitor,
|
|||
stats.reusedObjects += pack.getObjectCount();
|
||||
stats.reusedDeltas += deltaCnt;
|
||||
stats.totalDeltas += deltaCnt;
|
||||
reuseSupport.copyPackAsIs(out, pack, reuseValidate);
|
||||
reuseSupport.copyPackAsIs(out, pack);
|
||||
}
|
||||
writeChecksum(out);
|
||||
out.flush();
|
||||
|
@ -1866,7 +1866,7 @@ private void findObjectsToPackUsingBitmaps(
|
|||
false);
|
||||
BitmapBuilder needBitmap = wantBitmap.andNot(haveBitmap);
|
||||
|
||||
if (useCachedPacks && reuseSupport != null
|
||||
if (useCachedPacks && reuseSupport != null && !reuseValidate
|
||||
&& (excludeInPacks == null || excludeInPacks.length == 0))
|
||||
cachedPacks.addAll(
|
||||
reuseSupport.getCachedPacksAndUpdate(needBitmap));
|
||||
|
|
Loading…
Reference in New Issue