From 6c27553c8375515e9522facf5ea82b8161d5ffb2 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Mon, 5 Dec 2022 02:27:43 +0100 Subject: [PATCH 01/20] Improve exception message for duplicate field names (#2251) --- .../bind/ReflectiveTypeAdapterFactory.java | 16 ++++++------ .../internal/reflect/ReflectionHelper.java | 11 ++++++-- .../gson/functional/NamingPolicyTest.java | 10 +++++--- .../google/gson/functional/ObjectTest.java | 25 +++++++++++++++++++ 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 5ddac50e..4db9bd72 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -139,7 +139,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { } } - private ReflectiveTypeAdapterFactory.BoundField createBoundField( + private BoundField createBoundField( final Gson context, final Field field, final Method accessor, final String name, final TypeToken fieldType, boolean serialize, boolean deserialize, final boolean blockInaccessible) { @@ -161,7 +161,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { @SuppressWarnings("unchecked") final TypeAdapter typeAdapter = (TypeAdapter) mapped; - return new ReflectiveTypeAdapterFactory.BoundField(name, field.getName(), serialize, deserialize) { + return new BoundField(name, field, serialize, deserialize) { @Override void write(JsonWriter writer, Object source) throws IOException, IllegalAccessException { if (!serialized) return; @@ -232,7 +232,6 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { return result; } - Type declaredType = type.getType(); Class originalRaw = raw; while (raw != Object.class) { Field[] fields = raw.getDeclaredFields(); @@ -298,8 +297,9 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { if (previous == null) previous = replaced; } if (previous != null) { - throw new IllegalArgumentException(declaredType - + " declares multiple JSON fields named " + previous.name); + throw new IllegalArgumentException("Class " + originalRaw.getName() + + " declares multiple JSON fields named '" + previous.name + "'; conflict is caused" + + " by fields " + ReflectionHelper.fieldToString(previous.field) + " and " + ReflectionHelper.fieldToString(field)); } } type = TypeToken.get($Gson$Types.resolve(type.getType(), raw, raw.getGenericSuperclass())); @@ -310,14 +310,16 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { static abstract class BoundField { final String name; + final Field field; /** Name of the underlying field */ final String fieldName; final boolean serialized; final boolean deserialized; - protected BoundField(String name, String fieldName, boolean serialized, boolean deserialized) { + protected BoundField(String name, Field field, boolean serialized, boolean deserialized) { this.name = name; - this.fieldName = fieldName; + this.field = field; + this.fieldName = field.getName(); this.serialized = serialized; this.deserialized = deserialized; } diff --git a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java index ac061212..737bb5d4 100644 --- a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java +++ b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java @@ -53,8 +53,7 @@ public class ReflectionHelper { String description; if (object instanceof Field) { - Field field = (Field) object; - description = "field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'"; + description = "field '" + fieldToString((Field) object) + "'"; } else if (object instanceof Method) { Method method = (Method) object; @@ -75,6 +74,14 @@ public class ReflectionHelper { return description; } + /** + * Creates a string representation for a field, omitting modifiers and + * the field type. + */ + public static String fieldToString(Field field) { + return field.getDeclaringClass().getName() + "#" + field.getName(); + } + /** * Creates a string representation for a constructor. * E.g.: {@code java.lang.String(char[], int, int)} diff --git a/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java b/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java index ab76e649..c19ee460 100644 --- a/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java +++ b/gson/src/test/java/com/google/gson/functional/NamingPolicyTest.java @@ -22,10 +22,8 @@ import com.google.gson.GsonBuilder; import com.google.gson.annotations.SerializedName; import com.google.gson.common.TestTypes.ClassWithSerializedNameFields; import com.google.gson.common.TestTypes.StringWrapper; - -import junit.framework.TestCase; - import java.lang.reflect.Field; +import junit.framework.TestCase; /** * Functional tests for naming policies. @@ -122,6 +120,12 @@ public class NamingPolicyTest extends TestCase { gson.toJson(target); fail(); } catch (IllegalArgumentException expected) { + assertEquals( + "Class com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields declares multiple JSON fields named 'a';" + + " conflict is caused by fields com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields#a and" + + " com.google.gson.functional.NamingPolicyTest$ClassWithDuplicateFields#b", + expected.getMessage() + ); } } diff --git a/gson/src/test/java/com/google/gson/functional/ObjectTest.java b/gson/src/test/java/com/google/gson/functional/ObjectTest.java index bed5b598..cf851e06 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -145,6 +145,31 @@ public class ObjectTest extends TestCase { assertEquals(expected, target); } + private static class Subclass extends Superclass1 { + } + private static class Superclass1 extends Superclass2 { + @SuppressWarnings("unused") + String s; + } + private static class Superclass2 { + @SuppressWarnings("unused") + String s; + } + + public void testClassWithDuplicateFields() { + try { + gson.getAdapter(Subclass.class); + fail(); + } catch (IllegalArgumentException e) { + assertEquals( + "Class com.google.gson.functional.ObjectTest$Subclass declares multiple JSON fields named 's';" + + " conflict is caused by fields com.google.gson.functional.ObjectTest$Superclass1#s and" + + " com.google.gson.functional.ObjectTest$Superclass2#s", + e.getMessage() + ); + } + } + public void testNestedSerialization() throws Exception { Nested target = new Nested(new BagOfPrimitives(10, 20, false, "stringValue"), new BagOfPrimitives(30, 40, true, "stringValue")); From e4c3b653a6bdac992d64822796af95ff8e74a625 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Mon, 5 Dec 2022 18:10:36 +0100 Subject: [PATCH 02/20] Fix non-threadsafe creation of adapter for type with cyclic dependency (#1832) * Fix non-threadsafe creation of adapter for type with cyclic dependency * Improve handling of broken adapters during Gson.getAdapter(...) call * Improve test * Slightly improve implementation and extend tests * Simplify getAdapter implementation * Convert GsonTest to JUnit 4 test * Clarify getAdapter concurrency behavior --- gson/src/main/java/com/google/gson/Gson.java | 106 ++++++++----- .../com/google/gson/JsonDeserializer.java | 3 + .../java/com/google/gson/JsonSerializer.java | 3 + .../java/com/google/gson/TypeAdapter.java | 3 + .../test/java/com/google/gson/GsonTest.java | 142 ++++++++++++++++-- 5 files changed, 209 insertions(+), 48 deletions(-) diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index 262cd175..8a26760f 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -155,14 +155,18 @@ public final class Gson { private static final String JSON_NON_EXECUTABLE_PREFIX = ")]}'\n"; /** - * This thread local guards against reentrant calls to getAdapter(). In - * certain object graphs, creating an adapter for a type may recursively + * This thread local guards against reentrant calls to {@link #getAdapter(TypeToken)}. + * In certain object graphs, creating an adapter for a type may recursively * require an adapter for the same type! Without intervention, the recursive - * lookup would stack overflow. We cheat by returning a proxy type adapter. - * The proxy is wired up once the initial adapter has been created. + * lookup would stack overflow. We cheat by returning a proxy type adapter, + * {@link FutureTypeAdapter}, which is wired up once the initial adapter has + * been created. + * + *

The map stores the type adapters for ongoing {@code getAdapter} calls, + * with the type token provided to {@code getAdapter} as key and either + * {@code FutureTypeAdapter} or a regular {@code TypeAdapter} as value. */ - private final ThreadLocal, FutureTypeAdapter>> calls - = new ThreadLocal<>(); + private final ThreadLocal, TypeAdapter>> threadLocalAdapterResults = new ThreadLocal<>(); private final ConcurrentMap, TypeAdapter> typeTokenCache = new ConcurrentHashMap<>(); @@ -509,9 +513,14 @@ public final class Gson { } /** - * Returns the type adapter for {@code} type. + * Returns the type adapter for {@code type}. * - * @throws IllegalArgumentException if this GSON cannot serialize and + *

When calling this method concurrently from multiple threads and requesting + * an adapter for the same type this method may return different {@code TypeAdapter} + * instances. However, that should normally not be an issue because {@code TypeAdapter} + * implementations are supposed to be stateless. + * + * @throws IllegalArgumentException if this Gson instance cannot serialize and * deserialize {@code type}. */ public TypeAdapter getAdapter(TypeToken type) { @@ -523,47 +532,55 @@ public final class Gson { return adapter; } - Map, FutureTypeAdapter> threadCalls = calls.get(); - boolean requiresThreadLocalCleanup = false; + Map, TypeAdapter> threadCalls = threadLocalAdapterResults.get(); + boolean isInitialAdapterRequest = false; if (threadCalls == null) { threadCalls = new HashMap<>(); - calls.set(threadCalls); - requiresThreadLocalCleanup = true; - } - - // the key and value type parameters always agree - @SuppressWarnings("unchecked") - FutureTypeAdapter ongoingCall = (FutureTypeAdapter) threadCalls.get(type); - if (ongoingCall != null) { - return ongoingCall; + threadLocalAdapterResults.set(threadCalls); + isInitialAdapterRequest = true; + } else { + // the key and value type parameters always agree + @SuppressWarnings("unchecked") + TypeAdapter ongoingCall = (TypeAdapter) threadCalls.get(type); + if (ongoingCall != null) { + return ongoingCall; + } } + TypeAdapter candidate = null; try { FutureTypeAdapter call = new FutureTypeAdapter<>(); threadCalls.put(type, call); for (TypeAdapterFactory factory : factories) { - TypeAdapter candidate = factory.create(this, type); + candidate = factory.create(this, type); if (candidate != null) { - @SuppressWarnings("unchecked") - TypeAdapter existingAdapter = (TypeAdapter) typeTokenCache.putIfAbsent(type, candidate); - // If other thread concurrently added adapter prefer that one instead - if (existingAdapter != null) { - candidate = existingAdapter; - } - call.setDelegate(candidate); - return candidate; + // Replace future adapter with actual adapter + threadCalls.put(type, candidate); + break; } } - throw new IllegalArgumentException("GSON (" + GsonBuildConfig.VERSION + ") cannot handle " + type); } finally { - threadCalls.remove(type); - - if (requiresThreadLocalCleanup) { - calls.remove(); + if (isInitialAdapterRequest) { + threadLocalAdapterResults.remove(); } } + + if (candidate == null) { + throw new IllegalArgumentException("GSON (" + GsonBuildConfig.VERSION + ") cannot handle " + type); + } + + if (isInitialAdapterRequest) { + /* + * Publish resolved adapters to all threads + * Can only do this for the initial request because cyclic dependency TypeA -> TypeB -> TypeA + * would otherwise publish adapter for TypeB which uses not yet resolved adapter for TypeA + * See https://github.com/google/gson/issues/625 + */ + typeTokenCache.putAll(threadCalls); + } + return candidate; } /** @@ -641,9 +658,9 @@ public final class Gson { } /** - * Returns the type adapter for {@code} type. + * Returns the type adapter for {@code type}. * - * @throws IllegalArgumentException if this GSON cannot serialize and + * @throws IllegalArgumentException if this Gson instance cannot serialize and * deserialize {@code type}. */ public TypeAdapter getAdapter(Class type) { @@ -1319,19 +1336,32 @@ public final class Gson { return fromJson(new JsonTreeReader(json), typeOfT); } + /** + * Proxy type adapter for cyclic type graphs. + * + *

Important: Setting the delegate adapter is not thread-safe; instances of + * {@code FutureTypeAdapter} must only be published to other threads after the delegate + * has been set. + * + * @see Gson#threadLocalAdapterResults + */ static class FutureTypeAdapter extends SerializationDelegatingTypeAdapter { - private TypeAdapter delegate; + private TypeAdapter delegate = null; public void setDelegate(TypeAdapter typeAdapter) { if (delegate != null) { - throw new AssertionError(); + throw new AssertionError("Delegate is already set"); } delegate = typeAdapter; } private TypeAdapter delegate() { + TypeAdapter delegate = this.delegate; if (delegate == null) { - throw new IllegalStateException("Delegate has not been set yet"); + // Can occur when adapter is leaked to other thread or when adapter is used for (de-)serialization + // directly within the TypeAdapterFactory which requested it + throw new IllegalStateException("Adapter for type with cyclic dependency has been used" + + " before dependency has been resolved"); } return delegate; } diff --git a/gson/src/main/java/com/google/gson/JsonDeserializer.java b/gson/src/main/java/com/google/gson/JsonDeserializer.java index 6462d45c..cef51946 100644 --- a/gson/src/main/java/com/google/gson/JsonDeserializer.java +++ b/gson/src/main/java/com/google/gson/JsonDeserializer.java @@ -63,6 +63,9 @@ import java.lang.reflect.Type; * Gson gson = new GsonBuilder().registerTypeAdapter(Id.class, new IdDeserializer()).create(); * * + *

Deserializers should be stateless and thread-safe, otherwise the thread-safety + * guarantees of {@link Gson} might not apply. + * *

New applications should prefer {@link TypeAdapter}, whose streaming API * is more efficient than this interface's tree API. * diff --git a/gson/src/main/java/com/google/gson/JsonSerializer.java b/gson/src/main/java/com/google/gson/JsonSerializer.java index 2bdb8fb2..01d62723 100644 --- a/gson/src/main/java/com/google/gson/JsonSerializer.java +++ b/gson/src/main/java/com/google/gson/JsonSerializer.java @@ -60,6 +60,9 @@ import java.lang.reflect.Type; * Gson gson = new GsonBuilder().registerTypeAdapter(Id.class, new IdSerializer()).create(); * * + *

Serializers should be stateless and thread-safe, otherwise the thread-safety + * guarantees of {@link Gson} might not apply. + * *

New applications should prefer {@link TypeAdapter}, whose streaming API * is more efficient than this interface's tree API. * diff --git a/gson/src/main/java/com/google/gson/TypeAdapter.java b/gson/src/main/java/com/google/gson/TypeAdapter.java index 98e1668a..5fdea225 100644 --- a/gson/src/main/java/com/google/gson/TypeAdapter.java +++ b/gson/src/main/java/com/google/gson/TypeAdapter.java @@ -81,6 +81,9 @@ import java.io.Writer; * when writing to a JSON object) will be omitted automatically. In either case * your type adapter must handle null. * + *

Type adapters should be stateless and thread-safe, otherwise the thread-safety + * guarantees of {@link Gson} might not apply. + * *

To use a custom type adapter with Gson, you must register it with a * {@link GsonBuilder}:

   {@code
  *
diff --git a/gson/src/test/java/com/google/gson/GsonTest.java b/gson/src/test/java/com/google/gson/GsonTest.java
index 4274d26a..fd335e49 100644
--- a/gson/src/test/java/com/google/gson/GsonTest.java
+++ b/gson/src/test/java/com/google/gson/GsonTest.java
@@ -16,6 +16,11 @@
 
 package com.google.gson;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import com.google.gson.Gson.FutureTypeAdapter;
 import com.google.gson.internal.Excluder;
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
@@ -30,16 +35,17 @@ import java.text.DateFormat;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
-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()
@@ -54,6 +60,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>(), true, false, true, false,
@@ -69,6 +76,7 @@ public final class GsonTest extends TestCase {
     assertEquals(false, gson.htmlSafe());
   }
 
+  @Test
   public void testClonedTypeAdapterFactoryListsAreIndependent() {
     Gson original = new Gson(CUSTOM_EXCLUDER, CUSTOM_FIELD_NAMING_STRATEGY,
         new HashMap>(), true, false, true, false,
@@ -92,6 +100,7 @@ public final class GsonTest extends TestCase {
     @Override public Object read(JsonReader in) throws IOException { return null; }
   }
 
+  @Test
   public void testGetAdapter_Null() {
     Gson gson = new Gson();
     try {
@@ -102,14 +111,15 @@ public final class GsonTest extends TestCase {
     }
   }
 
+  @Test
   public void testGetAdapter_Concurrency() {
     class DummyAdapter extends TypeAdapter {
       @Override public void write(JsonWriter out, T value) throws IOException {
-        throw new AssertionError("not needed for test");
+        throw new AssertionError("not needed for this test");
       }
 
       @Override public T read(JsonReader in) throws IOException {
-        throw new AssertionError("not needed for test");
+        throw new AssertionError("not needed for this test");
       }
     }
 
@@ -149,12 +159,118 @@ public final class GsonTest extends TestCase {
         .create();
 
     TypeAdapter adapter = gson.getAdapter(requestedType);
-    assertTrue(adapter instanceof DummyAdapter);
     assertEquals(2, adapterInstancesCreated.get());
-    // Should be the same adapter instance the concurrent thread received
-    assertSame(threadAdapter.get(), adapter);
+    assertTrue(adapter instanceof DummyAdapter);
+    assertTrue(threadAdapter.get() instanceof DummyAdapter);
   }
 
+  /**
+   * Verifies that two threads calling {@link Gson#getAdapter(TypeToken)} do not see the
+   * same unresolved {@link FutureTypeAdapter} instance, since that would not be thread-safe.
+   *
+   * This test constructs the cyclic dependency {@literal CustomClass1 -> CustomClass2 -> CustomClass1}
+   * and lets one thread wait after the adapter for CustomClass2 has been obtained (which still
+   * refers to the nested unresolved FutureTypeAdapter for CustomClass1).
+   */
+  @Test
+  public void testGetAdapter_FutureAdapterConcurrency() throws Exception {
+    /**
+     * Adapter which wraps another adapter. Can be imagined as a simplified version of the
+     * {@code ReflectiveTypeAdapterFactory$Adapter}.
+     */
+    class WrappingAdapter extends TypeAdapter {
+      final TypeAdapter wrapped;
+      boolean isFirstCall = true;
+
+      WrappingAdapter(TypeAdapter wrapped) {
+        this.wrapped = wrapped;
+      }
+
+      @Override public void write(JsonWriter out, T value) throws IOException {
+        // Due to how this test is set up there is infinite recursion, therefore
+        // need to track how deeply nested this call is
+        if (isFirstCall) {
+          isFirstCall = false;
+          out.beginArray();
+          wrapped.write(out, null);
+          out.endArray();
+          isFirstCall = true;
+        } else {
+          out.value("wrapped-nested");
+        }
+      }
+
+      @Override public T read(JsonReader in) throws IOException {
+        throw new AssertionError("not needed for this test");
+      }
+    }
+
+    final CountDownLatch isThreadWaiting = new CountDownLatch(1);
+    final CountDownLatch canThreadProceed = new CountDownLatch(1);
+
+    final Gson gson = new GsonBuilder()
+      .registerTypeAdapterFactory(new TypeAdapterFactory() {
+        // volatile instead of AtomicBoolean is safe here because CountDownLatch prevents
+        // "true" concurrency
+        volatile boolean isFirstCaller = true;
+
+        @Override
+        public  TypeAdapter create(Gson gson, TypeToken type) {
+          Class raw = type.getRawType();
+
+          if (raw == CustomClass1.class) {
+            // Retrieves a WrappingAdapter containing a nested FutureAdapter for CustomClass1
+            TypeAdapter adapter = gson.getAdapter(CustomClass2.class);
+
+            // Let thread wait so the FutureAdapter for CustomClass1 nested in the adapter
+            // for CustomClass2 is not resolved yet
+            if (isFirstCaller) {
+              isFirstCaller = false;
+              isThreadWaiting.countDown();
+
+              try {
+                canThreadProceed.await();
+              } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+              }
+            }
+
+            return new WrappingAdapter<>(adapter);
+          }
+          else if (raw == CustomClass2.class) {
+            TypeAdapter adapter = gson.getAdapter(CustomClass1.class);
+            assertTrue(adapter instanceof FutureTypeAdapter);
+            return new WrappingAdapter<>(adapter);
+          }
+          else {
+            throw new AssertionError("Adapter for unexpected type requested: " + raw);
+          }
+        }
+      })
+      .create();
+
+    final AtomicReference> otherThreadAdapter = new AtomicReference<>();
+    Thread thread = new Thread() {
+      @Override
+      public void run() {
+        otherThreadAdapter.set(gson.getAdapter(CustomClass1.class));
+      }
+    };
+    thread.start();
+
+    // Wait until other thread has obtained FutureAdapter
+    isThreadWaiting.await();
+    TypeAdapter adapter = gson.getAdapter(CustomClass1.class);
+    // Should not fail due to referring to unresolved FutureTypeAdapter
+    assertEquals("[[\"wrapped-nested\"]]", adapter.toJson(null));
+
+    // Let other thread proceed and have it resolve its FutureTypeAdapter
+    canThreadProceed.countDown();
+    thread.join();
+    assertEquals("[[\"wrapped-nested\"]]", otherThreadAdapter.get().toJson(null));
+  }
+
+  @Test
   public void testNewJsonWriter_Default() throws IOException {
     StringWriter writer = new StringWriter();
     JsonWriter jsonWriter = new Gson().newJsonWriter(writer);
@@ -177,6 +293,7 @@ public final class GsonTest extends TestCase {
     assertEquals("{\"\\u003ctest2\":true}", writer.toString());
   }
 
+  @Test
   public void testNewJsonWriter_Custom() throws IOException {
     StringWriter writer = new StringWriter();
     JsonWriter jsonWriter = new GsonBuilder()
@@ -201,6 +318,7 @@ public final class GsonTest extends TestCase {
     assertEquals(")]}'\n{\n  \"test\": null,\n  \"() {
@@ -353,9 +474,9 @@ public final class GsonTest extends TestCase {
     assertEquals("custom-instance", customClass3.s);
   }
 
-  static class CustomClass1 { }
-  static class CustomClass2 { }
-  static class CustomClass3 {
+  private static class CustomClass1 { }
+  private static class CustomClass2 { }
+  private static class CustomClass3 {
     static final String NO_ARG_CONSTRUCTOR_VALUE = "default instance";
 
     final String s;
@@ -364,6 +485,7 @@ public final class GsonTest extends TestCase {
       this.s = s;
     }
 
+    @SuppressWarnings("unused") // called by Gson
     public CustomClass3() {
       this(NO_ARG_CONSTRUCTOR_VALUE);
     }

From c9c8e8f1bc34eafda7c331862642db79821e393a Mon Sep 17 00:00:00 2001
From: Maicol <79454487+MaicolAntali@users.noreply.github.com>
Date: Mon, 5 Dec 2022 21:59:14 +0100
Subject: [PATCH 03/20] Fix the javadoc of JsonDeserializer.deserialize()
 (#2243)

---
 gson/src/main/java/com/google/gson/JsonDeserializer.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gson/src/main/java/com/google/gson/JsonDeserializer.java b/gson/src/main/java/com/google/gson/JsonDeserializer.java
index cef51946..ca797eee 100644
--- a/gson/src/main/java/com/google/gson/JsonDeserializer.java
+++ b/gson/src/main/java/com/google/gson/JsonDeserializer.java
@@ -83,7 +83,7 @@ public interface JsonDeserializer {
    * 

In the implementation of this call-back method, you should consider invoking * {@link JsonDeserializationContext#deserialize(JsonElement, Type)} method to create objects * for any non-trivial field of the returned object. However, you should never invoke it on the - * the same type passing {@code json} since that will cause an infinite loop (Gson will call your + * same type passing {@code json} since that will cause an infinite loop (Gson will call your * call-back method again). * * @param json The Json data being deserialized From 66d934ba4478035c5abf6b559066ec4a60d82325 Mon Sep 17 00:00:00 2001 From: Maicol <79454487+MaicolAntali@users.noreply.github.com> Date: Tue, 6 Dec 2022 19:43:08 +0100 Subject: [PATCH 04/20] Remove already covered condition in JsonNull.equals() (#2271) --- gson/src/main/java/com/google/gson/JsonNull.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gson/src/main/java/com/google/gson/JsonNull.java b/gson/src/main/java/com/google/gson/JsonNull.java index b14fd3f1..1a4c136c 100644 --- a/gson/src/main/java/com/google/gson/JsonNull.java +++ b/gson/src/main/java/com/google/gson/JsonNull.java @@ -64,6 +64,6 @@ public final class JsonNull extends JsonElement { */ @Override public boolean equals(Object other) { - return this == other || other instanceof JsonNull; + return other instanceof JsonNull; } } From c33e03b133722e84d25c4f80244d74f7fe3f66cb Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 8 Dec 2022 21:29:50 +0200 Subject: [PATCH 05/20] build: harden build.yml permissions (#2274) Signed-off-by: Alex Signed-off-by: Alex --- .github/workflows/build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ef1b23d0..93cde7b5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,6 +2,9 @@ name: Build on: [push, pull_request] +permissions: + contents: read # to fetch code (actions/checkout) + jobs: build: name: "Build on JDK ${{ matrix.java }}" From dcbc164cb0d843dc2c7b67ea5245a93dfca60a43 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 9 Dec 2022 08:03:07 -0800 Subject: [PATCH 06/20] Bump bnd-maven-plugin from 6.3.1 to 6.4.0 (#2245) Bumps [bnd-maven-plugin](https://github.com/bndtools/bnd) from 6.3.1 to 6.4.0. - [Release notes](https://github.com/bndtools/bnd/releases) - [Changelog](https://github.com/bndtools/bnd/blob/master/docs/ADDING_RELEASE_DOCS.md) - [Commits](https://github.com/bndtools/bnd/compare/6.3.1...6.4.0) --- updated-dependencies: - dependency-name: biz.aQute.bnd:bnd-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- gson/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gson/pom.xml b/gson/pom.xml index 8977da14..251f8d01 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -81,7 +81,7 @@ biz.aQute.bnd bnd-maven-plugin - 6.3.1 + 6.4.0 From 28affcbdb9cc77efd6d4cb6cf6cab4acd48f4975 Mon Sep 17 00:00:00 2001 From: Maicol <79454487+MaicolAntali@users.noreply.github.com> Date: Fri, 9 Dec 2022 17:07:35 +0100 Subject: [PATCH 07/20] Remove the `final` keyword from `private` method (#2276) --- .../com/google/gson/internal/bind/DefaultDateTypeAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java index 4719ea15..9e578bf4 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java @@ -62,7 +62,7 @@ public final class DefaultDateTypeAdapter extends TypeAdapter protected abstract T deserialize(Date date); - private final TypeAdapterFactory createFactory(DefaultDateTypeAdapter adapter) { + private TypeAdapterFactory createFactory(DefaultDateTypeAdapter adapter) { return TypeAdapters.newFactory(dateClass, adapter); } From 0a42c31efe265368b71b629a071e7be6c9593d10 Mon Sep 17 00:00:00 2001 From: Maicol <79454487+MaicolAntali@users.noreply.github.com> Date: Tue, 13 Dec 2022 17:50:25 +0100 Subject: [PATCH 08/20] Code cleanup (#2282) * Simplify `if` condition in JsonReader.peekNumber() * Remove `if` to simplify a `return` in Excluder.excludeClassChecks() * Remove redundant variable in Gson.fromJson() * equal condition replace by `Objects.equals()` in $Gson$Types.equal() * equal condition replace by `Objects.equals()` in LinkedTreeMap.equal() * Replace `switch` with `if` in UtcDateTypeAdapter.read() * Remove redundant `throws` clause in GraphAdapterBuilder.read() * Remove redundant `throws` clause in JsonTreeReader.UNREADABLE_READER * Remove redundant `throws` clause in JsonTreeWriter.UNREADABLE_READER * Remove unnecessary `.initCause()` call * Remove redundant cast in TreeTypeAdapter.GsonContextImpl.deserialize * Replace `StringBuilder` with `String` * Fix the import and restore the `switch` * Fix the import * Add the `util.Objects` import * Fix indentation * Add a comment to clarify the condition * Fix indentation * Fix imports * Fix indentation * Fix indentation * Fix indentation --- .../gson/graph/GraphAdapterBuilder.java | 2 +- .../gson/typeadapters/UtcDateTypeAdapter.java | 18 ++++++------- gson/src/main/java/com/google/gson/Gson.java | 25 ++++++------------- .../com/google/gson/internal/$Gson$Types.java | 3 ++- .../com/google/gson/internal/Excluder.java | 6 +---- .../google/gson/internal/LinkedTreeMap.java | 3 ++- .../gson/internal/bind/JsonTreeReader.java | 4 +-- .../gson/internal/bind/JsonTreeWriter.java | 4 +-- .../gson/internal/bind/TreeTypeAdapter.java | 2 +- .../com/google/gson/stream/JsonReader.java | 4 ++- .../google/gson/metrics/BagOfPrimitives.java | 14 +++++------ 11 files changed, 37 insertions(+), 48 deletions(-) diff --git a/extras/src/main/java/com/google/gson/graph/GraphAdapterBuilder.java b/extras/src/main/java/com/google/gson/graph/GraphAdapterBuilder.java index c48c3cd9..b226b220 100644 --- a/extras/src/main/java/com/google/gson/graph/GraphAdapterBuilder.java +++ b/extras/src/main/java/com/google/gson/graph/GraphAdapterBuilder.java @@ -298,7 +298,7 @@ public final class GraphAdapterBuilder { } @SuppressWarnings("unchecked") - void read(Graph graph) throws IOException { + void read(Graph graph) { if (graph.nextCreate != null) { throw new IllegalStateException("Unexpected recursive call to read() for " + id); } diff --git a/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java b/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java index 1e889d37..2278f842 100644 --- a/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java +++ b/extras/src/main/java/com/google/gson/typeadapters/UtcDateTypeAdapter.java @@ -24,10 +24,10 @@ import java.util.Date; import java.util.GregorianCalendar; import java.util.Locale; import java.util.TimeZone; - import com.google.gson.JsonParseException; import com.google.gson.TypeAdapter; import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; public final class UtcDateTypeAdapter extends TypeAdapter { @@ -47,14 +47,14 @@ public final class UtcDateTypeAdapter extends TypeAdapter { public Date read(JsonReader in) throws IOException { try { switch (in.peek()) { - case NULL: - in.nextNull(); - return null; - default: - String date = in.nextString(); - // Instead of using iso8601Format.parse(value), we use Jackson's date parsing - // This is because Android doesn't support XXX because it is JDK 1.6 - return parse(date, new ParsePosition(0)); + case NULL: + in.nextNull(); + return null; + default: + String date = in.nextString(); + // Instead of using iso8601Format.parse(value), we use Jackson's date parsing + // This is because Android doesn't support XXX because it is JDK 1.6 + return parse(date, new ParsePosition(0)); } } catch (ParseException e) { throw new JsonParseException(e); diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index 8a26760f..0339d569 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -843,9 +843,7 @@ public final class Gson { } catch (IOException e) { throw new JsonIOException(e); } catch (AssertionError e) { - AssertionError error = new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage()); - error.initCause(e); - throw error; + throw new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage(), e); } finally { writer.setLenient(oldLenient); writer.setHtmlSafe(oldHtmlSafe); @@ -948,9 +946,7 @@ public final class Gson { } catch (IOException e) { throw new JsonIOException(e); } catch (AssertionError e) { - AssertionError error = new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage()); - error.initCause(e); - throw error; + throw new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage(), e); } finally { writer.setLenient(oldLenient); writer.setHtmlSafe(oldHtmlSafe); @@ -1228,8 +1224,7 @@ public final class Gson { reader.peek(); isEmpty = false; TypeAdapter typeAdapter = getAdapter(typeOfT); - T object = typeAdapter.read(reader); - return object; + return typeAdapter.read(reader); } catch (EOFException e) { /* * For compatibility with JSON 1.5 and earlier, we return null for empty @@ -1245,9 +1240,7 @@ public final class Gson { // TODO(inder): Figure out whether it is indeed right to rethrow this as JsonSyntaxException throw new JsonSyntaxException(e); } catch (AssertionError e) { - AssertionError error = new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage()); - error.initCause(e); - throw error; + throw new AssertionError("AssertionError (GSON " + GsonBuildConfig.VERSION + "): " + e.getMessage(), e); } finally { reader.setLenient(oldLenient); } @@ -1381,11 +1374,9 @@ public final class Gson { @Override public String toString() { - return new StringBuilder("{serializeNulls:") - .append(serializeNulls) - .append(",factories:").append(factories) - .append(",instanceCreators:").append(constructorConstructor) - .append("}") - .toString(); + return "{serializeNulls:" + serializeNulls + + ",factories:" + factories + + ",instanceCreators:" + constructorConstructor + + "}"; } } diff --git a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java index 965e010f..4a925aa4 100644 --- a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java +++ b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.Map; import java.util.NoSuchElementException; import java.util.Properties; +import java.util.Objects; /** * Static methods for working with types. @@ -167,7 +168,7 @@ public final class $Gson$Types { } private static boolean equal(Object a, Object b) { - return a == b || (a != null && a.equals(b)); + return Objects.equals(a, b); } /** diff --git a/gson/src/main/java/com/google/gson/internal/Excluder.java b/gson/src/main/java/com/google/gson/internal/Excluder.java index 03bd45cb..dd167b48 100644 --- a/gson/src/main/java/com/google/gson/internal/Excluder.java +++ b/gson/src/main/java/com/google/gson/internal/Excluder.java @@ -198,11 +198,7 @@ public final class Excluder implements TypeAdapterFactory, Cloneable { return true; } - if (isAnonymousOrNonStaticLocal(clazz)) { - return true; - } - - return false; + return isAnonymousOrNonStaticLocal(clazz); } public boolean excludeClass(Class clazz, boolean serialize) { diff --git a/gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java b/gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java index e47e165d..1fe512ad 100644 --- a/gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java +++ b/gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java @@ -30,6 +30,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.NoSuchElementException; import java.util.Set; +import java.util.Objects; /** * A map of comparable keys to values. Unlike {@code TreeMap}, this class uses @@ -227,7 +228,7 @@ public final class LinkedTreeMap extends AbstractMap implements Seri } private boolean equal(Object a, Object b) { - return a == b || (a != null && a.equals(b)); + return Objects.equals(a, b); } /** diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java index 47e70e68..085090f2 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java @@ -38,10 +38,10 @@ import java.util.Map; */ public final class JsonTreeReader extends JsonReader { private static final Reader UNREADABLE_READER = new Reader() { - @Override public int read(char[] buffer, int offset, int count) throws IOException { + @Override public int read(char[] buffer, int offset, int count) { throw new AssertionError(); } - @Override public void close() throws IOException { + @Override public void close() { throw new AssertionError(); } }; 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 6ff1aa46..a87896da 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 @@ -36,10 +36,10 @@ public final class JsonTreeWriter extends JsonWriter { @Override public void write(char[] buffer, int offset, int counter) { throw new AssertionError(); } - @Override public void flush() throws IOException { + @Override public void flush() { throw new AssertionError(); } - @Override public void close() throws IOException { + @Override public void close() { throw new AssertionError(); } }; diff --git a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java index 560234c0..2efd6c6b 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java +++ b/gson/src/main/java/com/google/gson/internal/bind/TreeTypeAdapter.java @@ -176,7 +176,7 @@ public final class TreeTypeAdapter extends SerializationDelegatingTypeAdapter } @SuppressWarnings("unchecked") @Override public R deserialize(JsonElement json, Type typeOfT) throws JsonParseException { - return (R) gson.fromJson(json, typeOfT); + return gson.fromJson(json, typeOfT); } } } diff --git a/gson/src/main/java/com/google/gson/stream/JsonReader.java b/gson/src/main/java/com/google/gson/stream/JsonReader.java index ed6bab97..1d180eac 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -737,7 +737,9 @@ public class JsonReader implements Closeable { } // We've read a complete number. Decide if it's a PEEKED_LONG or a PEEKED_NUMBER. - if (last == NUMBER_CHAR_DIGIT && fitsInLong && (value != Long.MIN_VALUE || negative) && (value!=0 || false==negative)) { + // Don't store -0 as long; user might want to read it as double -0.0 + // Don't try to convert Long.MIN_VALUE to positive long; it would overflow MAX_VALUE + if (last == NUMBER_CHAR_DIGIT && fitsInLong && (value != Long.MIN_VALUE || negative) && (value!=0 || !negative)) { peekedLong = negative ? value : -value; pos += i; return peeked = PEEKED_LONG; diff --git a/metrics/src/main/java/com/google/gson/metrics/BagOfPrimitives.java b/metrics/src/main/java/com/google/gson/metrics/BagOfPrimitives.java index 008e31f3..a073974d 100644 --- a/metrics/src/main/java/com/google/gson/metrics/BagOfPrimitives.java +++ b/metrics/src/main/java/com/google/gson/metrics/BagOfPrimitives.java @@ -43,14 +43,12 @@ public class BagOfPrimitives { } public String getExpectedJson() { - StringBuilder sb = new StringBuilder(); - sb.append("{"); - sb.append("\"longValue\":").append(longValue).append(","); - sb.append("\"intValue\":").append(intValue).append(","); - sb.append("\"booleanValue\":").append(booleanValue).append(","); - sb.append("\"stringValue\":\"").append(stringValue).append("\""); - sb.append("}"); - return sb.toString(); + return "{" + + "\"longValue\":" + longValue + "," + + "\"intValue\":" + intValue + "," + + "\"booleanValue\":" + booleanValue + "," + + "\"stringValue\":\"" + stringValue + "\"" + + "}"; } @Override From 6c3cf2243581c6ade4d6341b7d46884528261108 Mon Sep 17 00:00:00 2001 From: Maicol <79454487+MaicolAntali@users.noreply.github.com> Date: Tue, 13 Dec 2022 18:33:57 +0100 Subject: [PATCH 09/20] Unnecessary unboxing at JsonPrimitive.getAsBoolean() (#2277) --- gson/src/main/java/com/google/gson/JsonPrimitive.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gson/src/main/java/com/google/gson/JsonPrimitive.java b/gson/src/main/java/com/google/gson/JsonPrimitive.java index 92a8df15..66dacfe5 100644 --- a/gson/src/main/java/com/google/gson/JsonPrimitive.java +++ b/gson/src/main/java/com/google/gson/JsonPrimitive.java @@ -104,7 +104,7 @@ public final class JsonPrimitive extends JsonElement { @Override public boolean getAsBoolean() { if (isBoolean()) { - return ((Boolean) value).booleanValue(); + return (Boolean) value; } // Check to see if the value as a String is "true" in any case. return Boolean.parseBoolean(getAsString()); From f63a1b85aec695f4d464e07a5c13fc038d581527 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 14 Dec 2022 17:33:33 +0100 Subject: [PATCH 10/20] Remove EOFException special casing of JsonStreamParser.next() (#2281) * Remove EOFException special casing of JsonStreamParser.next() The previous behavior violated the Iterator contract where for `JsonStreamParser("[")` a call to `hasNext()` would return true, but `next()` would throw a NoSuchElementException. * Fix incorrect documented thrown exception type for JsonStreamParser --- .../com/google/gson/JsonStreamParser.java | 22 +++--- .../com/google/gson/JsonStreamParserTest.java | 74 ++++++++++++++++--- 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/gson/src/main/java/com/google/gson/JsonStreamParser.java b/gson/src/main/java/com/google/gson/JsonStreamParser.java index 27597da6..2bd86ac5 100644 --- a/gson/src/main/java/com/google/gson/JsonStreamParser.java +++ b/gson/src/main/java/com/google/gson/JsonStreamParser.java @@ -15,18 +15,16 @@ */ package com.google.gson; -import java.io.EOFException; +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; import java.util.Iterator; import java.util.NoSuchElementException; -import com.google.gson.internal.Streams; -import com.google.gson.stream.JsonReader; -import com.google.gson.stream.JsonToken; -import com.google.gson.stream.MalformedJsonException; - /** * A streaming parser that allows reading of multiple {@link JsonElement}s from the specified reader * asynchronously. The JSON data is parsed in lenient mode, see also @@ -61,7 +59,7 @@ public final class JsonStreamParser implements Iterator { public JsonStreamParser(String json) { this(new StringReader(json)); } - + /** * @param reader The data stream containing JSON elements concatenated to each other. * @since 1.4 @@ -71,13 +69,13 @@ public final class JsonStreamParser implements Iterator { parser.setLenient(true); lock = new Object(); } - + /** * Returns the next available {@link JsonElement} on the reader. Throws a * {@link NoSuchElementException} if no element is available. * * @return the next available {@code JsonElement} on the reader. - * @throws JsonSyntaxException if the incoming stream is malformed JSON. + * @throws JsonParseException if the incoming stream is malformed JSON. * @throws NoSuchElementException if no {@code JsonElement} is available. * @since 1.4 */ @@ -86,22 +84,20 @@ public final class JsonStreamParser implements Iterator { if (!hasNext()) { throw new NoSuchElementException(); } - + try { return Streams.parse(parser); } catch (StackOverflowError e) { throw new JsonParseException("Failed parsing JSON source to Json", e); } catch (OutOfMemoryError e) { throw new JsonParseException("Failed parsing JSON source to Json", e); - } catch (JsonParseException e) { - throw e.getCause() instanceof EOFException ? new NoSuchElementException() : e; } } /** * Returns true if a {@link JsonElement} is available on the input for consumption * @return true if a {@link JsonElement} is available on the input, false otherwise - * @throws JsonSyntaxException if the incoming stream is malformed JSON. + * @throws JsonParseException if the incoming stream is malformed JSON. * @since 1.4 */ @Override diff --git a/gson/src/test/java/com/google/gson/JsonStreamParserTest.java b/gson/src/test/java/com/google/gson/JsonStreamParserTest.java index 1b40b58b..402b98b4 100644 --- a/gson/src/test/java/com/google/gson/JsonStreamParserTest.java +++ b/gson/src/test/java/com/google/gson/JsonStreamParserTest.java @@ -15,24 +15,30 @@ */ package com.google.gson; -import junit.framework.TestCase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.io.EOFException; import java.util.NoSuchElementException; +import org.junit.Before; +import org.junit.Test; /** * Unit tests for {@link JsonStreamParser} - * + * * @author Inderjeet Singh */ -public class JsonStreamParserTest extends TestCase { +public class JsonStreamParserTest { private JsonStreamParser parser; - - @Override - protected void setUp() throws Exception { - super.setUp(); + + @Before + public void setUp() throws Exception { parser = new JsonStreamParser("'one' 'two'"); } + @Test public void testParseTwoStrings() { String actualOne = parser.next().getAsString(); assertEquals("one", actualOne); @@ -40,6 +46,7 @@ public class JsonStreamParserTest extends TestCase { assertEquals("two", actualTwo); } + @Test public void testIterator() { assertTrue(parser.hasNext()); assertEquals("one", parser.next().getAsString()); @@ -48,20 +55,22 @@ public class JsonStreamParserTest extends TestCase { assertFalse(parser.hasNext()); } + @Test public void testNoSideEffectForHasNext() throws Exception { assertTrue(parser.hasNext()); assertTrue(parser.hasNext()); assertTrue(parser.hasNext()); assertEquals("one", parser.next().getAsString()); - + assertTrue(parser.hasNext()); assertTrue(parser.hasNext()); assertEquals("two", parser.next().getAsString()); - + assertFalse(parser.hasNext()); assertFalse(parser.hasNext()); } + @Test public void testCallingNextBeyondAvailableInput() { parser.next(); parser.next(); @@ -71,4 +80,51 @@ public class JsonStreamParserTest extends TestCase { } catch (NoSuchElementException expected) { } } + + @Test + public void testEmptyInput() { + JsonStreamParser parser = new JsonStreamParser(""); + try { + parser.next(); + fail(); + } catch (JsonIOException e) { + assertTrue(e.getCause() instanceof EOFException); + } + + parser = new JsonStreamParser(""); + try { + parser.hasNext(); + fail(); + } catch (JsonIOException e) { + assertTrue(e.getCause() instanceof EOFException); + } + } + + @Test + public void testIncompleteInput() { + JsonStreamParser parser = new JsonStreamParser("["); + assertTrue(parser.hasNext()); + try { + parser.next(); + fail(); + } catch (JsonSyntaxException e) { + } + } + + @Test + public void testMalformedInput() { + JsonStreamParser parser = new JsonStreamParser(":"); + try { + parser.hasNext(); + fail(); + } catch (JsonSyntaxException e) { + } + + parser = new JsonStreamParser(":"); + try { + parser.next(); + fail(); + } catch (JsonSyntaxException e) { + } + } } From f2f53fbe8e54020656de67e2511e9b36932963e3 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Thu, 15 Dec 2022 17:27:16 +0100 Subject: [PATCH 11/20] Add troubleshooting guide (#2285) --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- README.md | 7 +- Troubleshooting.md | 192 ++++++++++++++++++ .../com/google/gson/stream/JsonReader.java | 4 +- 4 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 Troubleshooting.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 483fd2ef..7776465c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -1,6 +1,6 @@ --- name: Bug report -about: Report a Gson bug. +about: Report a Gson bug. Please have a look at the troubleshooting guide (Troubleshooting.md) first. title: '' labels: bug assignees: '' diff --git a/README.md b/README.md index dddaf95f..49a02642 100644 --- a/README.md +++ b/README.md @@ -57,9 +57,10 @@ see [`GsonBuilder.disableJdkUnsafe()`](https://javadoc.io/doc/com.google.code.gs ### Documentation * [API Javadoc](https://www.javadoc.io/doc/com.google.code.gson/gson): Documentation for the current release - * [User guide](https://github.com/google/gson/blob/master/UserGuide.md): This guide contains examples on how to use Gson in your code. - * [Change log](https://github.com/google/gson/blob/master/CHANGELOG.md): Changes in the recent versions - * [Design document](https://github.com/google/gson/blob/master/GsonDesignDocument.md): This document discusses issues we faced while designing Gson. It also includes a comparison of Gson with other Java libraries that can be used for Json conversion + * [User guide](UserGuide.md): This guide contains examples on how to use Gson in your code + * [Troubleshooting guide](Troubleshooting.md): Describes how to solve common issues when using Gson + * [Change log](CHANGELOG.md): Changes in the recent versions + * [Design document](GsonDesignDocument.md): This document discusses issues we faced while designing Gson. It also includes a comparison of Gson with other Java libraries that can be used for Json conversion Please use the ['gson' tag on StackOverflow](https://stackoverflow.com/questions/tagged/gson) or the [google-gson Google group](https://groups.google.com/group/google-gson) to discuss Gson or to post questions. diff --git a/Troubleshooting.md b/Troubleshooting.md new file mode 100644 index 00000000..f8749ac6 --- /dev/null +++ b/Troubleshooting.md @@ -0,0 +1,192 @@ +# Troubleshooting Guide + +This guide describes how to troubleshoot common issues when using Gson. + +## `ClassCastException` when using deserialized object + +**Symptom:** `ClassCastException` is thrown when accessing an object deserialized by Gson + +**Reason:** Your code is most likely not type-safe + +**Solution:** Make sure your code adheres to the following: + +- Avoid raw types: Instead of calling `fromJson(..., List.class)`, create for example a `TypeToken>`. + See the [user guide](UserGuide.md#TOC-Collections-Examples) for more information. +- When using `TypeToken` prefer the `Gson.fromJson` overloads with `TypeToken` parameter such as [`fromJson(Reader, TypeToken)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/Gson.html#fromJson(java.io.Reader,com.google.gson.reflect.TypeToken)). + The overloads with `Type` parameter do not provide any type-safety guarantees. +- When using `TypeToken` make sure you don't capture a type variable. For example avoid something like `new TypeToken>()` (where `T` is a type variable). Due to Java type erasure the actual type of `T` is not available at runtime. Refactor your code to pass around `TypeToken` instances or use [`TypeToken.getParameterized(...)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/reflect/TypeToken.html#getParameterized(java.lang.reflect.Type,java.lang.reflect.Type...)), for example `TypeToken.getParameterized(List.class, elementClass)`. + +## `InaccessibleObjectException`: 'module ... does not "opens ..." to unnamed module' + +**Symptom:** An exception with a message in the form 'module ... does not "opens ..." to unnamed module' is thrown + +**Reason:** You use Gson by accident to access internal fields of third-party classes + +**Solution:** Write custom Gson [`TypeAdapter`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) implementations for the affected classes or change the type of your data + +**Explanation:** + +When no built-in adapter for a type exists and no custom adapter has been registered, Gson falls back to using reflection to access the fields of a class (including `private` ones). Most likely you are seeing this error because you (by accident) rely on the reflection-based adapter for third-party classes. That should be avoided because you make yourself dependent on the implementation details of these classes which could change at any point. For the JDK it is also not possible anymore to access internal fields using reflection starting with JDK 17, see [JEP 403](https://openjdk.org/jeps/403). + +If you want to prevent using reflection on third-party classes in the future you can write your own [`ReflectionAccessFilter`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/ReflectionAccessFilter.html) or use one of the predefined ones, such as `ReflectionAccessFilter.BLOCK_ALL_PLATFORM`. + +## `InaccessibleObjectException`: 'module ... does not "opens ..." to module com.google.gson' + +**Symptom:** An exception with a message in the form 'module ... does not "opens ..." to module com.google.gson' is thrown + +**Reason:** + +- If the reported package is your own package then you have not configured the module declaration of your project to allow Gson to use reflection on your classes. +- If the reported package is from a third party library or the JDK see [this troubleshooting point](#inaccessibleobjectexception-module--does-not-opens--to-unnamed-module). + +**Solution:** Make sure the `module-info.java` file of your project allows Gson to use reflection on your classes, for example: + +```java +module mymodule { + requires com.google.gson; + + opens mypackage to com.google.gson; +} +``` + +## Android app not working in Release mode; random property names + +**Symptom:** Your Android app is working fine in Debug mode but fails in Release mode and the JSON properties have seemingly random names such as `a`, `b`, ... + +**Reason:** You probably have not configured ProGuard / R8 correctly + +**Solution:** Make sure you have configured ProGuard / R8 correctly to preserve the names of your fields. See the [Android example](examples/android-proguard-example/README.md) for more information. + +## Android app unable to parse JSON after app update + +**Symptom:** You released a new version of your Android app and it fails to parse JSON data created by the previous version of your app + +**Reason:** You probably have not configured ProGuard / R8 correctly; probably the fields names are being obfuscated and their naming changed between the versions of your app + +**Solution:** Make sure you have configured ProGuard / R8 correctly to preserve the names of your fields. See the [Android example](examples/android-proguard-example/README.md) for more information. + +If you want to preserve backward compatibility for you app you can use [`@SerializedName`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/annotations/SerializedName.html) on the fields to specify the obfuscated name as alternate, for example: `@SerializedName(value = "myprop", alternate = "a")` + +Normally ProGuard and R8 produce a mapping file, this makes it easier to find out the obfuscated field names instead of having to find them out through trial and error or other means. See the [Android Studio user guide](https://developer.android.com/studio/build/shrink-code.html#retracing) for more information. + +## Default field values not present after deserialization + +**Symptom:** You have assign default values to fields but after deserialization the fields have their standard value (such as `null` or `0`) + +**Reason:** Gson cannot invoke the constructor of your class and falls back to JDK `Unsafe` (or similar means) + +**Solution:** Make sure that the class: + +- is `static` (explicitly or implicitly when it is a top-level class) +- has a no-args constructor + +Otherwise Gson will by default try to use JDK `Unsafe` or similar means to create an instance of your class without invoking the constructor and without running any initializers. You can also disable that behavior through [`GsonBuilder.disableJdkUnsafe()`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#disableJdkUnsafe()) to notice such issues early on. + +## `null` values for anonymous and local classes + +**Symptom:** Objects of a class are always serialized as JSON `null` / always deserialized as Java `null` + +**Reason:** The class you are serializing or deserializing is an anonymous or a local class (or you have specified a custom `ExclusionStrategy`) + +**Solution:** Convert the class to a `static` nested class. If the class is already `static` make sure you have not specified a Gson `ExclusionStrategy` which might exclude the class. + +Notes: + +- "double brace-initialization" also creates anonymous classes +- Local record classes (feature added in Java 16) are supported by Gson and are not affected by this + +## Map keys having unexpected format in JSON + +**Symptom:** JSON output for `Map` keys is unexpected / cannot be deserialized again + +**Reason:** The `Map` key type is 'complex' and you have not configured the `GsonBuilder` properly + +**Solution:** Use [`GsonBuilder.enableComplexMapKeySerialization()`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#enableComplexMapKeySerialization()). See also the [user guide](UserGuide.md#TOC-Maps-Examples) for more information. + +## Parsing JSON fails with `MalformedJsonException` + +**Symptom:** JSON parsing fails with `MalformedJsonException` + +**Reason:** The JSON data is actually malformed + +**Solution:** During debugging log the JSON data right before calling Gson methods or set a breakpoint to inspect the data and make sure it has the expected format. Sometimes APIs might return HTML error pages (instead of JSON data) when reaching rate limits or when other errors occur. Also read the location information of the `MalformedJsonException` exception message, it indicates where exactly in the document the malformed data was detected, including the [JSONPath](https://goessner.net/articles/JsonPath/). + +## Integral JSON number is parsed as `double` + +**Symptom:** JSON data contains an integral number such as `45` but Gson returns it as `double` + +**Reason:** When parsing a JSON number as `Object`, Gson will by default create always return a `double` + +**Solution:** Use [`GsonBuilder.setObjectToNumberStrategy`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#setObjectToNumberStrategy(com.google.gson.ToNumberStrategy)) to specify what type of number should be returned + +## Malformed JSON not rejected + +**Symptom:** Gson parses malformed JSON without throwing any exceptions + +**Reason:** Due to legacy reasons Gson performs parsing by default in lenient mode + +**Solution:** See [`Gson` class documentation](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/Gson.html) section "Lenient JSON handling" + +Note: Even in non-lenient mode Gson deviates slightly from the JSON specification, see [`JsonReader.setLenient`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/stream/JsonReader.html#setLenient(boolean)) for more details. + +## `IllegalStateException`: "Expected ... but was ..." + +**Symptom:** An `IllegalStateException` with a message in the form "Expected ... but was ..." is thrown + +**Reason:** The JSON data does not have the correct format + +**Solution:** Make sure that your classes correctly model the JSON data. Also during debugging log the JSON data right before calling Gson methods or set a breakpoint to inspect the data and make sure it has the expected format. Read the location information of the exception message, it indicates where exactly in the document the error occurred, including the [JSONPath](https://goessner.net/articles/JsonPath/). + +## `IllegalStateException`: "Expected ... but was NULL" + +**Symptom:** An `IllegalStateException` with a message in the form "Expected ... but was NULL" is thrown + +**Reason:** You have written a custom `TypeAdapter` which does not properly handle a JSON null value + +**Solution:** Add code similar to the following at the beginning of the `read` method of your adapter: + +```java +@Override +public MyClass read(JsonReader in) throws IOException { + if (in.peek() == JsonToken.NULL) { + in.nextNull(); + return null; + } + + ... +} +``` + +Alternatively you can call [`nullSafe()`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html#nullSafe()) on the adapter instance you created. + +## Properties missing in JSON + +**Symptom:** Properties are missing in the JSON output + +**Reason:** Gson by default omits JSON null from the output (or: ProGuard / R8 is not configured correctly and removed unused fields) + +**Solution:** Use [`GsonBuilder.serializeNulls()`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#serializeNulls()) + +Note: Gson does not support anonymous and local classes and will serialize them as JSON null, see the [related troubleshooting point](#null-values-for-anonymous-and-local-classes). + +## JSON output changes for newer Android versions + +**Symptom:** The JSON output differs when running on newer Android versions + +**Reason:** You use Gson by accident to access internal fields of Android classes + +**Solution:** Write custom Gson [`TypeAdapter`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) implementations for the affected classes or change the type of your data + +**Explanation:** + +When no built-in adapter for a type exists and no custom adapter has been registered, Gson falls back to using reflection to access the fields of a class (including `private` ones). Most likely you are experiencing this issue because you (by accident) rely on the reflection-based adapter for Android classes. That should be avoided because you make yourself dependent on the implementation details of these classes which could change at any point. + +If you want to prevent using reflection on third-party classes in the future you can write your own [`ReflectionAccessFilter`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/ReflectionAccessFilter.html) or use one of the predefined ones, such as `ReflectionAccessFilter.BLOCK_ALL_PLATFORM`. + +## JSON output contains values of `static` fields + +**Symptom:** The JSON output contains values of `static` fields + +**Reason:** You used `GsonBuilder.excludeFieldsWithModifiers` to overwrite the default excluded modifiers + +**Solution:** When calling `GsonBuilder.excludeFieldsWithModifiers` you overwrite the default excluded modifiers. Therefore, you have to explicitly exclude `static` fields if desired. This can be done by adding `| Modifier.STATIC` to the argument. diff --git a/gson/src/main/java/com/google/gson/stream/JsonReader.java b/gson/src/main/java/com/google/gson/stream/JsonReader.java index 1d180eac..718a7c2a 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -1547,7 +1547,7 @@ public class JsonReader implements Closeable { } /** - * Returns a JsonPath + * Returns a JSONPath * in dot-notation to the previous (or current) location in the JSON document: *