Make default adapters stricter; improve exception messages (#2000)

* Make default adapters stricter; improve exception messages * Reduce scope of synchronized blocks * Improve JsonReader.getPath / getPreviousPath Javadoc
google · Nov 1, 2021 · b4dab86 · b4dab86
1 parent b3188c1
commit b4dab86
Show file tree

Hide file tree

Showing 12 changed files with 327 additions and 77 deletions.
diff --git a/gson/src/main/java/com/google/gson/ToNumberPolicy.java b/gson/src/main/java/com/google/gson/ToNumberPolicy.java
@@ -71,11 +71,11 @@ public enum ToNumberPolicy implements ToNumberStrategy {
         try {
           Double d = Double.valueOf(value);
           if ((d.isInfinite() || d.isNaN()) && !in.isLenient()) {
-            throw new MalformedJsonException("JSON forbids NaN and infinities: " + d + "; at path " + in.getPath());
+            throw new MalformedJsonException("JSON forbids NaN and infinities: " + d + "; at path " + in.getPreviousPath());
           }
           return d;
         } catch (NumberFormatException doubleE) {
-          throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPath(), doubleE);
+          throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPreviousPath(), doubleE);
         }
       }
     }
@@ -91,7 +91,7 @@ public enum ToNumberPolicy implements ToNumberStrategy {
       try {
         return new BigDecimal(value);
       } catch (NumberFormatException e) {
-        throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPath(), e);
+        throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPreviousPath(), e);
       }
     }
   }

diff --git a/gson/src/main/java/com/google/gson/internal/bind/DateTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/DateTypeAdapter.java
@@ -72,30 +72,36 @@ public DateTypeAdapter() {
       in.nextNull();
       return null;
     }
-    return deserializeToDate(in.nextString());
+    return deserializeToDate(in);
   }
 
-  private synchronized Date deserializeToDate(String json) {
-    for (DateFormat dateFormat : dateFormats) {
-      try {
-        return dateFormat.parse(json);
-      } catch (ParseException ignored) {}
+  private Date deserializeToDate(JsonReader in) throws IOException {
+    String s = in.nextString();
+    synchronized (dateFormats) {
+      for (DateFormat dateFormat : dateFormats) {
+        try {
+          return dateFormat.parse(s);
+        } catch (ParseException ignored) {}
+      }
     }
     try {
-    	return ISO8601Utils.parse(json, new ParsePosition(0));
+      return ISO8601Utils.parse(s, new ParsePosition(0));
     } catch (ParseException e) {
-      throw new JsonSyntaxException(json, e);
+      throw new JsonSyntaxException("Failed parsing '" + s + "' as Date; at path " + in.getPreviousPath(), e);
     }
   }
 
-  @Override public synchronized void write(JsonWriter out, Date value) throws IOException {
+  @Override public void write(JsonWriter out, Date value) throws IOException {
     if (value == null) {
       out.nullValue();
       return;
     }
-    String dateFormatAsString = dateFormats.get(0).format(value);
+
+    DateFormat dateFormat = dateFormats.get(0);
+    String dateFormatAsString;
+    synchronized (dateFormats) {
+      dateFormatAsString = dateFormat.format(value);
+    }
     out.value(dateFormatAsString);
   }
-
-
 }
diff --git a/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java
@@ -130,10 +130,13 @@ public void write(JsonWriter out, Date value) throws IOException {
       out.nullValue();
       return;
     }
-    synchronized(dateFormats) {
-      String dateFormatAsString = dateFormats.get(0).format(value);
-      out.value(dateFormatAsString);
+
+    DateFormat dateFormat = dateFormats.get(0);
+    String dateFormatAsString;
+    synchronized (dateFormats) {
+      dateFormatAsString = dateFormat.format(value);
     }
+    out.value(dateFormatAsString);
   }
 
   @Override
@@ -142,11 +145,12 @@ public T read(JsonReader in) throws IOException {
       in.nextNull();
       return null;
     }
-    Date date = deserializeToDate(in.nextString());
+    Date date = deserializeToDate(in);
     return dateType.deserialize(date);
   }
 
-  private Date deserializeToDate(String s) {
+  private Date deserializeToDate(JsonReader in) throws IOException {
+    String s = in.nextString();
     synchronized (dateFormats) {
       for (DateFormat dateFormat : dateFormats) {
         try {
@@ -158,7 +162,7 @@ private Date deserializeToDate(String s) {
     try {
       return ISO8601Utils.parse(s, new ParsePosition(0));
     } catch (ParseException e) {
-      throw new JsonSyntaxException(s, e);
+      throw new JsonSyntaxException("Failed parsing '" + s + "' as Date; at path " + in.getPreviousPath(), e);
     }
   }
 

diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java
@@ -304,12 +304,19 @@ private void push(Object newTop) {
     stack[stackSize++] = newTop;
   }
 
-  @Override public String getPath() {
+  private String getPath(boolean usePreviousPath) {
     StringBuilder result = new StringBuilder().append('$');
     for (int i = 0; i < stackSize; i++) {
       if (stack[i] instanceof JsonArray) {
         if (++i < stackSize && stack[i] instanceof Iterator) {
-          result.append('[').append(pathIndices[i]).append(']');
+          int pathIndex = pathIndices[i];
+          // If index is last path element it points to next array element; have to decrement
+          // `- 1` covers case where iterator for next element is on stack
+          // `- 2` covers case where peek() already pushed next element onto stack
+          if (usePreviousPath && pathIndex > 0 && (i == stackSize - 1 || i == stackSize - 2)) {
+            pathIndex--;
+          }
+          result.append('[').append(pathIndex).append(']');
         }
       } else if (stack[i] instanceof JsonObject) {
         if (++i < stackSize && stack[i] instanceof Iterator) {
@@ -323,6 +330,14 @@ private void push(Object newTop) {
     return result.toString();
   }
 
+  @Override public String getPreviousPath() {
+    return getPath(true);
+  }
+
+  @Override public String getPath() {
+    return getPath(false);
+  }
+
   private String locationString() {
     return " at path " + getPath();
   }

diff --git a/gson/src/main/java/com/google/gson/internal/bind/NumberTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/NumberTypeAdapter.java
@@ -72,7 +72,7 @@ public static TypeAdapterFactory getFactory(ToNumberStrategy toNumberStrategy) {
     case STRING:
       return toNumberStrategy.readNumber(in);
     default:
-      throw new JsonSyntaxException("Expecting number, got: " + jsonToken);
+      throw new JsonSyntaxException("Expecting number, got: " + jsonToken + "; at path " + in.getPath());
     }
   }
 

diff --git a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java
@@ -92,22 +92,21 @@ public Class read(JsonReader in) throws IOException {
         boolean set;
         switch (tokenType) {
         case NUMBER:
-          set = in.nextInt() != 0;
+        case STRING:
+          int intValue = in.nextInt();
+          if (intValue == 0) {
+            set = false;
+          } else if (intValue == 1) {
+            set = true;
+          } else {
+            throw new JsonSyntaxException("Invalid bitset value " + intValue + ", expected 0 or 1; at path " + in.getPreviousPath());
+          }
           break;
         case BOOLEAN:
           set = in.nextBoolean();
           break;
-        case STRING:
-          String stringValue = in.nextString();
-          try {
-            set = Integer.parseInt(stringValue) != 0;
-          } catch (NumberFormatException e) {
-            throw new JsonSyntaxException(
-                "Error: Expecting: bitset number value (1, 0), Found: " + stringValue);
-          }
-          break;
         default:
-          throw new JsonSyntaxException("Invalid bitset value type: " + tokenType);
+          throw new JsonSyntaxException("Invalid bitset value type: " + tokenType + "; at path " + in.getPath());
         }
         if (set) {
           bitset.set(i);
@@ -178,12 +177,18 @@ public Number read(JsonReader in) throws IOException {
         in.nextNull();
         return null;
       }
+
+      int intValue;
       try {
-        int intValue = in.nextInt();
-        return (byte) intValue;
+        intValue = in.nextInt();
       } catch (NumberFormatException e) {
         throw new JsonSyntaxException(e);
       }
+      // Allow up to 255 to support unsigned values
+      if (intValue > 255 || intValue < Byte.MIN_VALUE) {
+        throw new JsonSyntaxException("Lossy conversion from " + intValue + " to byte; at path " + in.getPreviousPath());
+      }
+      return (byte) intValue;
     }
     @Override
     public void write(JsonWriter out, Number value) throws IOException {
@@ -201,11 +206,18 @@ public Number read(JsonReader in) throws IOException {
         in.nextNull();
         return null;
       }
+
+      int intValue;
       try {
-        return (short) in.nextInt();
+        intValue = in.nextInt();
       } catch (NumberFormatException e) {
         throw new JsonSyntaxException(e);
       }
+      // Allow up to 65535 to support unsigned values
+      if (intValue > 65535 || intValue < Short.MIN_VALUE) {
+        throw new JsonSyntaxException("Lossy conversion from " + intValue + " to short; at path " + in.getPreviousPath());
+      }
+      return (short) intValue;
     }
     @Override
     public void write(JsonWriter out, Number value) throws IOException {
@@ -352,7 +364,7 @@ public Character read(JsonReader in) throws IOException {
       }
       String str = in.nextString();
       if (str.length() != 1) {
-        throw new JsonSyntaxException("Expecting character, got: " + str);
+        throw new JsonSyntaxException("Expecting character, got: " + str + "; at " + in.getPreviousPath());
       }
       return str.charAt(0);
     }
@@ -391,10 +403,11 @@ public void write(JsonWriter out, String value) throws IOException {
         in.nextNull();
         return null;
       }
+      String s = in.nextString();
       try {
-        return new BigDecimal(in.nextString());
+        return new BigDecimal(s);
       } catch (NumberFormatException e) {
-        throw new JsonSyntaxException(e);
+        throw new JsonSyntaxException("Failed parsing '" + s + "' as BigDecimal; at path " + in.getPreviousPath(), e);
       }
     }
 
@@ -409,10 +422,11 @@ public void write(JsonWriter out, String value) throws IOException {
         in.nextNull();
         return null;
       }
+      String s = in.nextString();
       try {
-        return new BigInteger(in.nextString());
+        return new BigInteger(s);
       } catch (NumberFormatException e) {
-        throw new JsonSyntaxException(e);
+        throw new JsonSyntaxException("Failed parsing '" + s + "' as BigInteger; at path " + in.getPreviousPath(), e);
       }
     }
 
@@ -525,7 +539,12 @@ public UUID read(JsonReader in) throws IOException {
         in.nextNull();
         return null;
       }
-      return java.util.UUID.fromString(in.nextString());
+      String s = in.nextString();
+      try {
+        return java.util.UUID.fromString(s);
+      } catch (IllegalArgumentException e) {
+        throw new JsonSyntaxException("Failed parsing '" + s + "' as UUID; at path " + in.getPreviousPath(), e);
+      }
     }
     @Override
     public void write(JsonWriter out, UUID value) throws IOException {
@@ -538,7 +557,12 @@ public void write(JsonWriter out, UUID value) throws IOException {
   public static final TypeAdapter<Currency> CURRENCY = new TypeAdapter<Currency>() {
     @Override
     public Currency read(JsonReader in) throws IOException {
-      return Currency.getInstance(in.nextString());
+      String s = in.nextString();
+      try {
+        return Currency.getInstance(s);
+      } catch (IllegalArgumentException e) {
+        throw new JsonSyntaxException("Failed parsing '" + s + "' as Currency; at path " + in.getPreviousPath(), e);
+      }
     }
     @Override
     public void write(JsonWriter out, Currency value) throws IOException {
@@ -866,7 +890,7 @@ public static <T1> TypeAdapterFactory newTypeHierarchyFactory(
             T1 result = typeAdapter.read(in);
             if (result != null && !requestedType.isInstance(result)) {
               throw new JsonSyntaxException("Expected a " + requestedType.getName()
-                  + " but was " + result.getClass().getName());
+                  + " but was " + result.getClass().getName() + "; at path " + in.getPreviousPath());
             }
             return result;
           }

diff --git a/gson/src/main/java/com/google/gson/internal/sql/SqlDateTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/sql/SqlDateTypeAdapter.java
@@ -28,6 +28,7 @@
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.Date;
 
 /**
  * Adapter for java.sql.Date. Although this class appears stateless, it is not.
@@ -50,21 +51,33 @@ private SqlDateTypeAdapter() {
   }
 
   @Override
-  public synchronized java.sql.Date read(JsonReader in) throws IOException {
+  public java.sql.Date read(JsonReader in) throws IOException {
     if (in.peek() == JsonToken.NULL) {
       in.nextNull();
       return null;
     }
+    String s = in.nextString();
     try {
-      final long utilDate = format.parse(in.nextString()).getTime();
-      return new java.sql.Date(utilDate);
+      Date utilDate;
+      synchronized (this) {
+        utilDate = format.parse(s);
+      }
+      return new java.sql.Date(utilDate.getTime());
     } catch (ParseException e) {
-      throw new JsonSyntaxException(e);
+      throw new JsonSyntaxException("Failed parsing '" + s + "' as SQL Date; at path " + in.getPreviousPath(), e);
     }
   }
 
   @Override
-  public synchronized void write(JsonWriter out, java.sql.Date value) throws IOException {
-    out.value(value == null ? null : format.format(value));
+  public void write(JsonWriter out, java.sql.Date value) throws IOException {
+    if (value == null) {
+      out.nullValue();
+      return;
+    }
+    String dateString;
+    synchronized (this) {
+      dateString = format.format(value);
+    }
+    out.value(dateString);
   }
 }
diff --git a/gson/src/main/java/com/google/gson/internal/sql/SqlTimeTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/sql/SqlTimeTypeAdapter.java
@@ -50,20 +50,31 @@ final class SqlTimeTypeAdapter extends TypeAdapter<Time> {
   private SqlTimeTypeAdapter() {
   }
 
-  @Override public synchronized Time read(JsonReader in) throws IOException {
+  @Override public Time read(JsonReader in) throws IOException {
     if (in.peek() == JsonToken.NULL) {
       in.nextNull();
       return null;
     }
+    String s = in.nextString();
     try {
-      Date date = format.parse(in.nextString());
-      return new Time(date.getTime());
+      synchronized (this) {
+        Date date = format.parse(s);
+        return new Time(date.getTime());
+      }
     } catch (ParseException e) {
-      throw new JsonSyntaxException(e);
+      throw new JsonSyntaxException("Failed parsing '" + s + "' as SQL Time; at path " + in.getPreviousPath(), e);
     }
   }
 
-  @Override public synchronized void write(JsonWriter out, Time value) throws IOException {
-    out.value(value == null ? null : format.format(value));
+  @Override public void write(JsonWriter out, Time value) throws IOException {
+    if (value == null) {
+      out.nullValue();
+      return;
+    }
+    String timeString;
+    synchronized (this) {
+      timeString = format.format(value);
+    }
+    out.value(timeString);
   }
 }