IO: fix IO.readFully(InputStream, byte[], int)

This would run into an endless loop if the offset given was not zero.
Fix the logic to exit the read loop when the buffer is full.

Luckily all existing uses of this method call it only with offset zero.

Change-Id: I0ec2a4fb43efe4a605d06ac2e88cf155d50e2f1e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

2 files changed, 87 insertions, 3 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/IOTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/IOTest.java
new file mode 100644
index 0000000000..10a858fbfb
--- /dev/null
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/IOTest.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2021, Thomas Wolf <thomas.wolf@paranor.ch> and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.util;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+import org.junit.Test;
+
+public class IOTest {
+
+	private static final byte[] DATA = "abcdefghijklmnopqrstuvwxyz"
+			.getBytes(StandardCharsets.US_ASCII);
+
+	private byte[] initBuffer(int size) {
+		byte[] buffer = new byte[size];
+		for (int i = 0; i < size; i++) {
+			buffer[i] = (byte) ('0' + (i % 10));
+		}
+		return buffer;
+	}
+
+	private int read(byte[] buffer, int from) throws IOException {
+		try (InputStream in = new ByteArrayInputStream(DATA)) {
+			return IO.readFully(in, buffer, from);
+		}
+	}
+
+	@Test
+	public void readFullyBufferShorter() throws Exception {
+		byte[] buffer = initBuffer(9);
+		int length = read(buffer, 0);
+		assertEquals(buffer.length, length);
+		assertArrayEquals(buffer, Arrays.copyOfRange(DATA, 0, length));
+	}
+
+	@Test
+	public void readFullyBufferLonger() throws Exception {
+		byte[] buffer = initBuffer(50);
+		byte[] initial = Arrays.copyOf(buffer, buffer.length);
+		int length = read(buffer, 0);
+		assertEquals(DATA.length, length);
+		assertArrayEquals(Arrays.copyOfRange(buffer, 0, length), DATA);
+		assertArrayEquals(Arrays.copyOfRange(buffer, length, buffer.length),
+				Arrays.copyOfRange(initial, length, initial.length));
+	}
+
+	@Test
+	public void readFullyBufferShorterOffset() throws Exception {
+		byte[] buffer = initBuffer(9);
+		byte[] initial = Arrays.copyOf(buffer, buffer.length);
+		int length = read(buffer, 6);
+		assertEquals(3, length);
+		assertArrayEquals(Arrays.copyOfRange(buffer, 0, 6),
+				Arrays.copyOfRange(initial, 0, 6));
+		assertArrayEquals(Arrays.copyOfRange(buffer, 6, buffer.length),
+				Arrays.copyOfRange(DATA, 0, 3));
+	}
+
+	@Test
+	public void readFullyBufferLongerOffset() throws Exception {
+		byte[] buffer = initBuffer(50);
+		byte[] initial = Arrays.copyOf(buffer, buffer.length);
+		int length = read(buffer, 40);
+		assertEquals(10, length);
+		assertArrayEquals(Arrays.copyOfRange(buffer, 0, 40),
+				Arrays.copyOfRange(initial, 0, 40));
+		assertArrayEquals(Arrays.copyOfRange(buffer, 40, buffer.length),
+				Arrays.copyOfRange(DATA, 0, 10));
+	}
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/IO.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/IO.java
index 680d90392d..6d5694e435 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/IO.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/IO.java
@@ -246,7 +246,7 @@ public class IO {
 	 *            buffer that must be fully populated, [off, off+len).
 	 * @param off
 	 *            position within the buffer to start writing to.
-	 * @return number of bytes in buffer or stream, whichever is shortest
+	 * @return number of bytes read
 	 * @throws java.io.IOException
 	 *             there was an error reading from the stream.
 	 */
@@ -254,8 +254,8 @@ public class IO {
 			throws IOException {
 		int r;
 		int len = 0;
-		while ((r = fd.read(dst, off, dst.length - off)) >= 0
-				&& len < dst.length) {
+		while (off < dst.length
+				&& (r = fd.read(dst, off, dst.length - off)) >= 0) {
 			off += r;
 			len += r;
 		}