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

Make default adapters stricter; improve exception messages #2000

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions gson/src/main/java/com/google/gson/ToNumberPolicy.java
Expand Up @@ -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);
}
}
}
Expand All @@ -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);
}
}
}
Expand Down
Expand Up @@ -72,30 +72,34 @@ 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);
String dateFormatAsString;
synchronized (dateFormats) {
dateFormatAsString = dateFormats.get(0).format(value);
}
out.value(dateFormatAsString);
}


}
Expand Up @@ -130,10 +130,11 @@ public void write(JsonWriter out, Date value) throws IOException {
out.nullValue();
return;
}
synchronized(dateFormats) {
String dateFormatAsString = dateFormats.get(0).format(value);
out.value(dateFormatAsString);
String dateFormatAsString;
synchronized (dateFormats) {
dateFormatAsString = dateFormats.get(0).format(value);
}
out.value(dateFormatAsString);
}

@Override
Expand All @@ -142,11 +143,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 {
Expand All @@ -158,7 +160,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);
}
}

Expand Down
Expand Up @@ -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 (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 (stack[++i] instanceof Iterator) {
Expand All @@ -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();
}
Expand Down
Expand Up @@ -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());
}
}

Expand Down
68 changes: 46 additions & 22 deletions gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java
Expand Up @@ -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();
Comment on lines +95 to +96
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonReader.nextInt() can also parse JSON strings as int so I simplified this instead of handling JSON strings separately (as it was the case before). However, this has the following disadvantages:

  • The exception thrown for a JSON string which is not an int (e.g. "abc") is not as descriptive anymore because it will now be thrown by JsonReader
  • It now allows parsing JSON strings which have a floating point value (but whose value is integral), previously it only allowed ints due to usage of Integer.parseInt

So maybe it would still be worth handling JSON strings here manually, as it was done before?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't think it's likely that people would be affected by these particular changes in behaviour. Only recognizing 0 or 1 seems more likely to affect people, but even that seems unlikely. (I'm not sure how often people are serializing BitSet instances anyway, especially given how verbose the encoding is for sparse sets.)

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);
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
Expand Up @@ -50,21 +50,32 @@ 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);
synchronized (this) {
long utilDate = format.parse(s).getTime();
return new java.sql.Date(utilDate);
}
} 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);
}
}
Expand Up @@ -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());
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved
}
} 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);
}
}