Skip to content

Commit

Permalink
Add limits when deserializing BigDecimal and BigInteger (#2510)
Browse files Browse the repository at this point in the history
* Add limits when deserializing `BigDecimal` and `BigInteger`

* Use assertThrows

* Don't check number limits in JsonReader
  • Loading branch information
Marcono1234 authored Oct 23, 2023
1 parent 802476a commit 6a9d240
Show file tree
Hide file tree
Showing 7 changed files with 284 additions and 182 deletions.
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

0 comments on commit 6a9d240

Please sign in to comment.