Fix infinite loop in IndexPack

A programming error using the Inflater API led to an infinite
loop within IndexPack, caused by the Inflater returning 0 from
the inflate() method, but it didn't want more input.  This happens
when it has reached the end of the stream, or has reached a spot
asking for an external dictionary.  Such a case is a failure for us,
and we should abort out.

Thanks to Alex for pointing out that we had 3 implementations of
the inflate rountine, which should be consolidated into one and
use a switch to determine where to load data from.

Bug: 317416
Change-Id: I34120482375b687ea36ed9154002d77047e94b1f
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties

@@ -357,6 +357,7 @@ unknownIndexVersionOrCorruptIndex=Unknown index version (or corrupt index): {0}
 unknownObjectType=Unknown object type {0}.
 unknownRepositoryFormat2=Unknown repository format "{0}"; expected "0".
 unknownRepositoryFormat=Unknown repository format
+unknownZlibError=Unknown zlib error.
 unmergedPath=Unmerged path: {0}
 unpackError=unpack error {0}
 unreadablePackIndex=Unreadable pack index: {0}

org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java

@@ -416,6 +416,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String unknownObjectType;
 	/***/ public String unknownRepositoryFormat2;
 	/***/ public String unknownRepositoryFormat;
+	/***/ public String unknownZlibError;
 	/***/ public String unmergedPath;
 	/***/ public String unpackError;
 	/***/ public String unreadablePackIndex;

org.eclipse.jgit/src/org/eclipse/jgit/transport/IndexPack.java

@@ -131,6 +131,19 @@ public class IndexPack {
 		return ip;
 	}
 
+	private static enum Source {
+		/** Data is read from the incoming stream. */
+		INPUT,
+
+		/**
+		 * Data is read from the spooled pack file.
+		 * <p>
+		 * During streaming, some (or all) data might be saved into the spooled
+		 * pack file so it can be randomly accessed later.
+		 */
+		FILE;
+	}
+
 	private final Repository repo;
 
 	/**
@@ -200,7 +213,7 @@ public class IndexPack {
 
 	private LongMap<UnresolvedDelta> baseByPos;
 
-	private byte[] objectData;
+	private byte[] skipBuffer;
 
 	private MessageDigest packDigest;
 
@@ -232,7 +245,7 @@ public class IndexPack {
 		inflater = InflaterCache.get();
 		readCurs = new WindowCursor();
 		buf = new byte[BUFFER_SIZE];
-		objectData = new byte[BUFFER_SIZE];
+		skipBuffer = new byte[512];
 		objectDigest = Constants.newMessageDigest();
 		tempObjectId = new MutableObjectId();
 		packDigest = Constants.newMessageDigest();
@@ -474,12 +487,12 @@ public class IndexPack {
 			byte[] data, PackedObjectInfo oe) throws IOException {
 		crc.reset();
 		position(pos);
-		int c = readFromFile();
+		int c = readFrom(Source.FILE);
 		final int typeCode = (c >> 4) & 7;
 		long sz = c & 15;
 		int shift = 4;
 		while ((c & 0x80) != 0) {
-			c = readFromFile();
+			c = readFrom(Source.FILE);
 			sz += (c & 0x7f) << shift;
 			shift += 7;
 		}
@@ -490,19 +503,19 @@ public class IndexPack {
 		case Constants.OBJ_BLOB:
 		case Constants.OBJ_TAG:
 			type = typeCode;
-			data = inflateFromFile((int) sz);
+			data = inflateAndReturn(Source.FILE, sz);
 			break;
 		case Constants.OBJ_OFS_DELTA: {
-			c = readFromFile() & 0xff;
+			c = readFrom(Source.FILE) & 0xff;
 			while ((c & 128) != 0)
-				c = readFromFile() & 0xff;
-			data = BinaryDelta.apply(data, inflateFromFile((int) sz));
+				c = readFrom(Source.FILE) & 0xff;
+			data = BinaryDelta.apply(data, inflateAndReturn(Source.FILE, sz));
 			break;
 		}
 		case Constants.OBJ_REF_DELTA: {
-			crc.update(buf, fillFromFile(20), 20);
+			crc.update(buf, fill(Source.FILE, 20), 20);
 			use(20);
-			data = BinaryDelta.apply(data, inflateFromFile((int) sz));
+			data = BinaryDelta.apply(data, inflateAndReturn(Source.FILE, sz));
 			break;
 		}
 		default:
@@ -657,7 +670,7 @@ public class IndexPack {
 		packOut.seek(0);
 		bAvail = 0;
 		bOffset = 0;
-		fillFromFile(12);
+		fill(Source.FILE, 12);
 
 		{
 			final int origCnt = (int) Math.min(bAvail, origRemaining);
@@ -728,7 +741,7 @@ public class IndexPack {
 
 	private void readPackHeader() throws IOException {
 		final int hdrln = Constants.PACK_SIGNATURE.length + 4 + 4;
-		final int p = fillFromInput(hdrln);
+		final int p = fill(Source.INPUT, hdrln);
 		for (int k = 0; k < Constants.PACK_SIGNATURE.length; k++)
 			if (buf[p + k] != Constants.PACK_SIGNATURE[k])
 				throw new IOException(JGitText.get().notAPACKFile);
@@ -743,7 +756,7 @@ public class IndexPack {
 	private void readPackFooter() throws IOException {
 		sync();
 		final byte[] cmpcsum = packDigest.digest();
-		final int c = fillFromInput(20);
+		final int c = fill(Source.INPUT, 20);
 		packcsum = new byte[20];
 		System.arraycopy(buf, c, packcsum, 0, 20);
 		use(20);
@@ -757,7 +770,7 @@ public class IndexPack {
 	// Cleanup all resources associated with our input parsing.
 	private void endInput() {
 		in = null;
-		objectData = null;
+		skipBuffer = null;
 	}
 
 	// Read one entire object or delta from the input.
@@ -765,12 +778,12 @@ public class IndexPack {
 		final long pos = position();
 
 		crc.reset();
-		int c = readFromInput();
+		int c = readFrom(Source.INPUT);
 		final int typeCode = (c >> 4) & 7;
 		long sz = c & 15;
 		int shift = 4;
 		while ((c & 0x80) != 0) {
-			c = readFromInput();
+			c = readFrom(Source.INPUT);
 			sz += (c & 0x7f) << shift;
 			shift += 7;
 		}
@@ -783,24 +796,24 @@ public class IndexPack {
 			whole(typeCode, pos, sz);
 			break;
 		case Constants.OBJ_OFS_DELTA: {
-			c = readFromInput();
+			c = readFrom(Source.INPUT);
 			long ofs = c & 127;
 			while ((c & 128) != 0) {
 				ofs += 1;
-				c = readFromInput();
+				c = readFrom(Source.INPUT);
 				ofs <<= 7;
 				ofs += (c & 127);
 			}
 			final long base = pos - ofs;
 			final UnresolvedDelta n;
-			skipInflateFromInput(sz);
+			inflateAndSkip(Source.INPUT, sz);
 			n = new UnresolvedDelta(pos, (int) crc.getValue());
 			n.next = baseByPos.put(base, n);
 			deltaCount++;
 			break;
 		}
 		case Constants.OBJ_REF_DELTA: {
-			c = fillFromInput(20);
+			c = fill(Source.INPUT, 20);
 			crc.update(buf, c, 20);
 			final ObjectId base = ObjectId.fromRaw(buf, c);
 			use(20);
@@ -809,7 +822,7 @@ public class IndexPack {
 				r = new DeltaChain(base);
 				baseById.add(r);
 			}
-			skipInflateFromInput(sz);
+			inflateAndSkip(Source.INPUT, sz);
 			r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
 			deltaCount++;
 			break;
@@ -821,7 +834,7 @@ public class IndexPack {
 
 	private void whole(final int type, final long pos, final long sz)
 			throws IOException {
-		final byte[] data = inflateFromInput(sz);
+		final byte[] data = inflateAndReturn(Source.INPUT, sz);
 		objectDigest.update(Constants.encodedTypeString(type));
 		objectDigest.update((byte) ' ');
 		objectDigest.update(Constants.encodeASCII(sz));
@@ -867,19 +880,9 @@ public class IndexPack {
 	}
 
 	// Consume exactly one byte from the buffer and return it.
-	private int readFromInput() throws IOException {
-		if (bAvail == 0)
-			fillFromInput(1);
-		bAvail--;
-		final int b = buf[bOffset++] & 0xff;
-		crc.update(b);
-		return b;
-	}
-
-	// Consume exactly one byte from the buffer and return it.
-	private int readFromFile() throws IOException {
+	private int readFrom(final Source src) throws IOException {
 		if (bAvail == 0)
-			fillFromFile(1);
+			fill(src, 1);
 		bAvail--;
 		final int b = buf[bOffset++] & 0xff;
 		crc.update(b);
@@ -893,36 +896,32 @@ public class IndexPack {
 	}
 
 	// Ensure at least need bytes are available in in {@link #buf}.
-	private int fillFromInput(final int need) throws IOException {
+	private int fill(final Source src, final int need) throws IOException {
 		while (bAvail < need) {
 			int next = bOffset + bAvail;
 			int free = buf.length - next;
 			if (free + bAvail < need) {
-				sync();
+				switch(src){
+				case INPUT:
+					sync();
+					break;
+				case FILE:
+					if (bAvail > 0)
+						System.arraycopy(buf, bOffset, buf, 0, bAvail);
+					bOffset = 0;
+					break;
+				}
 				next = bAvail;
 				free = buf.length - next;
 			}
-			next = in.read(buf, next, free);
-			if (next <= 0)
-				throw new EOFException(JGitText.get().packfileIsTruncated);
-			bAvail += next;
-		}
-		return bOffset;
-	}
-
-	// Ensure at least need bytes are available in in {@link #buf}.
-	private int fillFromFile(final int need) throws IOException {
-		if (bAvail < need) {
-			int next = bOffset + bAvail;
-			int free = buf.length - next;
-			if (free + bAvail < need) {
-				if (bAvail > 0)
-					System.arraycopy(buf, bOffset, buf, 0, bAvail);
-				bOffset = 0;
-				next = bAvail;
-				free = buf.length - next;
+			switch(src){
+			case INPUT:
+				next = in.read(buf, next, free);
+				break;
+			case FILE:
+				next = packOut.read(buf, next, free);
+				break;
 			}
-			next = packOut.read(buf, next, free);
 			if (next <= 0)
 				throw new EOFException(JGitText.get().packfileIsTruncated);
 			bAvail += next;
@@ -941,112 +940,69 @@ public class IndexPack {
 		bOffset = 0;
 	}
 
-	private void skipInflateFromInput(long sz) throws IOException {
-		final Inflater inf = inflater;
-		try {
-			final byte[] dst = objectData;
-			int n = 0;
-			int p = -1;
-			while (!inf.finished()) {
-				if (inf.needsInput()) {
-					if (p >= 0) {
-						crc.update(buf, p, bAvail);
-						use(bAvail);
-					}
-					p = fillFromInput(1);
-					inf.setInput(buf, p, bAvail);
-				}
-
-				int free = dst.length - n;
-				if (free < 8) {
-					sz -= n;
-					n = 0;
-					free = dst.length;
-				}
-				n += inf.inflate(dst, n, free);
-			}
-			if (n != sz)
-				throw new DataFormatException(JGitText.get().wrongDecompressedLength);
-			n = bAvail - inf.getRemaining();
-			if (n > 0) {
-				crc.update(buf, p, n);
-				use(n);
-			}
-		} catch (DataFormatException dfe) {
-			throw corrupt(dfe);
-		} finally {
-			inf.reset();
-		}
+	private void inflateAndSkip(final Source src, final long inflatedSize)
+			throws IOException {
+		inflate(src, inflatedSize, skipBuffer, false /* do not keep result */);
 	}
 
-	private byte[] inflateFromInput(final long sz) throws IOException {
-		final byte[] dst = new byte[(int) sz];
-		final Inflater inf = inflater;
-		try {
-			int n = 0;
-			int p = -1;
-			while (!inf.finished()) {
-				if (inf.needsInput()) {
-					if (p >= 0) {
-						crc.update(buf, p, bAvail);
-						use(bAvail);
-					}
-					p = fillFromInput(1);
-					inf.setInput(buf, p, bAvail);
-				}
-
-				n += inf.inflate(dst, n, dst.length - n);
-			}
-			if (n != sz)
-				throw new DataFormatException(JGitText.get().wrongDecompressedLength);
-			n = bAvail - inf.getRemaining();
-			if (n > 0) {
-				crc.update(buf, p, n);
-				use(n);
-			}
-			return dst;
-		} catch (DataFormatException dfe) {
-			throw corrupt(dfe);
-		} finally {
-			inf.reset();
-		}
+	private byte[] inflateAndReturn(final Source src, final long inflatedSize)
+			throws IOException {
+		final byte[] dst = new byte[(int) inflatedSize];
+		inflate(src, inflatedSize, dst, true /* keep result in dst */);
+		return dst;
 	}
 
-	private byte[] inflateFromFile(final int sz) throws IOException {
+	private void inflate(final Source src, final long inflatedSize,
+			final byte[] dst, final boolean keep) throws IOException {
 		final Inflater inf = inflater;
 		try {
-			final byte[] dst = new byte[sz];
-			int n = 0;
-			int p = -1;
-			while (!inf.finished()) {
-				if (inf.needsInput()) {
-					if (p >= 0) {
-						crc.update(buf, p, bAvail);
-						use(bAvail);
+			int off = 0;
+			long cnt = 0;
+			int p = fill(src, 24);
+			inf.setInput(buf, p, bAvail);
+
+			do {
+				int r = inf.inflate(dst, off, dst.length - off);
+				if (r == 0) {
+					if (inf.finished())
+						break;
+					if (inf.needsInput()) {
+						if (p >= 0) {
+							crc.update(buf, p, bAvail);
+							use(bAvail);
+						}
+						p = fill(src, 24);
+						inf.setInput(buf, p, bAvail);
+					} else {
+						throw new CorruptObjectException(MessageFormat.format(
+								JGitText.get().packfileCorruptionDetected,
+								JGitText.get().unknownZlibError));
 					}
-					p = fillFromFile(1);
-					inf.setInput(buf, p, bAvail);
 				}
-				n += inf.inflate(dst, n, sz - n);
+				cnt += r;
+				if (keep)
+					off += r;
+			} while (cnt < inflatedSize);
+
+			if (!inf.finished() || cnt != inflatedSize) {
+				throw new CorruptObjectException(MessageFormat.format(JGitText
+						.get().packfileCorruptionDetected,
+						JGitText.get().wrongDecompressedLength));
 			}
-			n = bAvail - inf.getRemaining();
-			if (n > 0) {
-				crc.update(buf, p, n);
-				use(n);
+
+			int left = bAvail - inf.getRemaining();
+			if (left > 0) {
+				crc.update(buf, p, left);
+				use(left);
 			}
-			return dst;
 		} catch (DataFormatException dfe) {
-			throw corrupt(dfe);
+			throw new CorruptObjectException(MessageFormat.format(JGitText
+					.get().packfileCorruptionDetected, dfe.getMessage()));
 		} finally {
 			inf.reset();
 		}
 	}
 
-	private static CorruptObjectException corrupt(final DataFormatException dfe) {
-		return new CorruptObjectException(MessageFormat.format(
-				JGitText.get().packfileCorruptionDetected, dfe.getMessage()));
-	}
-
 	private static class DeltaChain extends ObjectId {
 		UnresolvedDelta head;