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
61 changes: 29 additions & 32 deletions gson/src/main/java/com/google/gson/Gson.java
Expand Up @@ -38,7 +38,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 @@ -983,8 +982,7 @@ public <T> T fromJson(String json, Type typeOfT) throws JsonSyntaxException {
*/
public <T> T fromJson(Reader json, Class<T> classOfT) throws JsonSyntaxException, JsonIOException {
JsonReader jsonReader = newJsonReader(json);
Object object = fromJson(jsonReader, classOfT);
assertFullConsumption(object, jsonReader);
Object object = fromJson(jsonReader, classOfT, true);
return Primitives.wrap(classOfT).cast(object);
}

Expand Down Expand Up @@ -1013,49 +1011,29 @@ public <T> T fromJson(Reader json, Class<T> classOfT) throws JsonSyntaxException
@SuppressWarnings("unchecked")
public <T> T fromJson(Reader json, Type typeOfT) throws JsonIOException, JsonSyntaxException {
JsonReader jsonReader = newJsonReader(json);
T object = (T) fromJson(jsonReader, typeOfT);
assertFullConsumption(object, jsonReader);
T object = (T) 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);
}
}

/**
* Reads the next JSON value from {@code reader} and convert it to an object
* of type {@code typeOfT}. Returns {@code null}, if the {@code reader} is at EOF.
* Since Type is not parameterized by T, this method is type unsafe and should be used carefully.
*
* <p>Unlike the other {@code fromJson} methods, no exception is thrown if the JSON data has
* multiple top-level JSON elements, or if there is trailing data.
*
* <p>The JSON data is parsed in {@linkplain JsonReader#setLenient(boolean) lenient mode},
* regardless of the lenient mode setting of the provided reader. The lenient mode setting
* of the reader is restored once this method returns.
*
* @throws JsonIOException if there was a problem writing to the Reader
* @throws JsonSyntaxException if json is not a valid representation for an object of type
* @param requireEndDocument whether there must not be any trailing data after
* the first read JSON element
*/
@SuppressWarnings("unchecked")
public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, JsonSyntaxException {
private <T> T fromJson(JsonReader reader, Type typeOfT, boolean requireEndDocument) throws JsonIOException, JsonSyntaxException {
boolean isEmpty = true;
boolean oldLenient = reader.isLenient();
reader.setLenient(true);
try {
reader.peek();
isEmpty = false;
@SuppressWarnings("unchecked") // this is not actually safe
TypeToken<T> typeToken = (TypeToken<T>) TypeToken.get(typeOfT);
TypeAdapter<T> typeAdapter = getAdapter(typeToken);
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 All @@ -1080,6 +1058,25 @@ public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, J
}
}

/**
* Reads the next JSON value from {@code reader} and convert it to an object
* of type {@code typeOfT}. Returns {@code null}, if the {@code reader} is at EOF.
* Since Type is not parameterized by T, this method is type unsafe and should be used carefully.
*
* <p>Unlike the other {@code fromJson} methods, no exception is thrown if the JSON data has
* multiple top-level JSON elements, or if there is trailing data.
*
* <p>The JSON data is parsed in {@linkplain JsonReader#setLenient(boolean) lenient mode},
* regardless of the lenient mode setting of the provided reader. The lenient mode setting
* of the reader is restored once this method returns.
*
* @throws JsonIOException if there was a problem writing to the Reader
* @throws JsonSyntaxException if json is not a valid representation for an object of type
*/
public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, JsonSyntaxException {
return fromJson(reader, typeOfT, false);
}

/**
* This method deserializes the Json read from the specified parse tree into an object of the
* specified type. It is not suitable to use if the specified class is a generic type since it
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 All @@ -46,7 +43,7 @@ public JsonParser() {}
* @return a parse tree of {@link JsonElement}s corresponding to the specified JSON
* @throws JsonParseException if the specified text is not valid JSON
*/
public static JsonElement parseString(String json) throws JsonSyntaxException {
public static JsonElement parseString(String json) throws JsonParseException {
return parseReader(new StringReader(json));
}

Expand All @@ -62,20 +59,25 @@ public static JsonElement parseString(String json) throws JsonSyntaxException {
* @throws JsonParseException if there is an IOException or if the specified
* text is not valid JSON
*/
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 @@ -91,19 +93,8 @@ public static JsonElement parseReader(Reader reader) throws JsonIOException, Jso
* @throws JsonParseException if there is an IOException or if the specified
* text is not valid JSON
*/
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
14 changes: 12 additions & 2 deletions gson/src/main/java/com/google/gson/internal/Streams.java
Expand Up @@ -23,6 +23,7 @@
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 @@ -39,13 +40,22 @@ private Streams() {

/**
* 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 @@ -62,7 +62,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 (value.isJsonNull()) {
return null;
}
Expand Down Expand Up @@ -162,5 +162,5 @@ private final class GsonContextImpl implements JsonSerializationContext, JsonDes
@Override public <R> R deserialize(JsonElement json, Type typeOfT) throws JsonParseException {
return (R) gson.fromJson(json, typeOfT);
}
};
}
}
56 changes: 54 additions & 2 deletions gson/src/test/java/com/google/gson/GsonTest.java
Expand Up @@ -16,11 +16,15 @@

package com.google.gson;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import com.google.gson.internal.Excluder;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.io.StringWriter;
import java.lang.reflect.Field;
Expand All @@ -29,14 +33,14 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import junit.framework.TestCase;
import org.junit.Test;

/**
* Unit tests for {@link Gson}.
*
* @author Ryan Harter
*/
public final class GsonTest extends TestCase {
public final class GsonTest {

private static final Excluder CUSTOM_EXCLUDER = Excluder.DEFAULT
.excludeFieldsWithoutExposeAnnotation()
Expand All @@ -51,6 +55,7 @@ public final class GsonTest extends TestCase {
private static final ToNumberStrategy CUSTOM_OBJECT_TO_NUMBER_STRATEGY = ToNumberPolicy.DOUBLE;
private static final ToNumberStrategy CUSTOM_NUMBER_TO_NUMBER_STRATEGY = ToNumberPolicy.LAZILY_PARSED_NUMBER;

@Test
public void testOverridesDefaultExcluder() {
Gson gson = new Gson(CUSTOM_EXCLUDER, CUSTOM_FIELD_NAMING_STRATEGY,
new HashMap<Type, InstanceCreator<?>>(), true, false, true, false,
Expand All @@ -66,6 +71,7 @@ public void testOverridesDefaultExcluder() {
assertEquals(false, gson.htmlSafe());
}

@Test
public void testClonedTypeAdapterFactoryListsAreIndependent() {
Gson original = new Gson(CUSTOM_EXCLUDER, CUSTOM_FIELD_NAMING_STRATEGY,
new HashMap<Type, InstanceCreator<?>>(), true, false, true, false,
Expand All @@ -89,6 +95,7 @@ private static final class TestTypeAdapter extends TypeAdapter<Object> {
@Override public Object read(JsonReader in) throws IOException { return null; }
}

@Test
public void testNewJsonWriter_Default() throws IOException {
StringWriter writer = new StringWriter();
JsonWriter jsonWriter = new Gson().newJsonWriter(writer);
Expand All @@ -111,6 +118,7 @@ public void testNewJsonWriter_Default() throws IOException {
assertEquals("{\"\\u003ctest2\":true}", writer.toString());
}

@Test
public void testNewJsonWriter_Custom() throws IOException {
StringWriter writer = new StringWriter();
JsonWriter jsonWriter = new GsonBuilder()
Expand All @@ -135,6 +143,7 @@ public void testNewJsonWriter_Custom() throws IOException {
assertEquals(")]}'\n{\n \"test\": null,\n \"<test2\": true\n}1", writer.toString());
}

@Test
public void testNewJsonReader_Default() throws IOException {
String json = "test"; // String without quotes
JsonReader jsonReader = new Gson().newJsonReader(new StringReader(json));
Expand All @@ -146,6 +155,7 @@ public void testNewJsonReader_Default() throws IOException {
jsonReader.close();
}

@Test
public void testNewJsonReader_Custom() throws IOException {
String json = "test"; // String without quotes
JsonReader jsonReader = new GsonBuilder()
Expand All @@ -155,4 +165,46 @@ public void testNewJsonReader_Custom() throws IOException {
assertEquals("test", jsonReader.nextString());
jsonReader.close();
}

@Test
public void testFromJson_Reader_MultipleTopLevel() {
Gson gson = new Gson();
Reader reader = new StringReader("{}1");
try {
gson.fromJson(reader, Object.class);
fail();
} catch (JsonSyntaxException expected) {
assertEquals("JSON document was not fully consumed.", expected.getMessage());
}
}

@Test
public void testFromJson_Reader_MultipleTopLevel_Null() {
Gson gson = new Gson();
Reader reader = new StringReader("null[]");
try {
gson.fromJson(reader, Object.class);
fail();
} catch (JsonSyntaxException expected) {
assertEquals("JSON document was not fully consumed.", expected.getMessage());
}
}

@Test
public void testFromJson_JsonReader_MultipleTopLevel() {
Gson gson = new Gson();
String json = "{}1";
JsonReader jsonReader = new JsonReader(new StringReader(json));
Object object = gson.fromJson(jsonReader, Object.class);
assertEquals(Collections.emptyMap(), object);

object = gson.fromJson(jsonReader, Integer.class);
assertEquals(1, object);

try {
gson.fromJson(jsonReader, Object.class);
fail();
} catch (JsonSyntaxException expected) {
}
}
}