ReceivePack supports InputStream data after pack

When receiving a pack, data buffered after the pack can restored
to the InputStream if the stream supports mark and reset.

Change-Id: If04915c32c91be28db8df7e8491ed3e9fe0e1608

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PackParserTest.java

@@ -304,6 +304,154 @@ public class PackParserTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void testNonMarkingInputStream() throws Exception {
+		TestRepository d = new TestRepository(db);
+		RevBlob a = d.blob("a");
+
+		TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024);
+		packHeader(pack, 1);
+		pack.write((Constants.OBJ_REF_DELTA) << 4 | 4);
+		a.copyRawTo(pack);
+		deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' });
+		digest(pack);
+
+		InputStream in = new ByteArrayInputStream(pack.toByteArray()) {
+			@Override
+			public boolean markSupported() {
+				return false;
+			}
+
+			@Override
+			public void mark(int maxlength) {
+				fail("Mark should not be called");
+			}
+		};
+
+		PackParser p = index(in);
+		p.setAllowThin(true);
+		p.setCheckEofAfterPackFooter(false);
+		p.setExpectDataAfterPackFooter(true);
+
+		try {
+			p.parse(NullProgressMonitor.INSTANCE);
+			fail("PackParser should have failed");
+		} catch (IOException e) {
+			assertEquals(e.getMessage(),
+					JGitText.get().inputStreamMustSupportMark);
+		}
+	}
+
+	@Test
+	public void testDataAfterPackFooterSingleRead() throws Exception {
+		TestRepository d = new TestRepository(db);
+		RevBlob a = d.blob("a");
+
+		TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(32*1024);
+		packHeader(pack, 1);
+		pack.write((Constants.OBJ_REF_DELTA) << 4 | 4);
+		a.copyRawTo(pack);
+		deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' });
+		digest(pack);
+
+		byte packData[] = pack.toByteArray();
+		byte streamData[] = new byte[packData.length + 1];
+		System.arraycopy(packData, 0, streamData, 0, packData.length);
+		streamData[packData.length] = 0x7e;
+
+		InputStream in = new ByteArrayInputStream(streamData);
+		PackParser p = index(in);
+		p.setAllowThin(true);
+		p.setCheckEofAfterPackFooter(false);
+		p.setExpectDataAfterPackFooter(true);
+
+		p.parse(NullProgressMonitor.INSTANCE);
+
+		assertEquals(0x7e, in.read());
+	}
+
+	@Test
+	public void testDataAfterPackFooterSplitObjectRead() throws Exception {
+		final byte[] data = Constants.encode("0123456789");
+
+		// Build a pack ~17k
+		int objects = 900;
+		TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(32 * 1024);
+		packHeader(pack, objects);
+
+		for (int i = 0; i < objects; i++) {
+			pack.write((Constants.OBJ_BLOB) << 4 | 10);
+			deflate(pack, data);
+		}
+		digest(pack);
+
+		byte packData[] = pack.toByteArray();
+		byte streamData[] = new byte[packData.length + 1];
+		System.arraycopy(packData, 0, streamData, 0, packData.length);
+		streamData[packData.length] = 0x7e;
+		InputStream in = new ByteArrayInputStream(streamData);
+		PackParser p = index(in);
+		p.setAllowThin(true);
+		p.setCheckEofAfterPackFooter(false);
+		p.setExpectDataAfterPackFooter(true);
+
+		p.parse(NullProgressMonitor.INSTANCE);
+
+		assertEquals(0x7e, in.read());
+	}
+
+	@Test
+	public void testDataAfterPackFooterSplitHeaderRead() throws Exception {
+		TestRepository d = new TestRepository(db);
+		final byte[] data = Constants.encode("a");
+		RevBlob b = d.blob(data);
+
+		int objects = 248;
+		TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(32 * 1024);
+		packHeader(pack, objects + 1);
+		int offset = 13;
+		StringBuilder sb = new StringBuilder();
+		for (int i = 0; i < offset; i++)
+			sb.append(i);
+		offset = sb.toString().length();
+		int lenByte = (Constants.OBJ_BLOB) << 4 | (offset & 0x0F);
+		offset >>= 4;
+		if (offset > 0)
+			lenByte |= 1 << 7;
+		pack.write(lenByte);
+		while (offset > 0) {
+			lenByte = offset & 0x7F;
+			offset >>= 6;
+			if (offset > 0)
+				lenByte |= 1 << 7;
+			pack.write(lenByte);
+		}
+		deflate(pack, Constants.encode(sb.toString()));
+
+		for (int i = 0; i < objects; i++) {
+			// The last pack header written falls across the 8192 byte boundary
+			// between [8189:8210]
+			pack.write((Constants.OBJ_REF_DELTA) << 4 | 4);
+			b.copyRawTo(pack);
+			deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' });
+		}
+		digest(pack);
+
+		byte packData[] = pack.toByteArray();
+		byte streamData[] = new byte[packData.length + 1];
+		System.arraycopy(packData, 0, streamData, 0, packData.length);
+		streamData[packData.length] = 0x7e;
+		InputStream in = new ByteArrayInputStream(streamData);
+		PackParser p = index(in);
+		p.setAllowThin(true);
+		p.setCheckEofAfterPackFooter(false);
+		p.setExpectDataAfterPackFooter(true);
+
+		p.parse(NullProgressMonitor.INSTANCE);
+
+		assertEquals(0x7e, in.read());
+	}
+
 	private void packHeader(TemporaryBuffer.Heap tinyPack, int cnt)
 			throws IOException {
 		final byte[] hdr = new byte[8];

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

@@ -224,6 +224,7 @@ indexFileIsTooLargeForJgit=Index file is too large for jgit
 indexSignatureIsInvalid=Index signature is invalid: {0}
 indexWriteException=Modified index could not be written
 inMemoryBufferLimitExceeded=In-memory buffer limit exceeded
+inputStreamMustSupportMark=InputStream must support mark()
 integerValueOutOfRange=Integer value {0}.{1} out of range
 internalRevisionError=internal revision error
 internalServerError=internal server error

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

@@ -284,6 +284,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String indexSignatureIsInvalid;
 	/***/ public String indexWriteException;
 	/***/ public String inMemoryBufferLimitExceeded;
+	/***/ public String inputStreamMustSupportMark;
 	/***/ public String integerValueOutOfRange;
 	/***/ public String internalRevisionError;
 	/***/ public String internalServerError;

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

@@ -154,6 +154,9 @@ public abstract class BaseReceivePack {
 	 */
 	protected boolean biDirectionalPipe = true;
 
+	/** Expecting data after the pack footer */
+	protected boolean expectDataAfterPackFooter;
+
 	/** Should an incoming transfer validate objects? */
 	protected boolean checkReceivedObjects;
 
@@ -454,6 +457,19 @@ public abstract class BaseReceivePack {
 		biDirectionalPipe = twoWay;
 	}
 
+	/** @return true if there is data expected after the pack footer. */
+	public boolean isExpectDataAfterPackFooter() {
+		return expectDataAfterPackFooter;
+	}
+
+	/**
+	 * @param e
+	 *            true if there is additional data in InputStream after pack.
+	 */
+	public void setExpectDataAfterPackFooter(boolean e) {
+		expectDataAfterPackFooter = e;
+	}
+
 	/**
 	 * @return true if this instance will verify received objects are formatted
 	 *         correctly. Validating objects requires more CPU time on this side
@@ -909,6 +925,7 @@ public abstract class BaseReceivePack {
 			parser.setNeedNewObjectIds(checkReferencedIsReachable);
 			parser.setNeedBaseObjectIds(checkReferencedIsReachable);
 			parser.setCheckEofAfterPackFooter(!biDirectionalPipe);
+			parser.setExpectDataAfterPackFooter(isExpectDataAfterPackFooter());
 			parser.setObjectChecking(isCheckReceivedObjects());
 			parser.setLockMessage(lockMsg);
 			parser.setMaxObjectSizeLimit(maxObjectSizeLimit);

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

@@ -141,6 +141,8 @@ public abstract class PackParser {
 
 	private boolean checkEofAfterPackFooter;
 
+	private boolean expectDataAfterPackFooter;
+
 	private long objectCount;
 
 	private PackedObjectInfo[] entries;
@@ -305,6 +307,21 @@ public abstract class PackParser {
 		checkEofAfterPackFooter = b;
 	}
 
+	/** @return true if there is data expected after the pack footer. */
+	public boolean isExpectDataAfterPackFooter() {
+		return expectDataAfterPackFooter;
+	}
+
+	/**
+	 * @param e
+	 *            true if there is additional data in InputStream after pack.
+	 *            This requires the InputStream to support the mark and reset
+	 *            functions.
+	 */
+	public void setExpectDataAfterPackFooter(boolean e) {
+		expectDataAfterPackFooter = e;
+	}
+
 	/** @return the new objects that were sent by the user */
 	public ObjectIdSubclassMap<ObjectId> getNewObjectIds() {
 		if (newObjectIds != null)
@@ -826,6 +843,13 @@ public abstract class PackParser {
 	}
 
 	private void readPackHeader() throws IOException {
+		if (expectDataAfterPackFooter) {
+			if (!in.markSupported())
+				throw new IOException(
+						JGitText.get().inputStreamMustSupportMark);
+			in.mark(buf.length);
+		}
+
 		final int hdrln = Constants.PACK_SIGNATURE.length + 4 + 4;
 		final int p = fill(Source.INPUT, hdrln);
 		for (int k = 0; k < Constants.PACK_SIGNATURE.length; k++)
@@ -851,23 +875,19 @@ public abstract class PackParser {
 		System.arraycopy(buf, c, srcHash, 0, 20);
 		use(20);
 
-		// The input stream should be at EOF at this point. We do not support
-		// yielding back any remaining buffered data after the pack footer, so
-		// protocols that embed a pack stream are required to either end their
-		// stream with the pack, or embed the pack with a framing system like
-		// the SideBandInputStream does.
-
-		if (bAvail != 0)
+		if (bAvail != 0 && !expectDataAfterPackFooter)
 			throw new CorruptObjectException(MessageFormat.format(
 					JGitText.get().expectedEOFReceived,
 					"\\x" + Integer.toHexString(buf[bOffset] & 0xff)));
-
 		if (isCheckEofAfterPackFooter()) {
 			int eof = in.read();
 			if (0 <= eof)
 				throw new CorruptObjectException(MessageFormat.format(
 						JGitText.get().expectedEOFReceived,
 						"\\x" + Integer.toHexString(eof)));
+		} else if (bAvail > 0 && expectDataAfterPackFooter) {
+			in.reset();
+			IO.skipFully(in, bOffset);
 		}
 
 		if (!Arrays.equals(actHash, srcHash))
@@ -1142,7 +1162,14 @@ public abstract class PackParser {
 	private void sync() throws IOException {
 		packDigest.update(buf, 0, bOffset);
 		onStoreStream(buf, 0, bOffset);
-		if (bAvail > 0)
+		if (expectDataAfterPackFooter) {
+			if (bAvail > 0) {
+				in.reset();
+				IO.skipFully(in, bOffset);
+				bAvail = 0;
+			}
+			in.mark(buf.length);
+		} else if (bAvail > 0)
 			System.arraycopy(buf, bOffset, buf, 0, bAvail);
 		bBase += bOffset;
 		bOffset = 0;