From 741659fed4e73b71693b408e137804b17e65f0cb Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 6 Sep 2010 10:06:37 -0700 Subject: [PATCH] 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 --- .../jgit/storage/pack/DeltaStreamTest.java | 19 ++++++++++++++++++- .../jgit/storage/pack/DeltaStream.java | 4 +++- 2 files changed, 21 insertions(+), 2 deletions(-) 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 9b34ad5e0..3f3038b42 100644 --- 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 @@ private void assertValidState() throws IOException { 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 6f479eb90..72015d94e 100644 --- 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 int read(byte[] buf, int off, int len) throws IOException { if (n < 0) throw new CorruptObjectException( JGitText.get().baseLengthIncorrect); - baseOffset += n; + copyOffset += n; + baseOffset = copyOffset; break; case CMD_INSERT: @@ -225,6 +226,7 @@ public int read(byte[] buf, int off, int len) throws IOException { case CMD_EOF: return 0 < act ? act : -1; + default: throw new CorruptObjectException( JGitText.get().unsupportedCommand0);