Fix bug in skipRawBytes if request is larger than buffer.

Original solution by Scott Barta this change adds tests
and also fixes the same bug that was in CodedInputStream.

Change-Id: Idb49691822b3f292c5092edc52db4e153e9da49a

4 files changed, 95 insertions, 2 deletions

diff --git a/java/src/main/java/com/google/protobuf/CodedInputStream.java b/java/src/main/java/com/google/protobuf/CodedInputStream.java
index ad43f96..22995e9 100644
--- a/java/src/main/java/com/google/protobuf/CodedInputStream.java
+++ b/java/src/main/java/com/google/protobuf/CodedInputStream.java
@@ -847,7 +847,7 @@ public final class CodedInputStream {
     } else {
       // Skipping more bytes than are in the buffer.  First skip what we have.
       int pos = bufferSize - bufferPos;
-      totalBytesRetired += pos;
+      totalBytesRetired += bufferSize;
       bufferPos = 0;
       bufferSize = 0;
 
diff --git a/java/src/main/java/com/google/protobuf/micro/CodedInputStreamMicro.java b/java/src/main/java/com/google/protobuf/micro/CodedInputStreamMicro.java
index 0791b8f..3e3926c 100644
--- a/java/src/main/java/com/google/protobuf/micro/CodedInputStreamMicro.java
+++ b/java/src/main/java/com/google/protobuf/micro/CodedInputStreamMicro.java
@@ -786,7 +786,7 @@ public final class CodedInputStreamMicro {
     } else {
       // Skipping more bytes than are in the buffer.  First skip what we have.
       int pos = bufferSize - bufferPos;
-      totalBytesRetired += pos;
+      totalBytesRetired += bufferSize;
       bufferPos = 0;
       bufferSize = 0;
 
diff --git a/java/src/test/java/com/google/protobuf/CodedInputStreamTest.java b/java/src/test/java/com/google/protobuf/CodedInputStreamTest.java
index 6acd322..33d60d1 100644
--- a/java/src/test/java/com/google/protobuf/CodedInputStreamTest.java
+++ b/java/src/test/java/com/google/protobuf/CodedInputStreamTest.java
@@ -325,6 +325,32 @@ public class CodedInputStreamTest extends TestCase {
     assertEquals(2, input.readRawByte());
   }
 
+  /**
+   * Test that a bug in skipRawBytes() has been fixed:  if the skip skips
+   * past the end of a buffer with a limit that has been set past the end of
+   * that buffer, this should not break things.
+   */
+  public void testSkipRawBytesPastEndOfBufferWithLimit() throws Exception {
+    System.out.printf("testSkipRawBytesPastEndOfBufferWithLimit: E ...\n");
+
+    byte[] rawBytes = new byte[] { 1, 2, 3, 4, 5 };
+    CodedInputStream input = CodedInputStream.newInstance(
+        new SmallBlockInputStream(rawBytes, 3));
+
+    int limit = input.pushLimit(4);
+    System.out.printf("testSkipRawBytesPastEndOfBufferWithLimit: limit=%d\n", limit);
+    // In order to expose the bug we need to read at least one byte to prime the
+    // buffer inside the CodedInputStream.
+    assertEquals(1, input.readRawByte());
+    // Skip to the end of the limit.
+    input.skipRawBytes(3);
+    assertTrue(input.isAtEnd());
+    input.popLimit(limit);
+    assertEquals(5, input.readRawByte());
+
+    System.out.printf("testSkipRawBytesPastEndOfBufferWithLimit: X ...\n");
+  }
+
   public void testReadHugeBlob() throws Exception {
     // Allocate and initialize a 1MB blob.
     byte[] blob = new byte[1 << 20];
diff --git a/java/src/test/java/com/google/protobuf/MicroTest.java b/java/src/test/java/com/google/protobuf/MicroTest.java
index 12533ac..afca135 100644
--- a/java/src/test/java/com/google/protobuf/MicroTest.java
+++ b/java/src/test/java/com/google/protobuf/MicroTest.java
@@ -37,9 +37,15 @@ import com.google.protobuf.micro.SimpleMessageMicro;
 import com.google.protobuf.micro.StringUtf8;
 import com.google.protobuf.micro.UnittestImportMicro;
 import com.google.protobuf.micro.ByteStringMicro;
+import com.google.protobuf.micro.CodedInputStreamMicro;
 
 import junit.framework.TestCase;
 
+import java.io.ByteArrayInputStream;
+import java.io.FilterInputStream;
+import java.io.InputStream;
+import java.io.IOException;
+
 /**
  * Test micro runtime.
  *
@@ -2134,4 +2140,65 @@ public class MicroTest extends TestCase {
     assertFalse(msg.hasDefaultImportEnum());
     assertEquals(UnittestImportMicro.IMPORT_MICRO_BAR, msg.getDefaultImportEnum());
   }
+
+  /**
+   * Test that a bug in skipRawBytes() has been fixed:  if the skip skips
+   * exactly up to a limit, this should not break things.
+   */
+  public void testSkipRawBytesBug() throws Exception {
+    byte[] rawBytes = new byte[] { 1, 2 };
+    CodedInputStreamMicro input = CodedInputStreamMicro.newInstance(rawBytes);
+
+    int limit = input.pushLimit(1);
+    input.skipRawBytes(1);
+    input.popLimit(limit);
+    assertEquals(2, input.readRawByte());
+  }
+
+  /**
+   * Test that a bug in skipRawBytes() has been fixed:  if the skip skips
+   * past the end of a buffer with a limit that has been set past the end of
+   * that buffer, this should not break things.
+   */
+  public void testSkipRawBytesPastEndOfBufferWithLimit() throws Exception {
+    byte[] rawBytes = new byte[] { 1, 2, 3, 4, 5 };
+    CodedInputStreamMicro input = CodedInputStreamMicro.newInstance(
+        new SmallBlockInputStream(rawBytes, 3));
+
+    int limit = input.pushLimit(4);
+    // In order to expose the bug we need to read at least one byte to prime the
+    // buffer inside the CodedInputStream.
+    assertEquals(1, input.readRawByte());
+    // Skip to the end of the limit.
+    input.skipRawBytes(3);
+    assertTrue(input.isAtEnd());
+    input.popLimit(limit);
+    assertEquals(5, input.readRawByte());
+  }
+
+  /**
+   * An InputStream which limits the number of bytes it reads at a time.
+   * We use this to make sure that CodedInputStream doesn't screw up when
+   * reading in small blocks.
+   */
+  private static final class SmallBlockInputStream extends FilterInputStream {
+    private final int blockSize;
+
+    public SmallBlockInputStream(byte[] data, int blockSize) {
+      this(new ByteArrayInputStream(data), blockSize);
+    }
+
+    public SmallBlockInputStream(InputStream in, int blockSize) {
+      super(in);
+      this.blockSize = blockSize;
+    }
+
+    public int read(byte[] b) throws IOException {
+      return super.read(b, 0, Math.min(b.length, blockSize));
+    }
+
+    public int read(byte[] b, int off, int len) throws IOException {
+      return super.read(b, off, Math.min(len, blockSize));
+    }
+  }
 }