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

Improve Gson and JsonParser trailing data handling #2123

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Expand Up @@ -178,6 +178,8 @@ public static <T> RuntimeTypeAdapterFactory<T> of(Class<T> baseType) {
/**
* Ensures that this factory will handle not just the given {@code baseType}, but any subtype
* of that type.
*
* @return this
*/
public RuntimeTypeAdapterFactory<T> recognizeSubtypes() {
this.recognizeSubtypes = true;
Expand All @@ -190,6 +192,7 @@ public RuntimeTypeAdapterFactory<T> recognizeSubtypes() {
*
* @throws IllegalArgumentException if either {@code type} or {@code label}
* have already been registered on this type adapter.
* @return this
*/
public RuntimeTypeAdapterFactory<T> registerSubtype(Class<? extends T> type, String label) {
if (type == null || label == null) {
Expand All @@ -209,6 +212,7 @@ public RuntimeTypeAdapterFactory<T> registerSubtype(Class<? extends T> type, Str
*
* @throws IllegalArgumentException if either {@code type} or its simple name
* have already been registered on this type adapter.
* @return this
*/
public RuntimeTypeAdapterFactory<T> registerSubtype(Class<? extends T> type) {
return registerSubtype(type, type.getSimpleName());
Expand Down
Expand Up @@ -19,7 +19,10 @@
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonParseException;
import com.google.gson.TypeAdapter;
import com.google.gson.TypeAdapterFactory;
import com.google.gson.annotations.JsonAdapter;
import com.google.gson.reflect.TypeToken;
import junit.framework.TestCase;

public final class RuntimeTypeAdapterFactoryTest extends TestCase {
Expand Down Expand Up @@ -191,10 +194,10 @@ public void testSerializeCollidingTypeFieldName() {
public void testSerializeWrappedNullValue() {
TypeAdapterFactory billingAdapter = RuntimeTypeAdapterFactory.of(BillingInstrument.class)
.registerSubtype(CreditCard.class)
.registerSubtype(BankTransfer.class);
.registerSubtype(BankTransfer.class);
Gson gson = new GsonBuilder()
.registerTypeAdapterFactory(billingAdapter)
.create();
.create();
String serialized = gson.toJson(new BillingInstrumentWrapper(null), BillingInstrumentWrapper.class);
BillingInstrumentWrapper deserialized = gson.fromJson(serialized, BillingInstrumentWrapper.class);
assertNull(deserialized.instrument);
Expand Down Expand Up @@ -229,4 +232,51 @@ static class BankTransfer extends BillingInstrument {
this.bankAccount = bankAccount;
}
}

public void testJsonAdapterDelegate() throws Exception {
Gson gson = new Gson();
Shape shape = new Circle(25);
String json = gson.toJson(shape);
assertEquals("{\"radius\":25,\"type\":\"CIRCLE\"}", json);
shape = gson.fromJson(json, Shape.class);
assertEquals(25, ((Circle)shape).radius);

shape = new Square(15);
json = gson.toJson(shape);
assertEquals("{\"side\":15,\"type\":\"SQUARE\"}", json);
shape = gson.fromJson(json, Shape.class);
assertEquals(15, ((Square)shape).side);
assertEquals(ShapeType.SQUARE, shape.type);
}

@JsonAdapter(Shape.JsonAdapterFactory.class)
static class Shape {
final ShapeType type;
Shape(ShapeType type) { this.type = type; }

private static final class JsonAdapterFactory implements TypeAdapterFactory {
private static final RuntimeTypeAdapterFactory<Shape> delegate = RuntimeTypeAdapterFactory.of(Shape.class, "type", true)
.registerSubtype(Circle.class, ShapeType.CIRCLE.toString())
.registerSubtype(Square.class, ShapeType.SQUARE.toString());

@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
return delegate.create(gson, type);
}
}
}

enum ShapeType {
SQUARE, CIRCLE
}

private static final class Circle extends Shape {
final int radius;
Circle(int radius) { super(ShapeType.CIRCLE); this.radius = radius; }
}

private static final class Square extends Shape {
final int side;
Square(int side) { super(ShapeType.SQUARE); this.side = side; }
}
}
30 changes: 14 additions & 16 deletions gson/src/main/java/com/google/gson/Gson.java
Expand Up @@ -39,7 +39,6 @@
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;
import java.io.EOFException;
import java.io.IOException;
import java.io.Reader;
Expand Down Expand Up @@ -1119,23 +1118,10 @@ public <T> T fromJson(Reader json, Type typeOfT) throws JsonIOException, JsonSyn
*/
public <T> T fromJson(Reader json, TypeToken<T> typeOfT) throws JsonIOException, JsonSyntaxException {
JsonReader jsonReader = newJsonReader(json);
T object = fromJson(jsonReader, typeOfT);
assertFullConsumption(object, jsonReader);
T object = fromJson(jsonReader, typeOfT, true);
return object;
}

private static void assertFullConsumption(Object obj, JsonReader reader) {
try {
if (obj != null && reader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("JSON document was not fully consumed.");
}
} catch (MalformedJsonException e) {
throw new JsonSyntaxException(e);
} catch (IOException e) {
throw new JsonIOException(e);
}
}

// fromJson(JsonReader, Class) is unfortunately missing and cannot be added now without breaking
// source compatibility in certain cases, see https://github.com/google/gson/pull/1700#discussion_r973764414

Expand Down Expand Up @@ -1201,6 +1187,14 @@ public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, J
* @see #fromJson(JsonReader, Type)
*/
public <T> T fromJson(JsonReader reader, TypeToken<T> typeOfT) throws JsonIOException, JsonSyntaxException {
return fromJson(reader, typeOfT, false);
}

/**
* @param requireEndDocument whether there must not be any trailing data after
* the first read JSON element
*/
private <T> T fromJson(JsonReader reader, TypeToken<T> typeOfT, boolean requireEndDocument) throws JsonIOException, JsonSyntaxException {
boolean isEmpty = true;
boolean oldLenient = reader.isLenient();
reader.setLenient(true);
Expand All @@ -1209,6 +1203,10 @@ public <T> T fromJson(JsonReader reader, TypeToken<T> typeOfT) throws JsonIOExce
isEmpty = false;
TypeAdapter<T> typeAdapter = getAdapter(typeOfT);
T object = typeAdapter.read(reader);

if (requireEndDocument && reader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("JSON document was not fully consumed.");
}
return object;
} catch (EOFException e) {
/*
Expand Down Expand Up @@ -1313,7 +1311,7 @@ public <T> T fromJson(JsonElement json, TypeToken<T> typeOfT) throws JsonSyntaxE
if (json == null) {
return null;
}
return fromJson(new JsonTreeReader(json), typeOfT);
return fromJson(new JsonTreeReader(json), typeOfT, false);
}

static class FutureTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
Expand Down
47 changes: 19 additions & 28 deletions gson/src/main/java/com/google/gson/JsonParser.java
Expand Up @@ -17,9 +17,6 @@

import com.google.gson.internal.Streams;
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.Reader;
import java.io.StringReader;

Expand Down Expand Up @@ -47,7 +44,7 @@ public JsonParser() {}
* @throws JsonParseException if the specified text is not valid JSON
* @since 2.8.6
*/
public static JsonElement parseString(String json) throws JsonSyntaxException {
public static JsonElement parseString(String json) throws JsonParseException {
return parseReader(new StringReader(json));
}

Expand All @@ -64,20 +61,25 @@ public static JsonElement parseString(String json) throws JsonSyntaxException {
* text is not valid JSON
* @since 2.8.6
*/
public static JsonElement parseReader(Reader reader) throws JsonIOException, JsonSyntaxException {
public static JsonElement parseReader(Reader reader) throws JsonParseException {
JsonReader jsonReader = new JsonReader(reader);
return parseReader(jsonReader, true);
}

private static JsonElement parseReader(JsonReader reader, boolean requiredEndDocument)
throws JsonParseException {
boolean lenient = reader.isLenient();
reader.setLenient(true);
try {
JsonReader jsonReader = new JsonReader(reader);
JsonElement element = parseReader(jsonReader);
if (!element.isJsonNull() && jsonReader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("Did not consume the entire document.");
}
return element;
} catch (MalformedJsonException e) {
throw new JsonSyntaxException(e);
} catch (IOException e) {
throw new JsonIOException(e);
return Streams.parse(reader, requiredEndDocument);
} catch (NumberFormatException e) {
throw new JsonSyntaxException(e);
} catch (StackOverflowError e) {
throw new JsonParseException("Failed parsing JSON source: " + reader + " to Json", e);
} catch (OutOfMemoryError e) {
throw new JsonParseException("Failed parsing JSON source: " + reader + " to Json", e);
} finally {
reader.setLenient(lenient);
}
}

Expand All @@ -94,19 +96,8 @@ public static JsonElement parseReader(Reader reader) throws JsonIOException, Jso
* text is not valid JSON
* @since 2.8.6
*/
public static JsonElement parseReader(JsonReader reader)
throws JsonIOException, JsonSyntaxException {
boolean lenient = reader.isLenient();
reader.setLenient(true);
try {
return Streams.parse(reader);
} catch (StackOverflowError e) {
throw new JsonParseException("Failed parsing JSON source: " + reader + " to Json", e);
} catch (OutOfMemoryError e) {
throw new JsonParseException("Failed parsing JSON source: " + reader + " to Json", e);
} finally {
reader.setLenient(lenient);
}
public static JsonElement parseReader(JsonReader reader) throws JsonParseException {
return parseReader(reader, false);
}

/** @deprecated Use {@link JsonParser#parseString} */
Expand Down
8 changes: 4 additions & 4 deletions gson/src/main/java/com/google/gson/JsonStreamParser.java
Expand Up @@ -61,7 +61,7 @@ public final class JsonStreamParser implements Iterator<JsonElement> {
public JsonStreamParser(String json) {
this(new StringReader(json));
}

/**
* @param reader The data stream containing JSON elements concatenated to each other.
* @since 1.4
Expand All @@ -71,7 +71,7 @@ public JsonStreamParser(Reader reader) {
parser.setLenient(true);
lock = new Object();
}

/**
* Returns the next available {@link JsonElement} on the reader. Throws a
* {@link NoSuchElementException} if no element is available.
Expand All @@ -86,9 +86,9 @@ public JsonElement next() throws JsonParseException {
if (!hasNext()) {
throw new NoSuchElementException();
}

try {
return Streams.parse(parser);
return Streams.parse(parser, false);
} catch (StackOverflowError e) {
throw new JsonParseException("Failed parsing JSON source to Json", e);
} catch (OutOfMemoryError e) {
Expand Down
31 changes: 29 additions & 2 deletions gson/src/main/java/com/google/gson/internal/Streams.java
Expand Up @@ -20,9 +20,11 @@
import com.google.gson.JsonIOException;
import com.google.gson.JsonNull;
import com.google.gson.JsonParseException;
import com.google.gson.JsonParser;
import com.google.gson.JsonSyntaxException;
import com.google.gson.internal.bind.TypeAdapters;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;
import java.io.EOFException;
Expand All @@ -38,15 +40,40 @@ private Streams() {
throw new UnsupportedOperationException();
}

/**
* @deprecated
* This method is declared in an internal Gson class. Use the public API class {@link JsonParser}
* (note that {@code JsonParser} parses JSON in lenient mode), or obtain the {@code TypeAdapter}
* for {@code JsonElement} and use that for parsing:
* <pre>{@code
* TypeAdapter<JsonElement> adapter = gson.getAdapter(JsonElement.class);
* JsonElement element = adapter.read(...);
* }</pre>
*/
// Only keeping this internal method because third-party projects depend on it
@Deprecated
public static JsonElement parse(JsonReader reader) {
return parse(reader, false);
}

/**
* Takes a reader in any state and returns the next value as a JsonElement.
*
* @param requireEndDocument whether there must not be any trailing data after
* the JsonElement
*/
public static JsonElement parse(JsonReader reader) throws JsonParseException {
Copy link
Member

Choose a reason for hiding this comment

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

I'm finding a surprisingly large number of references to this method in Google's source base (including third-party code). In some cases it is because people have copied RuntimeTypeAdapterFactory into their own projects. So I think we probably need to keep this overload. Then of course we could continue to call it from RuntimeTypeAdapterFactory instead of the new (..., false) version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, before #1959 RuntimeTypeAdapterFactory was using the internal Streams class. I have now added back that method, but marked it as deprecated to make users aware that it is declared in an internal Gson class, I hope that is ok.

Then of course we could continue to call it from RuntimeTypeAdapterFactory instead of the new (..., false) version.

The RuntimeTypeAdapterFactory variant which was still using this method was actually a copy of the class from the extras module. I have now removed that copy (and the enclosing test class RuntimeTypeAdapterFactoryFunctionalTest) and have moved the adjusted test code to the RuntimeTypeAdapterFactoryTest class in the extras module to avoid code duplication.

public static JsonElement parse(JsonReader reader, boolean requireEndDocument) throws JsonParseException {
boolean isEmpty = true;
try {
reader.peek();
isEmpty = false;
return TypeAdapters.JSON_ELEMENT.read(reader);
JsonElement element = TypeAdapters.JSON_ELEMENT.read(reader);

if (requireEndDocument && reader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("Did not consume the entire document.");
}
return element;

} catch (EOFException e) {
/*
* For compatibility with JSON 1.5 and earlier, we return a JsonNull for
Expand Down
Expand Up @@ -69,7 +69,7 @@ public TreeTypeAdapter(JsonSerializer<T> serializer, JsonDeserializer<T> deseria
if (deserializer == null) {
return delegate().read(in);
}
JsonElement value = Streams.parse(in);
JsonElement value = Streams.parse(in, false);
if (nullSafe && value.isJsonNull()) {
return null;
}
Expand Down