Skip to content

fix of overflow detection for numeric primitive types #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 25, 2017
Merged

fix of overflow detection for numeric primitive types #121

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
518e1b7
fix of overflow detection for numeric primitive types
plokhotnyuk Oct 23, 2017
5cf4dae
fix of method prefix of error message
plokhotnyuk Oct 24, 2017
e85ebd7
fix of parse error message
plokhotnyuk Oct 24, 2017
48aed51
code clean up
plokhotnyuk Oct 24, 2017
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 45 additions & 30 deletions src/main/java/com/jsoniter/IterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,123 +323,136 @@ public static int updateStringCopyBound(final JsonIterator iter, final int bound
return bound;
}

static final int readPositiveInt(final JsonIterator iter, byte c) throws IOException {
static final int readInt(final JsonIterator iter, final byte c, final boolean negative) throws IOException {
int ind = IterImplNumber.intDigits[c];
if (ind == 0) {
IterImplForStreaming.assertNotLeadingZero(iter);
return 0;
}
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
throw iter.reportError("readPositiveInt", "expect 0~9");
throw iter.reportError("readInt", "expect 0~9");
}
if (iter.tail - iter.head > 9) {
int i = iter.head;
int ind2 = IterImplNumber.intDigits[iter.buf[i]];
if (ind2 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind;
return negative ? -ind : ind;
}
int ind3 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind3 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 10 + ind2;
ind = ind * 10 + ind2;
return negative ? -ind : ind;
}
int ind4 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind4 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 100 + ind2 * 10 + ind3;
ind = ind * 100 + ind2 * 10 + ind3;
return negative ? -ind : ind;
}
int ind5 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind5 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 1000 + ind2 * 100 + ind3 * 10 + ind4;
ind = ind * 1000 + ind2 * 100 + ind3 * 10 + ind4;
return negative ? -ind : ind;
}
int ind6 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind6 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 10000 + ind2 * 1000 + ind3 * 100 + ind4 * 10 + ind5;
ind = ind * 10000 + ind2 * 1000 + ind3 * 100 + ind4 * 10 + ind5;
return negative ? -ind : ind;
}
int ind7 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind7 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 100000 + ind2 * 10000 + ind3 * 1000 + ind4 * 100 + ind5 * 10 + ind6;
ind = ind * 100000 + ind2 * 10000 + ind3 * 1000 + ind4 * 100 + ind5 * 10 + ind6;
return negative ? -ind : ind;
}
int ind8 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind8 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 1000000 + ind2 * 100000 + ind3 * 10000 + ind4 * 1000 + ind5 * 100 + ind6 * 10 + ind7;
ind = ind * 1000000 + ind2 * 100000 + ind3 * 10000 + ind4 * 1000 + ind5 * 100 + ind6 * 10 + ind7;
return negative ? -ind : ind;
}
int ind9 = IterImplNumber.intDigits[iter.buf[++i]];
ind = ind * 10000000 + ind2 * 1000000 + ind3 * 100000 + ind4 * 10000 + ind5 * 1000 + ind6 * 100 + ind7 * 10 + ind8;
iter.head = i;
if (ind9 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
return ind;
return negative ? -ind : ind;
}
}
return IterImplForStreaming.readIntSlowPath(iter, ind);
return IterImplForStreaming.readIntSlowPath(iter, ind, negative);
}

static final long readPositiveLong(final JsonIterator iter, byte c) throws IOException {
static final long readLong(final JsonIterator iter, final byte c, final boolean negative) throws IOException {
long ind = IterImplNumber.intDigits[c];
if (ind == 0) {
IterImplForStreaming.assertNotLeadingZero(iter);
return 0;
}
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
throw iter.reportError("readPositiveLong", "expect 0~9");
throw iter.reportError("readLong", "expect 0~9");
}
if (iter.tail - iter.head > 9) {
int i = iter.head;
int ind2 = IterImplNumber.intDigits[iter.buf[i]];
if (ind2 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind;
return negative ? -ind : ind;
}
int ind3 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind3 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 10 + ind2;
ind = ind * 10 + ind2;
return negative ? -ind : ind;
}
int ind4 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind4 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 100 + ind2 * 10 + ind3;
ind = ind * 100 + ind2 * 10 + ind3;
return negative ? -ind : ind;
}
int ind5 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind5 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 1000 + ind2 * 100 + ind3 * 10 + ind4;
ind = ind * 1000 + ind2 * 100 + ind3 * 10 + ind4;
return negative ? -ind : ind;
}
int ind6 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind6 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 10000 + ind2 * 1000 + ind3 * 100 + ind4 * 10 + ind5;
ind = ind * 10000 + ind2 * 1000 + ind3 * 100 + ind4 * 10 + ind5;
return negative ? -ind : ind;
}
int ind7 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind7 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 100000 + ind2 * 10000 + ind3 * 1000 + ind4 * 100 + ind5 * 10 + ind6;
ind = ind * 100000 + ind2 * 10000 + ind3 * 1000 + ind4 * 100 + ind5 * 10 + ind6;
return negative ? -ind : ind;
}
int ind8 = IterImplNumber.intDigits[iter.buf[++i]];
if (ind8 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return ind * 1000000 + ind2 * 100000 + ind3 * 10000 + ind4 * 1000 + ind5 * 100 + ind6 * 10 + ind7;
ind = ind * 1000000 + ind2 * 100000 + ind3 * 10000 + ind4 * 1000 + ind5 * 100 + ind6 * 10 + ind7;
return negative ? -ind : ind;
}
int ind9 = IterImplNumber.intDigits[iter.buf[++i]];
ind = ind * 10000000 + ind2 * 1000000 + ind3 * 100000 + ind4 * 10000 + ind5 * 1000 + ind6 * 100 + ind7 * 10 + ind8;
iter.head = i;
if (ind9 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
return ind;
return negative ? -ind : ind;
}
}
return IterImplForStreaming.readLongSlowPath(iter, ind);
return IterImplForStreaming.readLongSlowPath(iter, ind, negative);
}

static final double readPositiveDouble(final JsonIterator iter) throws IOException {
static final double readDouble(final JsonIterator iter, final boolean negative) throws IOException {
int oldHead = iter.head;
try {
try {
long value = IterImplNumber.readLong(iter); // without the dot
long value = IterImplNumber.readLong(iter); // without the dot & sign
value = negative ? -value : value;
if (iter.head == iter.tail) {
return value;
}
Expand All @@ -448,27 +461,29 @@ static final double readPositiveDouble(final JsonIterator iter) throws IOExcepti
iter.head++;
int start = iter.head;
c = iter.buf[iter.head++];
long decimalPart = readPositiveLong(iter, c);
long decimalPart = readLong(iter, c, negative);
int decimalPlaces = iter.head - start;
if (decimalPlaces > 0 && decimalPlaces < IterImplNumber.POW10.length && (iter.head - oldHead) < 10) {
value = value * IterImplNumber.POW10[decimalPlaces] + decimalPart;
return value / (double) IterImplNumber.POW10[decimalPlaces];
return value + (decimalPart / (double) IterImplNumber.POW10[decimalPlaces]);
} else {
iter.head = oldHead;
return IterImplForStreaming.readDoubleSlowPath(iter);
double result = IterImplForStreaming.readDoubleSlowPath(iter);
return negative ? -result : result;
}
} else {
return value;
}
} finally {
if (iter.head < iter.tail && (iter.buf[iter.head] == 'e' || iter.buf[iter.head] == 'E')) {
iter.head = oldHead;
return IterImplForStreaming.readDoubleSlowPath(iter);
double result = IterImplForStreaming.readDoubleSlowPath(iter);
return negative ? -result : result;
}
}
} catch (JsonException e) {
iter.head = oldHead;
return IterImplForStreaming.readDoubleSlowPath(iter);
double result = IterImplForStreaming.readDoubleSlowPath(iter);
return negative ? -result : result;
}
}
}
55 changes: 31 additions & 24 deletions src/main/java/com/jsoniter/IterImplForStreaming.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,48 +487,56 @@ public final static int readStringSlowPath(JsonIterator iter, int j) throws IOEx
}
}

static long readLongSlowPath(JsonIterator iter, long value) throws IOException {
static long readLongSlowPath(final JsonIterator iter, long value, final boolean negative) throws IOException {
Copy link
Contributor

@taowen taowen Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final boolean negative is not necessary. we can always return negative number, and make it positive from the readLong. this way saved one branch.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some additional param is required until we inline these slow path methods or split them on pair of methods (one for negative, other for positive)
value = -value; // add negatives to avoid redundant checks for Long.MIN_VALUE on each iteration
long limit = negative ? Long.MIN_VALUE : -Long.MAX_VALUE;
long multmin = -922337203685477580L; // limit / 10
for (; ; ) {
for (int i = iter.head; i < iter.tail; i++) {
int ind = IterImplNumber.intDigits[iter.buf[i]];
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return value;
return negative ? value : -value;
}
if (value == Long.MIN_VALUE) {
throw iter.reportError("readLongSlowPath", "value is too large for long");
if (value < multmin) {
throw iter.reportError("readIntSlowPath", "value is too large for int");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw iter.reportError("readIntSlowPath", "value is too large for int");

should be long
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed by following 2 commits

please sqash them in case of merge
}
value = (value << 3) + (value << 1) + ind;
if (value < 0 && value != Long.MIN_VALUE) {
value = (value << 3) + (value << 1);
if (value < limit + ind) {
Copy link
Contributor

@taowen taowen Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be we can change

if (value < limit + ind) { throw iter.reportError("readLongSlowPath", "value is too large for long"); } value -= ind;

to

value -= ind; if (value >= 0) { throw iter.reportError("readLongSlowPath", "value is too large for long"); }

if value is positive, we add additional value != Long.MIN_VALUE check
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

limit + ind is required to properly detect overflow when number to parse is Long.MAX_VALUE + 1 to do not raise overflow when parsing Long.MIN_VALUE
throw iter.reportError("readLongSlowPath", "value is too large for long");
}
value -= ind;
}
if (!IterImpl.loadMore(iter)) {
iter.head = iter.tail;
return value;
return negative ? value : -value;
}
}
}

static int readIntSlowPath(JsonIterator iter, int value) throws IOException {
static int readIntSlowPath(final JsonIterator iter, int value, final boolean negative) throws IOException {
value = -value; // add negatives to avoid redundant checks for Integer.MIN_VALUE on each iteration
int limit = negative ? Integer.MIN_VALUE : -Integer.MAX_VALUE;
int multmin = -214748364; // limit / 10
for (; ; ) {
for (int i = iter.head; i < iter.tail; i++) {
int ind = IterImplNumber.intDigits[iter.buf[i]];
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
iter.head = i;
return value;
return negative ? value : -value;
}
if (value == Integer.MIN_VALUE) {
if (value < multmin) {
throw iter.reportError("readIntSlowPath", "value is too large for int");
}
value = (value << 3) + (value << 1) + ind;
if (value < 0 && value != Integer.MIN_VALUE) {
value = (value << 3) + (value << 1);
if (value < limit + ind) {
throw iter.reportError("readIntSlowPath", "value is too large for int");
}
value -= ind;
}
if (!IterImpl.loadMore(iter)) {
iter.head = iter.tail;
return value;
return negative ? value : -value;
}
}
}
Expand Down Expand Up @@ -582,34 +590,33 @@ public static final String readNumber(final JsonIterator iter) throws IOExceptio
}
}


static final double readPositiveDouble(final JsonIterator iter) throws IOException {
return readDoubleSlowPath(iter);
static final double readDouble(final JsonIterator iter, final boolean negative) throws IOException {
double result = readDoubleSlowPath(iter);
return negative ? -result : result;
}


static final long readPositiveLong(final JsonIterator iter, byte c) throws IOException {
static final long readLong(final JsonIterator iter, final byte c, final boolean negative) throws IOException {
long ind = IterImplNumber.intDigits[c];
if (ind == 0) {
assertNotLeadingZero(iter);
return 0;
}
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
throw iter.reportError("readPositiveLong", "expect 0~9");
throw iter.reportError("readLong", "expect 0~9");
}
return IterImplForStreaming.readLongSlowPath(iter, ind);
return IterImplForStreaming.readLongSlowPath(iter, ind, negative);
}

static final int readPositiveInt(final JsonIterator iter, byte c) throws IOException {
static final int readInt(final JsonIterator iter, final byte c, final boolean negative) throws IOException {
int ind = IterImplNumber.intDigits[c];
if (ind == 0) {
assertNotLeadingZero(iter);
return 0;
}
if (ind == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
throw iter.reportError("readPositiveInt", "expect 0~9");
throw iter.reportError("readInt", "expect 0~9");
}
return IterImplForStreaming.readIntSlowPath(iter, ind);
return IterImplForStreaming.readIntSlowPath(iter, ind, negative);
}

static void assertNotLeadingZero(JsonIterator iter) throws IOException {
Expand All @@ -620,7 +627,7 @@ static void assertNotLeadingZero(JsonIterator iter) throws IOException {
if (ind2 == IterImplNumber.INVALID_CHAR_FOR_NUMBER) {
return;
}
throw iter.reportError("readPositiveInt", "leading zero is invalid");
throw iter.reportError("assertNotLeadingZero", "leading zero is invalid");
} catch (ArrayIndexOutOfBoundsException e) {
iter.head = iter.tail;
return;
Expand Down
32 changes: 8 additions & 24 deletions src/main/java/com/jsoniter/IterImplNumber.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
*/
package com.jsoniter;

import com.jsoniter.spi.JsonException;

import java.io.IOException;

class IterImplNumber {
Expand Down Expand Up @@ -65,40 +63,26 @@ class IterImplNumber {

public static final double readDouble(final JsonIterator iter) throws IOException {
final byte c = IterImpl.nextToken(iter);
if (c == '-') {
return -IterImpl.readPositiveDouble(iter);
} else {
boolean negative = c == '-';
if (!negative) {
iter.unreadByte();
return IterImpl.readPositiveDouble(iter);
}
return IterImpl.readDouble(iter, negative);
}

public static final float readFloat(final JsonIterator iter) throws IOException {
final byte c = IterImpl.nextToken(iter);
if (c == '-') {
return (float)-IterImpl.readPositiveDouble(iter);
} else {
iter.unreadByte();
return (float) IterImpl.readPositiveDouble(iter);
}
return (float) IterImplNumber.readDouble(iter);
}

public static final int readInt(final JsonIterator iter) throws IOException {
byte c = IterImpl.nextToken(iter);
if (c == '-') {
return -IterImpl.readPositiveInt(iter, IterImpl.readByte(iter));
} else {
return IterImpl.readPositiveInt(iter, c);
}
boolean negative = c == '-';
return IterImpl.readInt(iter, negative ? IterImpl.readByte(iter) : c, negative);
}

public static final long readLong(JsonIterator iter) throws IOException {
byte c = IterImpl.nextToken(iter);
if (c == '-') {
return -IterImpl.readPositiveLong(iter, IterImpl.readByte(iter));
} else {
return IterImpl.readPositiveLong(iter, c);
}
boolean negative = c == '-';
return IterImpl.readLong(iter, negative ? IterImpl.readByte(iter) : c, negative);
}

}
5 changes: 5 additions & 0 deletions src/test/java/com/jsoniter/TestFloat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,15 @@ public void test_ieee_754() throws IOException {
public void test_decimal_places() throws IOException {
assertEquals(Long.MAX_VALUE, parseFloat("9223372036854775807,"), 0.01f);
assertEquals(Long.MAX_VALUE, parseDouble("9223372036854775807,"), 0.01f);
assertEquals(Long.MIN_VALUE, parseDouble("-9223372036854775808,"), 0.01f);
assertEquals(9923372036854775807f, parseFloat("9923372036854775807,"), 0.01f);
assertEquals(-9923372036854775808f, parseFloat("-9923372036854775808,"), 0.01f);
assertEquals(9923372036854775807d, parseDouble("9923372036854775807,"), 0.01f);
assertEquals(-9923372036854775808d, parseDouble("-9923372036854775808,"), 0.01f);
assertEquals(720368.54775807f, parseFloat("720368.54775807,"), 0.01f);
assertEquals(-720368.54775807f, parseFloat("-720368.54775807,"), 0.01f);
assertEquals(720368.54775807d, parseDouble("720368.54775807,"), 0.01f);
assertEquals(-720368.54775807d, parseDouble("-720368.54775807,"), 0.01f);
assertEquals(72036.854775807f, parseFloat("72036.854775807,"), 0.01f);
assertEquals(72036.854775807d, parseDouble("72036.854775807,"), 0.01f);
assertEquals(720368.54775807f, parseFloat("720368.547758075,"), 0.01f);
Expand Down
Loading