From ed1ea5a755aaa739f2bc91991e32dd6e32a08267 Mon Sep 17 00:00:00 2001 From: Eric Cochran Date: Wed, 21 Feb 2018 17:15:38 -0800 Subject: [PATCH] JsonAdapter.fromJson(String) must fully consume. (#441) * JsonAdapter.fromJson(String) must fully consume. * Replace field with method. --- .../java/com/squareup/moshi/JsonAdapter.java | 26 +++++++- .../com/squareup/moshi/JsonAdapterTest.java | 64 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java index 8b832a4..14927a5 100644 --- a/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/JsonAdapter.java @@ -37,7 +37,12 @@ public abstract class JsonAdapter { } @CheckReturnValue public final @Nullable T fromJson(String string) throws IOException { - return fromJson(new Buffer().writeUtf8(string)); + JsonReader reader = JsonReader.of(new Buffer().writeUtf8(string)); + T result = fromJson(reader); + if (!isLenient() && reader.peek() != JsonReader.Token.END_DOCUMENT) { + throw new JsonDataException("JSON document was not fully consumed."); + } + return result; } public abstract void toJson(JsonWriter writer, @Nullable T value) throws IOException; @@ -109,6 +114,9 @@ public abstract class JsonAdapter { writer.setSerializeNulls(serializeNulls); } } + @Override boolean isLenient() { + return delegate.isLenient(); + } @Override public String toString() { return delegate + ".serializeNulls()"; } @@ -136,6 +144,9 @@ public abstract class JsonAdapter { delegate.toJson(writer, value); } } + @Override boolean isLenient() { + return delegate.isLenient(); + } @Override public String toString() { return delegate + ".nullSafe()"; } @@ -164,6 +175,9 @@ public abstract class JsonAdapter { writer.setLenient(lenient); } } + @Override boolean isLenient() { + return true; + } @Override public String toString() { return delegate + ".lenient()"; } @@ -191,6 +205,9 @@ public abstract class JsonAdapter { @Override public void toJson(JsonWriter writer, @Nullable T value) throws IOException { delegate.toJson(writer, value); } + @Override boolean isLenient() { + return delegate.isLenient(); + } @Override public String toString() { return delegate + ".failOnUnknown()"; } @@ -223,12 +240,19 @@ public abstract class JsonAdapter { writer.setIndent(originalIndent); } } + @Override boolean isLenient() { + return delegate.isLenient(); + } @Override public String toString() { return delegate + ".indent(\"" + indent + "\")"; } }; } + boolean isLenient() { + return false; + } + public interface Factory { /** * Attempts to create an adapter for {@code type} annotated with {@code annotations}. This diff --git a/moshi/src/test/java/com/squareup/moshi/JsonAdapterTest.java b/moshi/src/test/java/com/squareup/moshi/JsonAdapterTest.java index 7adad4e..7cbf785 100644 --- a/moshi/src/test/java/com/squareup/moshi/JsonAdapterTest.java +++ b/moshi/src/test/java/com/squareup/moshi/JsonAdapterTest.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; +import javax.annotation.Nullable; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -182,4 +183,67 @@ public final class JsonAdapterTest { serializeNulls.toJson(writer, Collections.singletonMap("a", null)); assertThat(factory.json()).isEqualTo("{\"a\":null}"); } + + @Test public void stringDocumentMustBeFullyConsumed() throws IOException { + JsonAdapter brokenAdapter = new JsonAdapter() { + @Override public String fromJson(JsonReader reader) throws IOException { + return "Forgot to call reader.nextString()."; + } + + @Override public void toJson(JsonWriter writer, @Nullable String value) throws IOException { + throw new AssertionError(); + } + }; + try { + brokenAdapter.fromJson("\"value\""); + fail(); + } catch (JsonDataException e) { + assertThat(e).hasMessage("JSON document was not fully consumed."); + } + } + + @Test public void adapterFromJsonStringPeeksAtEnd() throws IOException { + JsonAdapter adapter = new JsonAdapter() { + @Override public Boolean fromJson(JsonReader reader) throws IOException { + return reader.nextBoolean(); + } + + @Override public void toJson(JsonWriter writer, @Nullable Boolean value) throws IOException { + throw new AssertionError(); + } + }; + try { + adapter.fromJson("true true"); + fail(); + } catch (JsonEncodingException e) { + assertThat(e).hasMessage( + "Use JsonReader.setLenient(true) to accept malformed JSON at path $"); + } + } + + @Test public void lenientAdapterFromJsonStringDoesNotPeekAtEnd() throws IOException { + JsonAdapter adapter = new JsonAdapter() { + @Override public Boolean fromJson(JsonReader reader) throws IOException { + return reader.nextBoolean(); + } + + @Override public void toJson(JsonWriter writer, @Nullable Boolean value) throws IOException { + throw new AssertionError(); + } + }.lenient(); + assertThat(adapter.fromJson("true true")).isEqualTo(true); + } + + @Test public void adaptersDelegateLeniency() throws IOException { + JsonAdapter adapter = new JsonAdapter() { + @Override public Boolean fromJson(JsonReader reader) throws IOException { + return reader.nextBoolean(); + } + + @Override public void toJson(JsonWriter writer, @Nullable Boolean value) throws IOException { + throw new AssertionError(); + } + }.lenient().nullSafe(); + assertThat(adapter.fromJson("true true")).isEqualTo(true); + } }