DeltaStream: Fix data corruption when reading large copies

If the copy instruction was larger than the input buffer given to us,
we copied the wrong part of the base stream during the next read().

This occurred on really big binary files where a copy instruction
of 64k wasn't unreasonable, but the caller's buffer was only 8192
bytes long.  We copied the first 8192 bytes correctly, but then
reseeked the base stream back to the start of the copy region on
the second read of 8192 bytes.  Instead of a sequence like ABCD
being read into the caller, we read AAAA.

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

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/pack/DeltaStreamTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/pack/DeltaStreamTest.java
index 9b34ad5e0973cd44fd62de261f7c416fb3272a95..3f3038b42a8e8b463c20ec9cd29d34e3adf91133 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/pack/DeltaStreamTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/pack/DeltaStreamTest.java
@@ -249,12 +249,29 @@ public class DeltaStreamTest extends TestCase {
                assertEquals(data.length, BinaryDelta.getResultSize(delta));
                assertTrue(Arrays.equals(data, BinaryDelta.apply(base, delta)));
 
+               // Assert that a single bulk read produces the correct result.
+               //
                byte[] act = new byte[data.length];
                DeltaStream in = open();
                assertEquals(data.length, in.getSize());
                assertEquals(data.length, in.read(act));
                assertEquals(-1, in.read());
-               assertTrue(Arrays.equals(data, act));
+               assertTrue("bulk read has same content", Arrays.equals(data, act));
+
+               // Assert that smaller tiny reads have the same result too.
+               //
+               act = new byte[data.length];
+               in = open();
+               int read = 0;
+               while (read < data.length) {
+                       int n = in.read(act, read, 128);
+                       if (n <= 0)
+                               break;
+                       read += n;
+               }
+               assertEquals(data.length, read);
+               assertEquals(-1, in.read());
+               assertTrue("small reads have same content", Arrays.equals(data, act));
        }
 
        private DeltaStream open() throws IOException {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/DeltaStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/DeltaStream.java
index 6f479eb90568544907eb0ea4d7c83d69f6ad3d05..72015d94e88317d0d982d41453dd7bb4a04c7f20 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/DeltaStream.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/pack/DeltaStream.java
@@ -215,7 +215,8 @@ public abstract class DeltaStream extends InputStream {
                                if (n < 0)
                                        throw new CorruptObjectException(
                                                        JGitText.get().baseLengthIncorrect);
-                               baseOffset += n;
+                               copyOffset += n;
+                               baseOffset = copyOffset;
                                break;
 
                        case CMD_INSERT:
@@ -225,6 +226,7 @@ public abstract class DeltaStream extends InputStream {
 
                        case CMD_EOF:
                                return 0 < act ? act : -1;
+
                        default:
                                throw new CorruptObjectException(
                                                JGitText.get().unsupportedCommand0);