Skip to content
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.

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

Add limits when deserializing BigDecimal and BigInteger #2510

Merged
merged 5 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions gson/src/main/java/com/google/gson/JsonPrimitive.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import com.google.gson.internal.LazilyParsedNumber;
import com.google.gson.internal.NumberLimits;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Objects;
Expand Down Expand Up @@ -172,7 +173,7 @@ public double getAsDouble() {
*/
@Override
public BigDecimal getAsBigDecimal() {
return value instanceof BigDecimal ? (BigDecimal) value : new BigDecimal(getAsString());
return value instanceof BigDecimal ? (BigDecimal) value : NumberLimits.parseBigDecimal(getAsString());
}

/**
Expand All @@ -184,7 +185,7 @@ public BigInteger getAsBigInteger() {
? (BigInteger) value
: isIntegral(this)
? BigInteger.valueOf(this.getAsNumber().longValue())
: new BigInteger(this.getAsString());
: NumberLimits.parseBigInteger(this.getAsString());
}

/**
Expand Down
3 changes: 2 additions & 1 deletion gson/src/main/java/com/google/gson/ToNumberPolicy.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import com.google.gson.internal.LazilyParsedNumber;
import com.google.gson.internal.NumberLimits;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.MalformedJsonException;
import java.io.IOException;
Expand Down Expand Up @@ -89,7 +90,7 @@ public enum ToNumberPolicy implements ToNumberStrategy {
@Override public BigDecimal readNumber(JsonReader in) throws IOException {
String value = in.nextString();
try {
return new BigDecimal(value);
return NumberLimits.parseBigDecimal(value);
} catch (NumberFormatException e) {
throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPreviousPath(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public LazilyParsedNumber(String value) {
this.value = value;
}

private BigDecimal asBigDecimal() {
return NumberLimits.parseBigDecimal(value);
}

@Override
public int intValue() {
try {
Expand All @@ -43,7 +47,7 @@ public int intValue() {
try {
return (int) Long.parseLong(value);
} catch (NumberFormatException nfe) {
return new BigDecimal(value).intValue();
return asBigDecimal().intValue();
}
}
}
Expand All @@ -53,7 +57,7 @@ public long longValue() {
try {
return Long.parseLong(value);
} catch (NumberFormatException e) {
return new BigDecimal(value).longValue();
return asBigDecimal().longValue();
}
}

Expand All @@ -78,7 +82,7 @@ public String toString() {
* deserialize it.
*/
private Object writeReplace() throws ObjectStreamException {
return new BigDecimal(value);
return asBigDecimal();
}

private void readObject(ObjectInputStream in) throws IOException {
Expand Down
37 changes: 37 additions & 0 deletions gson/src/main/java/com/google/gson/internal/NumberLimits.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.google.gson.internal;

import java.math.BigDecimal;
import java.math.BigInteger;

/**
* This class enforces limits on numbers parsed from JSON to avoid potential performance
* problems when extremely large numbers are used.
*/
public class NumberLimits {
private NumberLimits() {
}

private static final int MAX_NUMBER_STRING_LENGTH = 10_000;

private static void checkNumberStringLength(String s) {
if (s.length() > MAX_NUMBER_STRING_LENGTH) {
throw new NumberFormatException("Number string too large: " + s.substring(0, 30) + "...");
}
}

public static BigDecimal parseBigDecimal(String s) throws NumberFormatException {
checkNumberStringLength(s);
BigDecimal decimal = new BigDecimal(s);

// Cast to long to avoid issues with abs when value is Integer.MIN_VALUE
if (Math.abs((long) decimal.scale()) >= 10_000) {
throw new NumberFormatException("Number has unsupported scale: " + s);
}
return decimal;
}

public static BigInteger parseBigInteger(String s) throws NumberFormatException {
checkNumberStringLength(s);
return new BigInteger(s);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.gson.TypeAdapterFactory;
import com.google.gson.annotations.SerializedName;
import com.google.gson.internal.LazilyParsedNumber;
import com.google.gson.internal.NumberLimits;
import com.google.gson.internal.TroubleshootingGuide;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
Expand Down Expand Up @@ -437,7 +438,7 @@ public void write(JsonWriter out, String value) throws IOException {
}
String s = in.nextString();
try {
return new BigDecimal(s);
return NumberLimits.parseBigDecimal(s);
} catch (NumberFormatException e) {
throw new JsonSyntaxException("Failed parsing '" + s + "' as BigDecimal; at path " + in.getPreviousPath(), e);
}
Expand All @@ -456,7 +457,7 @@ public void write(JsonWriter out, String value) throws IOException {
}
String s = in.nextString();
try {
return new BigInteger(s);
return NumberLimits.parseBigInteger(s);
} catch (NumberFormatException e) {
throw new JsonSyntaxException("Failed parsing '" + s + "' as BigInteger; at path " + in.getPreviousPath(), e);
}
Expand Down
185 changes: 185 additions & 0 deletions gson/src/test/java/com/google/gson/functional/NumberLimitsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
package com.google.gson.functional;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.gson.Gson;
import com.google.gson.JsonParseException;
import com.google.gson.JsonPrimitive;
import com.google.gson.JsonSyntaxException;
import com.google.gson.ToNumberPolicy;
import com.google.gson.ToNumberStrategy;
import com.google.gson.TypeAdapter;
import com.google.gson.internal.LazilyParsedNumber;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.MalformedJsonException;
import java.io.IOException;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
import java.io.StringReader;
import java.math.BigDecimal;
import java.math.BigInteger;
import org.junit.Test;

public class NumberLimitsTest {
private static final int MAX_LENGTH = 10_000;

private static JsonReader jsonReader(String json) {
return new JsonReader(new StringReader(json));
}

/**
* Tests how {@link JsonReader} behaves for large numbers.
*
* <p>Currently {@link JsonReader} itself does not enforce any limits.
* The reasons for this are:
* <ul>
* <li>Methods such as {@link JsonReader#nextDouble()} seem to have no problem
* parsing extremely large or small numbers (it rounds to 0 or Infinity)
* (to be verified?; if it had performance problems with certain numbers, then
* it would affect other parts of Gson which parse as float or double as well)
* <li>Enforcing limits only when a JSON number is encountered would be ineffective
* unless users explicitly call {@link JsonReader#peek()} and check if the value
* is a JSON number. Otherwise the limits could be circumvented because
* {@link JsonReader#nextString()} reads both strings and numbers, and for
* JSON strings no restrictions are enforced.
* </ul>
*/
@Test
public void testJsonReader() throws IOException {
JsonReader reader = jsonReader("1".repeat(1000));
assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
assertThat(reader.nextString()).isEqualTo("1".repeat(1000));

JsonReader reader2 = jsonReader("1".repeat(MAX_LENGTH + 1));
// Currently JsonReader does not recognize large JSON numbers as numbers but treats them
// as unquoted string
MalformedJsonException e = assertThrows(MalformedJsonException.class, () -> reader2.peek());
assertThat(e).hasMessageThat().startsWith("Use JsonReader.setStrictness(Strictness.LENIENT) to accept malformed JSON");

reader = jsonReader("1e9999");
assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
assertThat(reader.nextString()).isEqualTo("1e9999");

reader = jsonReader("1e+9999");
assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
assertThat(reader.nextString()).isEqualTo("1e+9999");

reader = jsonReader("1e10000");
assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
assertThat(reader.nextString()).isEqualTo("1e10000");

reader = jsonReader("1e00001");
assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
assertThat(reader.nextString()).isEqualTo("1e00001");
}

@Test
public void testJsonPrimitive() {
assertThat(new JsonPrimitive("1".repeat(MAX_LENGTH)).getAsBigDecimal())
.isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
assertThat(new JsonPrimitive("1e9999").getAsBigDecimal())
.isEqualTo(new BigDecimal("1e9999"));
assertThat(new JsonPrimitive("1e-9999").getAsBigDecimal())
.isEqualTo(new BigDecimal("1e-9999"));

NumberFormatException e = assertThrows(NumberFormatException.class,
() -> new JsonPrimitive("1".repeat(MAX_LENGTH + 1)).getAsBigDecimal());
assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");

e = assertThrows(NumberFormatException.class,
() -> new JsonPrimitive("1e10000").getAsBigDecimal());
assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");

e = assertThrows(NumberFormatException.class,
() -> new JsonPrimitive("1e-10000").getAsBigDecimal());
assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e-10000");


assertThat(new JsonPrimitive("1".repeat(MAX_LENGTH)).getAsBigInteger())
.isEqualTo(new BigInteger("1".repeat(MAX_LENGTH)));

e = assertThrows(NumberFormatException.class,
() -> new JsonPrimitive("1".repeat(MAX_LENGTH + 1)).getAsBigInteger());
assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
}

@Test
public void testToNumberPolicy() throws IOException {
ToNumberStrategy strategy = ToNumberPolicy.BIG_DECIMAL;

assertThat(strategy.readNumber(jsonReader("\"" + "1".repeat(MAX_LENGTH) + "\"")))
.isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
assertThat(strategy.readNumber(jsonReader("1e9999")))
.isEqualTo(new BigDecimal("1e9999"));


JsonParseException e = assertThrows(JsonParseException.class,
() -> strategy.readNumber(jsonReader("\"" + "1".repeat(MAX_LENGTH + 1) + "\"")));
assertThat(e).hasMessageThat().isEqualTo("Cannot parse " + "1".repeat(MAX_LENGTH + 1) + "; at path $");
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");

e = assertThrows(JsonParseException.class, () -> strategy.readNumber(jsonReader("\"1e10000\"")));
assertThat(e).hasMessageThat().isEqualTo("Cannot parse 1e10000; at path $");
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
}

@Test
public void testLazilyParsedNumber() throws IOException {
assertThat(new LazilyParsedNumber("1".repeat(MAX_LENGTH)).intValue())
.isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)).intValue());
assertThat(new LazilyParsedNumber("1e9999").intValue())
.isEqualTo(new BigDecimal("1e9999").intValue());

NumberFormatException e = assertThrows(NumberFormatException.class,
() -> new LazilyParsedNumber("1".repeat(MAX_LENGTH + 1)).intValue());
assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");

e = assertThrows(NumberFormatException.class,
() -> new LazilyParsedNumber("1e10000").intValue());
assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");

e = assertThrows(NumberFormatException.class,
() -> new LazilyParsedNumber("1e10000").longValue());
assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");

ObjectOutputStream objOut = new ObjectOutputStream(OutputStream.nullOutputStream());
// Number is serialized as BigDecimal; should also enforce limits during this conversion
e = assertThrows(NumberFormatException.class, () -> objOut.writeObject(new LazilyParsedNumber("1e10000")));
assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
}

@Test
public void testBigDecimalAdapter() throws IOException {
TypeAdapter<BigDecimal> adapter = new Gson().getAdapter(BigDecimal.class);

assertThat(adapter.fromJson("\"" + "1".repeat(MAX_LENGTH) + "\""))
.isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
assertThat(adapter.fromJson("\"1e9999\""))
.isEqualTo(new BigDecimal("1e9999"));

JsonSyntaxException e = assertThrows(JsonSyntaxException.class,
() -> adapter.fromJson("\"" + "1".repeat(MAX_LENGTH + 1) + "\""));
assertThat(e).hasMessageThat().isEqualTo("Failed parsing '" + "1".repeat(MAX_LENGTH + 1) + "' as BigDecimal; at path $");
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");

e = assertThrows(JsonSyntaxException.class,
() -> adapter.fromJson("\"1e10000\""));
assertThat(e).hasMessageThat().isEqualTo("Failed parsing '1e10000' as BigDecimal; at path $");
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
}

@Test
public void testBigIntegerAdapter() throws IOException {
TypeAdapter<BigInteger> adapter = new Gson().getAdapter(BigInteger.class);

assertThat(adapter.fromJson("\"" + "1".repeat(MAX_LENGTH) + "\""))
.isEqualTo(new BigInteger("1".repeat(MAX_LENGTH)));

JsonSyntaxException e = assertThrows(JsonSyntaxException.class,
() -> adapter.fromJson("\"" + "1".repeat(MAX_LENGTH + 1) + "\""));
assertThat(e).hasMessageThat().isEqualTo("Failed parsing '" + "1".repeat(MAX_LENGTH + 1) + "' as BigInteger; at path $");
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
}
}
Loading