From d0b5fa8d0779244c9903fcf12033ce92df941ad4 Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 15 Jun 2019 14:37:08 +0200 Subject: [PATCH 1/8] Rewrote part of keepSkippedBytesThenRead function to get rid of duplicity and while keeping exactly same semantics. --- .../com/jsoniter/IterImplForStreaming.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index a2802cc7..1917ce55 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -274,19 +274,17 @@ public final static boolean loadMore(JsonIterator iter) throws IOException { } private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOException { - int n; - int offset; + int offset = iter.tail - iter.skipStartedAt; + byte[] srcBuffer = iter.buf; + // Double the size of internal buffer + // TODO: Fix NegativeArraySizeException that happens if source stream doesnt return as much + // of output as was requested i.e. when n < iter.buf.length - offset. Anyhow doubling the buffer + // size seems to be pretty dangerous idea and should be either disabled or solved safely. 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); + iter.buf = new byte[iter.buf.length * 2]; } + 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) { From 4504c2e0d63d648f40f7f41867b6f91de3e25e7a Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 15 Jun 2019 14:42:25 +0200 Subject: [PATCH 2/8] Added failing test of loadMore function simulating malfunction mentioned in previous commit. --- .../jsoniter/IterImplForStreamingTest.java | 39 ++++++++++++++++++- .../java/com/jsoniter/suite/AllTestCases.java | 1 + 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/jsoniter/IterImplForStreamingTest.java b/src/test/java/com/jsoniter/IterImplForStreamingTest.java index c190d469..efd0e553 100644 --- a/src/test/java/com/jsoniter/IterImplForStreamingTest.java +++ b/src/test/java/com/jsoniter/IterImplForStreamingTest.java @@ -1,6 +1,10 @@ package com.jsoniter; +import com.jsoniter.any.Any; +import java.io.IOException; +import java.io.InputStream; import junit.framework.TestCase; +import org.junit.experimental.categories.Category; public class IterImplForStreamingTest extends TestCase { @@ -11,4 +15,37 @@ public void testReadMaxDouble() throws Exception { String number = new String(numberChars.chars, 0, numberChars.charsLength); assertEquals(maxDouble, number); } -} \ No newline at end of file + + @Category(StreamingCategory.class) + public void testLoadMore() throws IOException { + final String originalContent = "1234"; + final byte[] src = ("{\"a\":\"" + originalContent + "\"}").getBytes(); + InputStream slowStream = new InputStream() { + int position = 0; + boolean pretendEmptyNextRead = false; + + @Override + public int read() throws IOException { + if (position < src.length) { + if (pretendEmptyNextRead) { + pretendEmptyNextRead = false; + return -1; + } else { + pretendEmptyNextRead = true; + return src[position++]; + } + } + return -1; + } + }; + + // Input must definitely fit into such large buffer + final int initialBufferSize = src.length * 2; + JsonIterator jsonIterator = JsonIterator.parse(slowStream, initialBufferSize); + jsonIterator.readObject(); + Any parsedString = jsonIterator.readAny(); + assertEquals(originalContent, parsedString.toString()); + // Check buffer was not expanded prematurely + assertEquals(initialBufferSize, jsonIterator.buf.length); + } +} diff --git a/src/test/java/com/jsoniter/suite/AllTestCases.java b/src/test/java/com/jsoniter/suite/AllTestCases.java index 70c904c8..d3196ed4 100644 --- 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, From 471ea9bfa29ec42b8632777018f9fdaaaa9a4bed Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 15 Jun 2019 15:55:27 +0200 Subject: [PATCH 3/8] Implemented new mechanism how to automatically expand internal buffer size. This fixes bug #241 and #124 issues. --- .../com/jsoniter/IterImplForStreaming.java | 14 ++-- src/main/java/com/jsoniter/JsonIterator.java | 14 +++- .../jsoniter/IterImplForStreamingTest.java | 70 ++++++++++++++----- 3 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index 1917ce55..f2c8765e 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -276,12 +276,14 @@ public final static boolean loadMore(JsonIterator iter) throws IOException { private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOException { int offset = iter.tail - iter.skipStartedAt; byte[] srcBuffer = iter.buf; - // Double the size of internal buffer - // TODO: Fix NegativeArraySizeException that happens if source stream doesnt return as much - // of output as was requested i.e. when n < iter.buf.length - offset. Anyhow doubling the buffer - // size seems to be pretty dangerous idea and should be either disabled or solved safely. - if (iter.skipStartedAt == 0 || iter.skipStartedAt < iter.tail / 2) { - iter.buf = new byte[iter.buf.length * 2]; + // Check there is no unused buffer capacity + if (iter.buf.length - iter.tail == 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", "buffer is full and autoexpansion is disabled"); + } } System.arraycopy(srcBuffer, iter.skipStartedAt, iter.buf, 0, offset); int n = iter.in.read(iter.buf, offset, iter.buf.length - offset); diff --git a/src/main/java/com/jsoniter/JsonIterator.java b/src/main/java/com/jsoniter/JsonIterator.java index 1f8d077e..c198540b 100644 --- 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 index efd0e553..c940909d 100644 --- a/src/test/java/com/jsoniter/IterImplForStreamingTest.java +++ b/src/test/java/com/jsoniter/IterImplForStreamingTest.java @@ -1,10 +1,12 @@ package com.jsoniter; import com.jsoniter.any.Any; +import com.jsoniter.spi.JsonException; 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 { @@ -18,34 +20,64 @@ public void testReadMaxDouble() throws Exception { @Category(StreamingCategory.class) public void testLoadMore() throws IOException { - final String originalContent = "1234"; + final String originalContent = "1234567890"; final byte[] src = ("{\"a\":\"" + originalContent + "\"}").getBytes(); - InputStream slowStream = new InputStream() { + + 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 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); + } + + private static InputStream getSluggishInputStream(final byte[] src) { + return new InputStream() { int position = 0; - boolean pretendEmptyNextRead = false; @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) { - if (pretendEmptyNextRead) { - pretendEmptyNextRead = false; - return -1; - } else { - pretendEmptyNextRead = true; - return src[position++]; - } + b[off] = src[position++]; + return 1; } return -1; } }; - - // Input must definitely fit into such large buffer - final int initialBufferSize = src.length * 2; - JsonIterator jsonIterator = JsonIterator.parse(slowStream, initialBufferSize); - jsonIterator.readObject(); - Any parsedString = jsonIterator.readAny(); - assertEquals(originalContent, parsedString.toString()); - // Check buffer was not expanded prematurely - assertEquals(initialBufferSize, jsonIterator.buf.length); } } From 442e48ae912d4ab86dd5cc74e2f1a7036bd096e6 Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 15 Jun 2019 17:36:27 +0200 Subject: [PATCH 4/8] Fixed bug with incorrectly computed free buffer capacity. --- src/main/java/com/jsoniter/IterImplForStreaming.java | 7 ++++++- .../java/com/jsoniter/IterImplForStreamingTest.java | 12 +++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index f2c8765e..05e0cce1 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -277,7 +277,7 @@ private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOExce int offset = iter.tail - iter.skipStartedAt; byte[] srcBuffer = iter.buf; // Check there is no unused buffer capacity - if (iter.buf.length - iter.tail == 0) { + 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]; @@ -301,6 +301,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)) { diff --git a/src/test/java/com/jsoniter/IterImplForStreamingTest.java b/src/test/java/com/jsoniter/IterImplForStreamingTest.java index c940909d..e0432d39 100644 --- a/src/test/java/com/jsoniter/IterImplForStreamingTest.java +++ b/src/test/java/com/jsoniter/IterImplForStreamingTest.java @@ -2,6 +2,7 @@ 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; @@ -35,7 +36,7 @@ public void testLoadMore() throws IOException { // Check buffer was not expanded assertEquals(initialBufferSize, jsonIterator.buf.length); - // Case #2: Data does fit into initial buffer, autoresizing off + // Case #2: Data does not fit into initial buffer, autoresizing off initialBufferSize = originalContent.length() / 2; jsonIterator = JsonIterator.parse(getSluggishInputStream(src), initialBufferSize, 0); jsonIterator.readObject(); @@ -59,6 +60,15 @@ public void testLoadMore() throws IOException { 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) { From 27556d0ecb41ebfa7ff358540d2e6e69e56c1e9e Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Fri, 21 Jun 2019 19:01:32 +0200 Subject: [PATCH 5/8] Added debug logging. --- src/main/java/com/jsoniter/IterImplForStreaming.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index 05e0cce1..512a75e0 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -282,7 +282,7 @@ private static boolean keepSkippedBytesThenRead(JsonIterator iter) throws IOExce if (iter.autoExpandBufferStep > 0) { iter.buf = new byte[iter.buf.length + iter.autoExpandBufferStep]; } else { - throw iter.reportError("loadMore", "buffer is full and autoexpansion is disabled"); + 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); From 9b3727cd07c07fb91c88c86ad20a651d2a008982 Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 22 Jun 2019 15:43:33 +0200 Subject: [PATCH 6/8] Fixed typos. --- src/main/java/com/jsoniter/IterImpl.java | 4 ++-- src/main/java/com/jsoniter/IterImplForStreaming.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/jsoniter/IterImpl.java b/src/main/java/com/jsoniter/IterImpl.java index 1cd9515e..ad779fd8 100644 --- 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 index 512a75e0..c36db508 100644 --- 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 From 2a575f876e0c92f846c958e7fc5044bea625df23 Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Sat, 22 Jun 2019 15:44:50 +0200 Subject: [PATCH 7/8] Fixed bug when reading string. --- src/main/java/com/jsoniter/IterImplForStreaming.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index c36db508..f4474f77 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -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; From ba73426ba625cd8727e57562b8b2ab7245ba982a Mon Sep 17 00:00:00 2001 From: Lukas Svoboda Date: Wed, 30 Oct 2019 20:00:54 +0100 Subject: [PATCH 8/8] Fixed parsing zero when streaming is enabled. --- .../com/jsoniter/IterImplForStreaming.java | 3 +-- src/test/java/com/jsoniter/any/TestLong.java | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/jsoniter/IterImplForStreaming.java b/src/main/java/com/jsoniter/IterImplForStreaming.java index f4474f77..2cef3a16 100644 --- a/src/main/java/com/jsoniter/IterImplForStreaming.java +++ b/src/main/java/com/jsoniter/IterImplForStreaming.java @@ -649,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/test/java/com/jsoniter/any/TestLong.java b/src/test/java/com/jsoniter/any/TestLong.java index a5591cd9..247dbe8f 100644 --- a/src/test/java/com/jsoniter/any/TestLong.java +++ b/src/test/java/com/jsoniter/any/TestLong.java @@ -1,5 +1,6 @@ package com.jsoniter.any; +import com.jsoniter.spi.JsonException; import junit.framework.TestCase; public class TestLong extends TestCase { @@ -7,4 +8,21 @@ 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()); + } }