Cleanup use of java.util.Inflater, fixing rare infinite loops
The native implementation of inflate() can set finished to return true at the same time as it copies the last bytes into the buffer. Check for finished on each iteration, terminating as soon as libz knows the stream was completely inflated. If not finished, it is likely input is required before the next native call could do any useful work. Most invocations are passing in a buffer large enough to store the entire result. A partial return from inflate() will need more input before it can continue. Checking right away that needsInput() is true saves a native call to determine no bytes can be inflated without more input. This should fix a rare infinite loop condition inside of inflation when an object ends exactly at the end of a block boundary, and the next block contains only the 20 byte trailing SHA-1. When the stream is finished each new attempt to inflate() returns n == 0, as no additional bytes were output. The needsInput() test tries to add the length of the footer block to itself, but then loops back around an reloads the same block as the block is smaller than a full block size. A zero length input is set to the inflater, which triggers needsInput() condition again. Change-Id: I95d02bfeab4bf995a254d49166b4ae62d1f21346
This commit is contained in:
parent
861f5f649f
commit
94c4d7eee8
|
@ -74,11 +74,6 @@ int size() {
|
||||||
return block.length;
|
return block.length;
|
||||||
}
|
}
|
||||||
|
|
||||||
int remaining(long pos) {
|
|
||||||
int ptr = (int) (pos - start);
|
|
||||||
return block.length - ptr;
|
|
||||||
}
|
|
||||||
|
|
||||||
boolean contains(DfsPackKey want, long pos) {
|
boolean contains(DfsPackKey want, long pos) {
|
||||||
return pack == want && start <= pos && pos < end;
|
return pack == want && start <= pos && pos < end;
|
||||||
}
|
}
|
||||||
|
@ -94,9 +89,11 @@ int copy(int p, byte[] b, int o, int n) {
|
||||||
return n;
|
return n;
|
||||||
}
|
}
|
||||||
|
|
||||||
void setInput(Inflater inf, long pos) {
|
int setInput(long pos, Inflater inf) {
|
||||||
int ptr = (int) (pos - start);
|
int ptr = (int) (pos - start);
|
||||||
inf.setInput(block, ptr, block.length - ptr);
|
int cnt = block.length - ptr;
|
||||||
|
inf.setInput(block, ptr, cnt);
|
||||||
|
return cnt;
|
||||||
}
|
}
|
||||||
|
|
||||||
void crc32(CRC32 out, long pos, int cnt) {
|
void crc32(CRC32 out, long pos, int cnt) {
|
||||||
|
|
|
@ -460,31 +460,29 @@ byte[] inflate(DfsReader ctx, long pos, int len) throws IOException,
|
||||||
}
|
}
|
||||||
|
|
||||||
Inflater inf = ctx.inflater();
|
Inflater inf = ctx.inflater();
|
||||||
DfsBlock b = setInput(inf, pos);
|
pos += setInput(pos, inf);
|
||||||
for (int dstoff = 0;;) {
|
for (int dstoff = 0;;) {
|
||||||
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
||||||
if (n > 0)
|
|
||||||
dstoff += n;
|
dstoff += n;
|
||||||
else if (inf.needsInput() && b != null) {
|
if (inf.finished())
|
||||||
pos += b.remaining(pos);
|
|
||||||
b = setInput(inf, pos);
|
|
||||||
} else if (inf.needsInput())
|
|
||||||
throw new EOFException(DfsText.get().unexpectedEofInPack);
|
|
||||||
else if (inf.finished())
|
|
||||||
return dstbuf;
|
return dstbuf;
|
||||||
else
|
if (inf.needsInput())
|
||||||
|
pos += setInput(pos, inf);
|
||||||
|
else if (n == 0)
|
||||||
throw new DataFormatException();
|
throw new DataFormatException();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private DfsBlock setInput(Inflater inf, long pos) throws IOException {
|
private int setInput(long pos, Inflater inf) throws IOException {
|
||||||
if (pos < currPos) {
|
if (pos < currPos)
|
||||||
DfsBlock b = getOrLoadBlock(pos);
|
return getOrLoadBlock(pos).setInput(pos, inf);
|
||||||
b.setInput(inf, pos);
|
if (pos < currPos + currPtr) {
|
||||||
return b;
|
int s = (int) (pos - currPos);
|
||||||
|
int n = currPtr - s;
|
||||||
|
inf.setInput(currBuf, s, n);
|
||||||
|
return n;
|
||||||
}
|
}
|
||||||
inf.setInput(currBuf, (int) (pos - currPos), currPtr);
|
throw new EOFException(DfsText.get().unexpectedEofInPack);
|
||||||
return null;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private DfsBlock getOrLoadBlock(long pos) throws IOException {
|
private DfsBlock getOrLoadBlock(long pos) throws IOException {
|
||||||
|
|
|
@ -600,11 +600,11 @@ void copyPackAsIs(DfsPackFile pack, long length, boolean validate,
|
||||||
* position within the file to read from.
|
* position within the file to read from.
|
||||||
* @param dstbuf
|
* @param dstbuf
|
||||||
* destination buffer the inflater should output decompressed
|
* destination buffer the inflater should output decompressed
|
||||||
* data to.
|
* data to. Must be large enough to store the entire stream,
|
||||||
|
* unless headerOnly is true.
|
||||||
* @param headerOnly
|
* @param headerOnly
|
||||||
* if true the caller wants only {@code dstbuf.length} bytes.
|
* if true the caller wants only {@code dstbuf.length} bytes.
|
||||||
* @return updated <code>dstoff</code> based on the number of bytes
|
* @return number of bytes inflated into <code>dstbuf</code>.
|
||||||
* successfully inflated into <code>dstbuf</code>.
|
|
||||||
* @throws IOException
|
* @throws IOException
|
||||||
* this cursor does not match the provider or id and the proper
|
* this cursor does not match the provider or id and the proper
|
||||||
* window could not be acquired through the provider's cache.
|
* window could not be acquired through the provider's cache.
|
||||||
|
@ -616,25 +616,18 @@ int inflate(DfsPackFile pack, long position, byte[] dstbuf,
|
||||||
boolean headerOnly) throws IOException, DataFormatException {
|
boolean headerOnly) throws IOException, DataFormatException {
|
||||||
prepareInflater();
|
prepareInflater();
|
||||||
pin(pack, position);
|
pin(pack, position);
|
||||||
block.setInput(inf, position);
|
position += block.setInput(position, inf);
|
||||||
int dstoff = 0;
|
for (int dstoff = 0;;) {
|
||||||
for (;;) {
|
|
||||||
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
||||||
if (n == 0) {
|
dstoff += n;
|
||||||
if (headerOnly && dstoff == dstbuf.length)
|
if (inf.finished() || (headerOnly && dstoff == dstbuf.length))
|
||||||
return dstoff;
|
return dstoff;
|
||||||
if (inf.needsInput()) {
|
if (inf.needsInput()) {
|
||||||
position += block.remaining(position);
|
|
||||||
pin(pack, position);
|
pin(pack, position);
|
||||||
block.setInput(inf, position);
|
position += block.setInput(position, inf);
|
||||||
continue;
|
} else if (n == 0)
|
||||||
}
|
|
||||||
if (inf.finished())
|
|
||||||
return dstoff;
|
|
||||||
throw new DataFormatException();
|
throw new DataFormatException();
|
||||||
}
|
}
|
||||||
dstoff += n;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
DfsBlock quickCopy(DfsPackFile p, long pos, long cnt)
|
DfsBlock quickCopy(DfsPackFile p, long pos, long cnt)
|
||||||
|
|
|
@ -335,7 +335,7 @@ private final byte[] decompress(final long position, final int sz,
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (curs.inflate(this, position, dstbuf, 0) != sz)
|
if (curs.inflate(this, position, dstbuf, false) != sz)
|
||||||
throw new EOFException(MessageFormat.format(
|
throw new EOFException(MessageFormat.format(
|
||||||
JGitText.get().shortCompressedStreamAt,
|
JGitText.get().shortCompressedStreamAt,
|
||||||
Long.valueOf(position)));
|
Long.valueOf(position)));
|
||||||
|
@ -886,7 +886,7 @@ byte[] getDeltaHeader(WindowCursor wc, long pos)
|
||||||
// the longest delta instruction header.
|
// the longest delta instruction header.
|
||||||
//
|
//
|
||||||
final byte[] hdr = new byte[18];
|
final byte[] hdr = new byte[18];
|
||||||
wc.inflate(this, pos, hdr, 0);
|
wc.inflate(this, pos, hdr, true /* headerOnly */);
|
||||||
return hdr;
|
return hdr;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -290,11 +290,11 @@ void copyPackAsIs(final PackFile pack, final long length, boolean validate,
|
||||||
* position within the file to read from.
|
* position within the file to read from.
|
||||||
* @param dstbuf
|
* @param dstbuf
|
||||||
* destination buffer the inflater should output decompressed
|
* destination buffer the inflater should output decompressed
|
||||||
* data to.
|
* data to. Must be large enough to store the entire stream,
|
||||||
* @param dstoff
|
* unless headerOnly is true.
|
||||||
* current offset within <code>dstbuf</code> to inflate into.
|
* @param headerOnly
|
||||||
* @return updated <code>dstoff</code> based on the number of bytes
|
* if true the caller wants only {@code dstbuf.length} bytes.
|
||||||
* successfully inflated into <code>dstbuf</code>.
|
* @return number of bytes inflated into <code>dstbuf</code>.
|
||||||
* @throws IOException
|
* @throws IOException
|
||||||
* this cursor does not match the provider or id and the proper
|
* this cursor does not match the provider or id and the proper
|
||||||
* window could not be acquired through the provider's cache.
|
* window could not be acquired through the provider's cache.
|
||||||
|
@ -303,24 +303,21 @@ void copyPackAsIs(final PackFile pack, final long length, boolean validate,
|
||||||
* stream corruption is likely.
|
* stream corruption is likely.
|
||||||
*/
|
*/
|
||||||
int inflate(final PackFile pack, long position, final byte[] dstbuf,
|
int inflate(final PackFile pack, long position, final byte[] dstbuf,
|
||||||
int dstoff) throws IOException, DataFormatException {
|
boolean headerOnly) throws IOException, DataFormatException {
|
||||||
prepareInflater();
|
prepareInflater();
|
||||||
pin(pack, position);
|
pin(pack, position);
|
||||||
position += window.setInput(position, inf);
|
position += window.setInput(position, inf);
|
||||||
do {
|
for (int dstoff = 0;;) {
|
||||||
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
int n = inf.inflate(dstbuf, dstoff, dstbuf.length - dstoff);
|
||||||
if (n == 0) {
|
dstoff += n;
|
||||||
|
if (inf.finished() || (headerOnly && dstoff == dstbuf.length))
|
||||||
|
return dstoff;
|
||||||
if (inf.needsInput()) {
|
if (inf.needsInput()) {
|
||||||
pin(pack, position);
|
pin(pack, position);
|
||||||
position += window.setInput(position, inf);
|
position += window.setInput(position, inf);
|
||||||
} else if (inf.finished())
|
} else if (n == 0)
|
||||||
return dstoff;
|
|
||||||
else
|
|
||||||
throw new DataFormatException();
|
throw new DataFormatException();
|
||||||
}
|
}
|
||||||
dstoff += n;
|
|
||||||
} while (dstoff < dstbuf.length);
|
|
||||||
return dstoff;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
ByteArrayWindow quickCopy(PackFile p, long pos, long cnt)
|
ByteArrayWindow quickCopy(PackFile p, long pos, long cnt)
|
||||||
|
|
|
@ -1653,7 +1653,7 @@ public int read(byte[] dst, int pos, int cnt) throws IOException {
|
||||||
int n = 0;
|
int n = 0;
|
||||||
while (n < cnt) {
|
while (n < cnt) {
|
||||||
int r = inf.inflate(dst, pos + n, cnt - n);
|
int r = inf.inflate(dst, pos + n, cnt - n);
|
||||||
if (r == 0) {
|
n += r;
|
||||||
if (inf.finished())
|
if (inf.finished())
|
||||||
break;
|
break;
|
||||||
if (inf.needsInput()) {
|
if (inf.needsInput()) {
|
||||||
|
@ -1662,16 +1662,11 @@ public int read(byte[] dst, int pos, int cnt) throws IOException {
|
||||||
|
|
||||||
p = fill(src, 1);
|
p = fill(src, 1);
|
||||||
inf.setInput(buf, p, bAvail);
|
inf.setInput(buf, p, bAvail);
|
||||||
} else {
|
} else if (r == 0) {
|
||||||
throw new CorruptObjectException(
|
throw new CorruptObjectException(MessageFormat.format(
|
||||||
MessageFormat
|
|
||||||
.format(
|
|
||||||
JGitText.get().packfileCorruptionDetected,
|
JGitText.get().packfileCorruptionDetected,
|
||||||
JGitText.get().unknownZlibError));
|
JGitText.get().unknownZlibError));
|
||||||
}
|
}
|
||||||
} else {
|
|
||||||
n += r;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
actualSize += n;
|
actualSize += n;
|
||||||
return 0 < n ? n : -1;
|
return 0 < n ? n : -1;
|
||||||
|
|
Loading…
Reference in New Issue