Don't check number limits in JsonReader

google · Oct 13, 2023 · d1004f9 · d1004f9
1 parent cf297ec
commit d1004f9
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 26 deletions.
diff --git a/gson/src/main/java/com/google/gson/internal/NumberLimits.java b/gson/src/main/java/com/google/gson/internal/NumberLimits.java
@@ -11,7 +11,7 @@ public class NumberLimits {
   private NumberLimits() {
   }
 
-  public static final int MAX_NUMBER_STRING_LENGTH = 10_000;
+  private static final int MAX_NUMBER_STRING_LENGTH = 10_000;
 
   private static void checkNumberStringLength(String s) {
     if (s.length() > MAX_NUMBER_STRING_LENGTH) {

diff --git a/gson/src/main/java/com/google/gson/stream/JsonReader.java b/gson/src/main/java/com/google/gson/stream/JsonReader.java
@@ -20,7 +20,6 @@
 import com.google.gson.GsonBuilder;
 import com.google.gson.Strictness;
 import com.google.gson.internal.JsonReaderInternalAccess;
-import com.google.gson.internal.NumberLimits;
 import com.google.gson.internal.TroubleshootingGuide;
 import com.google.gson.internal.bind.JsonTreeReader;
 import java.io.Closeable;
@@ -722,7 +721,6 @@ private int peekNumber() throws IOException {
     long value = 0; // Negative to accommodate Long.MIN_VALUE more easily.
     boolean negative = false;
     boolean fitsInLong = true;
-    int exponentDigitsCount = 0;
     int last = NUMBER_CHAR_NONE;
 
     int i = 0;
@@ -799,15 +797,6 @@ private int peekNumber() throws IOException {
           last = NUMBER_CHAR_FRACTION_DIGIT;
         } else if (last == NUMBER_CHAR_EXP_E || last == NUMBER_CHAR_EXP_SIGN) {
           last = NUMBER_CHAR_EXP_DIGIT;
-          exponentDigitsCount++;
-        } else if (last == NUMBER_CHAR_EXP_DIGIT) {
-          exponentDigitsCount++;
-
-          // Similar to the scale limit enforced by NumberLimits.parseBigDecimal(String)
-          // This also considers leading 0s (e.g. '1e000001'), but probably not worth the effort ignoring them
-          if (exponentDigitsCount > 4) {
-            throw new MalformedJsonException("Too many number exponent digits" + locationString());
-          }
         }
       }
     }
@@ -822,11 +811,6 @@ private int peekNumber() throws IOException {
     } else if (last == NUMBER_CHAR_DIGIT || last == NUMBER_CHAR_FRACTION_DIGIT
         || last == NUMBER_CHAR_EXP_DIGIT) {
       peekedNumberLength = i;
-      // Note: This will currently always be false because for PEEKED_NUMBER the number is backed
-      // by the `buffer` and its length is smaller than the max number string length
-      if (peekedNumberLength > NumberLimits.MAX_NUMBER_STRING_LENGTH) {
-        throw new MalformedJsonException("Number too large" + locationString());
-      }
       return peeked = PEEKED_NUMBER;
     } else {
       return PEEKED_NONE;

diff --git a/gson/src/test/java/com/google/gson/functional/NumberLimitsTest.java b/gson/src/test/java/com/google/gson/functional/NumberLimitsTest.java
@@ -29,6 +29,23 @@ 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));
@@ -49,14 +66,13 @@ public void testJsonReader() throws IOException {
     assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
     assertThat(reader.nextString()).isEqualTo("1e+9999");
 
-    JsonReader reader3 = jsonReader("1e10000");
-    e = assertThrows(MalformedJsonException.class, () -> reader3.peek());
-    assertThat(e).hasMessageThat().isEqualTo("Too many number exponent digits at line 1 column 1 path $");
+    reader = jsonReader("1e10000");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e10000");
 
-    JsonReader reader4 = jsonReader("1e00001");
-    // Currently JsonReader does not ignore leading 0s in exponent
-    e = assertThrows(MalformedJsonException.class, () -> reader4.peek());
-    assertThat(e).hasMessageThat().isEqualTo("Too many number exponent digits at line 1 column 1 path $");
+    reader = jsonReader("1e00001");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e00001");
   }
 
   @Test

diff --git a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java
@@ -601,8 +601,8 @@ public void testDoubles() throws IOException {
         + "0e0,"
         + "1e+0,"
         + "1e-0,"
-        + "1e000," // leading 0 is allowed for exponent
-        + "1e001,"
+        + "1e0000," // leading 0 is allowed for exponent
+        + "1e00001,"
         + "1e+1]";
     JsonReader reader = new JsonReader(reader(json));
     reader.beginArray();