From 07c38bc36502f6f7b5d5b36252617a4abcf28f23 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Tue, 22 Aug 2023 22:09:53 +0200 Subject: [PATCH] Throw exception when calling `JsonWriter.name` outside JSON object Previous fix in 392cc65ff3004689e03b219d9b00979072f9c9d0 only covered writing name as top level value, but not when trying to write name inside JSON array. Removed `stackSize == 0` check from `JsonWriter.name` because that is done already by `peek()` call. --- .../gson/internal/bind/JsonTreeWriter.java | 4 +- .../com/google/gson/stream/JsonWriter.java | 8 +-- .../internal/bind/JsonTreeWriterTest.java | 40 ++++++++++++ .../google/gson/stream/JsonWriterTest.java | 65 +++++++------------ 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java index b1110f1874..f82257287d 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java @@ -139,14 +139,14 @@ private void put(JsonElement value) { @Override public JsonWriter name(String name) throws IOException { Objects.requireNonNull(name, "name == null"); if (stack.isEmpty() || pendingName != null) { - throw new IllegalStateException(); + throw new IllegalStateException("Did not expect a name"); } JsonElement element = peek(); if (element instanceof JsonObject) { pendingName = name; return this; } - throw new IllegalStateException(); + throw new IllegalStateException("Please begin an object before writing a name."); } @CanIgnoreReturnValue diff --git a/gson/src/main/java/com/google/gson/stream/JsonWriter.java b/gson/src/main/java/com/google/gson/stream/JsonWriter.java index bb285bd0c1..00ae92fd1b 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonWriter.java +++ b/gson/src/main/java/com/google/gson/stream/JsonWriter.java @@ -495,11 +495,9 @@ public JsonWriter name(String name) throws IOException { if (deferredName != null) { throw new IllegalStateException("Already wrote a name, expecting a value."); } - if (stackSize == 0) { - throw new IllegalStateException("JsonWriter is closed."); - } - if (stackSize == 1 && (peek() == EMPTY_DOCUMENT || peek() == NONEMPTY_DOCUMENT)) { - throw new IllegalStateException("Please begin an object before this."); + int context = peek(); + if (context != EMPTY_OBJECT && context != NONEMPTY_OBJECT) { + throw new IllegalStateException("Please begin an object before writing a name."); } deferredName = name; return this; diff --git a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java index 75f50469f7..ae65da9356 100644 --- a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java +++ b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java @@ -17,6 +17,7 @@ package com.google.gson.internal.bind; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.gson.JsonElement; @@ -112,6 +113,45 @@ public void testPrematureClose() throws Exception { } } + @Test + public void testNameAsTopLevelValue() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Did not expect a name"); + + writer.value(12); + writer.close(); + + e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + } + + @Test + public void testNameInArray() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + + writer.beginArray(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + writer.value(12); + e = assertThrows(IllegalStateException.class, () -> writer.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + writer.endArray(); + + assertThat(writer.get().toString()).isEqualTo("[12]"); + } + + @Test + public void testTwoNames() throws IOException { + JsonTreeWriter writer = new JsonTreeWriter(); + writer.beginObject(); + writer.name("a"); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("a")); + assertThat(e).hasMessageThat().isEqualTo("Did not expect a name"); + } + @Test public void testSerializeNullsFalse() throws IOException { JsonTreeWriter writer = new JsonTreeWriter(); diff --git a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java index 80bf15f66e..5752876b6f 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonWriterTest.java @@ -28,8 +28,6 @@ import java.math.BigDecimal; import java.math.BigInteger; import org.junit.Test; -import java.util.Arrays; -import java.util.List; @SuppressWarnings("resource") public final class JsonWriterTest { @@ -113,20 +111,36 @@ public void testTopLevelValueTypes() throws IOException { } @Test - public void testInvalidTopLevelTypes() throws IOException { + public void testNameAsTopLevelValue() throws IOException { StringWriter stringWriter = new StringWriter(); JsonWriter jsonWriter = new JsonWriter(stringWriter); - assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.value(12); + jsonWriter.close(); + + e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("JsonWriter is closed."); } @Test - public void closeAllObjectsAndTryToAddElements() throws IOException { - JsonWriter jsonWriterForNameAddition = getJsonWriterWithObjects(); - assertThrows(IllegalStateException.class, () -> jsonWriterForNameAddition.name("this_throw_exception_as_all_objects_are_closed")); - jsonWriterForNameAddition.close(); - JsonWriter jsonWriterForValueAddition = getJsonWriterWithObjects(); - assertThrows(IllegalStateException.class, () -> jsonWriterForValueAddition.value("this_throw_exception_as_only_one_top_level_entry")); - jsonWriterForValueAddition.close(); + public void testNameInArray() throws IOException { + StringWriter stringWriter = new StringWriter(); + JsonWriter jsonWriter = new JsonWriter(stringWriter); + + jsonWriter.beginArray(); + IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.value(12); + e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello")); + assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name."); + + jsonWriter.endArray(); + jsonWriter.close(); + + assertThat(stringWriter.toString()).isEqualTo("[12]"); } @Test @@ -979,33 +993,4 @@ public void testIndentOverwritesFormattingStyle() throws IOException { + "}"; assertThat(stringWriter.toString()).isEqualTo(expected); } - - /** - * This method wites a json object and return a jsonwriter object - * that we can use for the testing purpose - * @return JsonWriter Object with nested object and an array - */ - private JsonWriter getJsonWriterWithObjects() throws IOException { - StringWriter stringWriter = new StringWriter(); - JsonWriter jsonWriter = new JsonWriter(stringWriter); - jsonWriter.beginObject(); - jsonWriter.name("a").value(20); - jsonWriter.name("age").value(30); - - // Start the nested "address" object - jsonWriter.name("address").beginObject(); - jsonWriter.name("city").value("New York"); - jsonWriter.name("country").value("USA"); - jsonWriter.endObject(); // End the nested "address" object - jsonWriter.name("random_prop").value(78); - // Add an array of phone numbers (list of numbers) - List phoneNumbers = Arrays.asList(1234567890, 98989, 9909); - jsonWriter.name("phoneNumbers").beginArray(); - for (Integer phoneNumber : phoneNumbers) { - jsonWriter.value(phoneNumber); - } - jsonWriter.endArray(); // End the array - jsonWriter.endObject(); // End the outer object - return jsonWriter; - } }