Merge pull request #243 from svobol13/master

svobol13 · web-flow · commit 9928ac55ba05 · 2022-01-20T21:42:27.000+01:00
Fixed leaky buffer when streaming.
diff --git a/src/main/java/com/jsoniter/IterImpl.java b/src/main/java/com/jsoniter/IterImpl.java
@@ -59,7 +59,7 @@ final static void skipArray(JsonIterator iter) throws IOException {
                 case '[': // If open symbol, increase level
                     level++;
                     break;
-                case ']': // If close symbol, increase level
+                case ']': // If close symbol, decrease level
                     level--;
 
                     // If we have returned to the original level, we're done
@@ -85,7 +85,7 @@ final static void skipObject(JsonIterator iter) throws IOException {
                 case '{': // If open symbol, increase level
                     level++;
                     break;
-                case '}': // If close symbol, increase level
+                case '}': // If close symbol, decrease level
                     level--;
 
                     // If we have returned to the original level, we're done
diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java
@@ -71,7 +71,7 @@ final static void skipArray(JsonIterator iter) throws IOException {
                     case '[': // If open symbol, increase level
                         level++;
                         break;
-                    case ']': // If close symbol, increase level
+                    case ']': // If close symbol, decrease level
                         level--;
 
                         // If we have returned to the original level, we're done
@@ -101,7 +101,7 @@ final static void skipObject(JsonIterator iter) throws IOException {
                     case '{': // If open symbol, increase level
                         level++;
                         break;
-                    case '}': // If close symbol, increase level
+                    case '}': // If close symbol, decrease level
                         level--;
 
                         // If we have returned to the original level, we're done
@@ -147,7 +147,8 @@ final static void skipString(JsonIterator iter) throws IOException {
                     throw iter.reportError("skipString", "incomplete string");
                 }
                 if (escaped) {
-                    iter.head = 1; // skip the first char as last char is \
+                    // TODO add unit test to prove/verify bug
+                    iter.head += 1; // skip the first char as last char is \
                 }
             } else {
                 iter.head = end;
@@ -274,19 +275,19 @@ public final static boolean loadMore(JsonIterator iter) throws IOException {
     }
 
     private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOException {
-        int n;
-        int offset;
-        if (iter.skipStartedAt == 0 || iter.skipStartedAt < iter.tail / 2) {
-            byte[] newBuf = new byte[iter.buf.length * 2];
-            offset = iter.tail - iter.skipStartedAt;
-            System.arraycopy(iter.buf, iter.skipStartedAt, newBuf, 0, offset);
-            iter.buf = newBuf;
-            n = iter.in.read(iter.buf, offset, iter.buf.length - offset);
-        } else {
-            offset = iter.tail - iter.skipStartedAt;
-            System.arraycopy(iter.buf, iter.skipStartedAt, iter.buf, 0, offset);
-            n = iter.in.read(iter.buf, offset, iter.buf.length - offset);
-        }
+        int offset = iter.tail - iter.skipStartedAt;
+        byte[] srcBuffer = iter.buf;
+        // Check there is no unused buffer capacity
+        if ((getUnusedBufferByteCount(iter)) == 0) {
+          // If auto expand buffer enabled, then create larger buffer
+          if (iter.autoExpandBufferStep > 0) {
+            iter.buf = new byte[iter.buf.length + iter.autoExpandBufferStep];
+          } else {
+            throw iter.reportError("loadMore", String.format("buffer is full and autoexpansion is disabled. tail: [%s] skipStartedAt: [%s]", iter.tail, iter.skipStartedAt));
+          }
+        }
+        System.arraycopy(srcBuffer, iter.skipStartedAt, iter.buf, 0, offset);
+        int n = iter.in.read(iter.buf, offset, iter.buf.length - offset);
         iter.skipStartedAt = 0;
         if (n < 1) {
             if (n == -1) {
@@ -301,6 +302,11 @@ private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOExce
         return true;
     }
 
+    private static int getUnusedBufferByteCount(JsonIterator iter) {
+        // Get bytes from 0 to skipStart + from tail till end
+        return iter.buf.length - iter.tail + iter.skipStartedAt;
+    }
+
     final static byte readByte(JsonIterator iter) throws IOException {
         if (iter.head == iter.tail) {
             if (!loadMore(iter)) {
@@ -643,8 +649,7 @@ static final int readInt(final JsonIterator iter, final byte c) throws IOExcepti
 
     static void assertNotLeadingZero(JsonIterator iter) throws IOException {
         try {
-            byte nextByte = IterImpl.readByte(iter);
-            iter.unreadByte();
+            byte nextByte = iter.buf[iter.head];
             int ind2 = IterImplNumber.intDigits[nextByte];
             if (ind2 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
                 return;
diff --git a/src/main/java/com/jsoniter/JsonIterator.java b/src/main/java/com/jsoniter/JsonIterator.java
@@ -21,6 +21,9 @@ public class JsonIterator implements Closeable {
     final static ValueType[] valueTypes = new ValueType[256];
     InputStream in;
     byte[] buf;
+    // Whenever buf is not large enough new one is created with size of
+    // buf.length + autoExpandBufferStep. Set to < 1 to disable auto expanding.
+    int autoExpandBufferStep;
     int head;
     int tail;
     int skipStartedAt = -1; // skip should keep bytes starting at this pos
@@ -60,13 +63,22 @@ private JsonIterator(InputStream in, byte[] buf, int head, int tail) {
         this.tail = tail;
     }
 
+    private JsonIterator(InputStream in, byte[] buf, int autoExpandBufferStep) {
+        this(in, buf, 0, 0);
+        this.autoExpandBufferStep = autoExpandBufferStep;
+    }
+
     public JsonIterator() {
         this(null, new byte[0], 0, 0);
     }
 
     public static JsonIterator parse(InputStream in, int bufSize) {
+        return parse(in, bufSize, bufSize);
+    }
+
+    public static JsonIterator parse(InputStream in, int bufSize, int autoExpandBufferStep) {
         enableStreamingSupport();
-        return new JsonIterator(in, new byte[bufSize], 0, 0);
+        return new JsonIterator(in, new byte[bufSize], autoExpandBufferStep);
     }
 
     public static JsonIterator parse(byte[] buf) {
diff --git a/src/test/java/com/jsoniter/IterImplForStreamingTest.java b/src/test/java/com/jsoniter/IterImplForStreamingTest.java
@@ -1,6 +1,13 @@
 package com.jsoniter;
 
+import com.jsoniter.any.Any;
+import com.jsoniter.spi.JsonException;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
 import junit.framework.TestCase;
+import org.junit.experimental.categories.Category;
+import sun.reflect.generics.reflectiveObjects.NotImplementedException;
 
 public class IterImplForStreamingTest extends TestCase {
 
@@ -11,4 +18,76 @@ public void testReadMaxDouble() throws Exception {
 		String number = new String(numberChars.chars, 0, numberChars.charsLength);
 		assertEquals(maxDouble, number);
 	}
-}
+
+	@Category(StreamingCategory.class)
+	public void testLoadMore() throws IOException {
+		final String originalContent = "1234567890";
+		final byte[] src = ("{\"a\":\"" + originalContent + "\"}").getBytes();
+
+		int initialBufferSize;
+		Any parsedString;
+		// Case #1: Data fits into initial buffer, autoresizing on
+		// Input must definitely fit into such large buffer
+		initialBufferSize = src.length * 2;
+		JsonIterator jsonIterator = JsonIterator.parse(getSluggishInputStream(src), initialBufferSize, 512);
+		jsonIterator.readObject();
+		parsedString = jsonIterator.readAny();
+		assertEquals(originalContent, parsedString.toString());
+		// Check buffer was not expanded
+		assertEquals(initialBufferSize, jsonIterator.buf.length);
+
+		// Case #2: Data does not fit into initial buffer, autoresizing off
+		initialBufferSize = originalContent.length() / 2;
+		jsonIterator = JsonIterator.parse(getSluggishInputStream(src), initialBufferSize, 0);
+		jsonIterator.readObject();
+		try {
+			jsonIterator.readAny();
+			fail("Expect to fail because buffer is too small.");
+		} catch (JsonException e) {
+			if (!e.getMessage().startsWith("loadMore")) {
+				throw e;
+			}
+		}
+		// Check buffer was not expanded
+		assertEquals(initialBufferSize, jsonIterator.buf.length);
+
+		// Case #3: Data does fit into initial buffer, autoresizing on
+		initialBufferSize = originalContent.length() / 2;
+		int autoExpandBufferStep = initialBufferSize * 3;
+		jsonIterator = JsonIterator.parse(getSluggishInputStream(src), initialBufferSize, autoExpandBufferStep);
+		jsonIterator.readObject();
+		parsedString = jsonIterator.readAny();
+		assertEquals(originalContent, parsedString.toString());
+		// Check buffer was expanded exactly once
+		assertEquals(initialBufferSize + autoExpandBufferStep, jsonIterator.buf.length);
+
+		// Case #4: Data does not fit (but largest string does) into initial buffer, autoresizing on
+		initialBufferSize = originalContent.length() + 2;
+		jsonIterator = JsonIterator.parse(new ByteArrayInputStream(src), initialBufferSize, 0);
+		jsonIterator.readObject();
+		parsedString = jsonIterator.readAny();
+		assertEquals(originalContent, parsedString.toString());
+		// Check buffer was expanded exactly once
+		assertEquals(initialBufferSize, jsonIterator.buf.length);
+	}
+
+	private static InputStream getSluggishInputStream(final byte[] src) {
+		return new InputStream() {
+			int position = 0;
+
+			@Override
+			public int read() throws IOException {
+				throw new NotImplementedException();
+			}
+
+			@Override
+			public int read(byte[] b, int off, int len) throws IOException {
+				if (position < src.length) {
+					b[off] = src[position++];
+					return 1;
+				}
+				return -1;
+			}
+		};
+	}
+}
diff --git a/src/test/java/com/jsoniter/any/TestLong.java b/src/test/java/com/jsoniter/any/TestLong.java
@@ -1,10 +1,28 @@
 package com.jsoniter.any;
 
+import com.jsoniter.spi.JsonException;
 import junit.framework.TestCase;
 
 public class TestLong extends TestCase {
     public void test_to_string_should_trim() {
         Any any = Any.lazyLong(" 1000".getBytes(), 0, " 1000".length());
         assertEquals("1000", any.toString());
     }
+
+    public void test_should_fail_with_leading_zero() {
+        byte[] bytes = "01".getBytes();
+        Any any = Any.lazyLong(bytes, 0, bytes.length);
+        try {
+            any.toLong();
+            fail("This should fail.");
+        } catch (JsonException e) {
+
+        }
+    }
+
+    public void test_should_work_with_zero() {
+        byte[] bytes = "0".getBytes();
+        Any any = Any.lazyLong(bytes, 0, bytes.length);
+        assertEquals(0L, any.toLong());
+    }
 }
diff --git a/src/test/java/com/jsoniter/suite/AllTestCases.java b/src/test/java/com/jsoniter/suite/AllTestCases.java
@@ -53,6 +53,7 @@
         TestGson.class,
         com.jsoniter.output.TestGson.class,
         TestStreamBuffer.class,
+        IterImplForStreamingTest.class,
         TestCollection.class,
         TestList.class,
         TestAnnotationJsonObject.class,
