From b4dab86b105c85e6b7d7106c9ff11e3e923e3485 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Mon, 1 Nov 2021 23:08:04 +0100 Subject: [PATCH] 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 --- .../java/com/google/gson/ToNumberPolicy.java | 6 +- .../gson/internal/bind/DateTypeAdapter.java | 30 ++++---- .../internal/bind/DefaultDateTypeAdapter.java | 16 +++-- .../gson/internal/bind/JsonTreeReader.java | 19 ++++- .../gson/internal/bind/NumberTypeAdapter.java | 2 +- .../gson/internal/bind/TypeAdapters.java | 68 ++++++++++++------ .../gson/internal/sql/SqlDateTypeAdapter.java | 25 +++++-- .../gson/internal/sql/SqlTimeTypeAdapter.java | 23 +++++-- .../com/google/gson/stream/JsonReader.java | 62 +++++++++++++---- .../functional/DefaultTypeAdaptersTest.java | 16 ++++- .../google/gson/functional/PrimitiveTest.java | 69 +++++++++++++++++-- .../gson/stream/JsonReaderPathTest.java | 68 ++++++++++++++++++ 12 files changed, 327 insertions(+), 77 deletions(-) diff --git a/gson/src/main/java/com/google/gson/ToNumberPolicy.java b/gson/src/main/java/com/google/gson/ToNumberPolicy.java index e7f91c93..bd70213b 100644 --- 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 index 6e849690..c5b16a81 100644 --- 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 final class DateTypeAdapter extends TypeAdapter { 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 index f56faee0..f5ee4e21 100644 --- 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 final class DefaultDateTypeAdapter extends TypeAdapter 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 final class DefaultDateTypeAdapter extends TypeAdapter 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 @@ public final class DefaultDateTypeAdapter extends TypeAdapter 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 index 0954fb33..f8238bc2 100644 --- 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 @@ public final class JsonTreeReader extends JsonReader { 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 @@ public final class JsonTreeReader extends JsonReader { 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 index f5efff28..5aaeae32 100644 --- 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 final class NumberTypeAdapter extends TypeAdapter { 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 index cd5ba2e3..1b6b0118 100644 --- 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 final class TypeAdapters { 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 final class TypeAdapters { 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 final class TypeAdapters { 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 final class TypeAdapters { } 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 final class TypeAdapters { 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 final class TypeAdapters { 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 final class TypeAdapters { 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 final class TypeAdapters { public static final TypeAdapter CURRENCY = new TypeAdapter() { @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 final class TypeAdapters { 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 index b3da1fef..6ae4c3ef 100644 --- 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.io.IOException; 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 @@ final class SqlDateTypeAdapter extends TypeAdapter { } @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 index ee65726b..c2a37073 100644 --- 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