issue#2436: Throw exception when registering adapter for Object or Js…

…onElement (google#2479) * Code changes and tests for google#2436 to throw exception when trying to register adapter for Object or JsonElement * google#2436 - Updates to User guide & comments to indicate exception cases and fix for 7 test cases of Parameterized Type * google#2436 - Fixes as per the review comments. * google#2436 - Refactored as per latest review comments + throwing error message. * google#2436 - added a clarifying comment in a positive test case. * google#2436 - formatting and minor changes as per review. * Update gson/src/main/java/com/google/gson/GsonBuilder.java Co-authored-by: Marcono1234 <[email protected]> * Update gson/src/test/java/com/google/gson/GsonBuilderTest.java Co-authored-by: Marcono1234 <[email protected]> * Update gson/src/test/java/com/google/gson/GsonBuilderTest.java Co-authored-by: Marcono1234 <[email protected]> --------- Co-authored-by: Sachin Patil <[email protected]> Co-authored-by: Marcono1234 <[email protected]>
getuniverse · Aug 17, 2024 · 0201ff7 · 0201ff7
1 parent c5a1568
commit 0201ff7
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 2 deletions.
diff --git a/UserGuide.md b/UserGuide.md
@@ -405,7 +405,9 @@ gson.registerTypeAdapter(MyType.class, new MyDeserializer());
 gson.registerTypeAdapter(MyType.class, new MyInstanceCreator());
 ```
 
-`registerTypeAdapter` call checks if the type adapter implements more than one of these interfaces and register it for all of them.
+`registerTypeAdapter` call checks 
+1. if the type adapter implements more than one of these interfaces, in that case it registers the adapter for all of them.
+2. if the type adapter is for the Object class or JsonElement or any of its subclasses, in that case it throws IllegalArgumentException because overriding the built-in adapters for these types is not supported.
 
 #### Writing a Serializer
 

diff --git a/gson/src/main/java/com/google/gson/GsonBuilder.java b/gson/src/main/java/com/google/gson/GsonBuilder.java
@@ -41,6 +41,7 @@
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonWriter;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.text.DateFormat;
 import java.util.ArrayDeque;
@@ -664,6 +665,7 @@ public GsonBuilder setDateFormat(int dateStyle, int timeStyle) {
    * @param typeAdapter This object must implement at least one of the {@link TypeAdapter},
    * {@link InstanceCreator}, {@link JsonSerializer}, and a {@link JsonDeserializer} interfaces.
    * @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
+   * @throws IllegalArgumentException if the type adapter being registered is for {@code Object} class or {@link JsonElement} or any of its subclasses
    */
   @CanIgnoreReturnValue
   public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
@@ -672,6 +674,11 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
         || typeAdapter instanceof JsonDeserializer<?>
         || typeAdapter instanceof InstanceCreator<?>
         || typeAdapter instanceof TypeAdapter<?>);
+
+    if (isTypeObjectOrJsonElement(type)){
+      throw new IllegalArgumentException("Cannot override built-in adapter for " + type);
+    }
+
     if (typeAdapter instanceof InstanceCreator<?>) {
       instanceCreators.put(type, (InstanceCreator<?>) typeAdapter);
     }
@@ -687,6 +694,12 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
     return this;
   }
 
+  private boolean isTypeObjectOrJsonElement(Type type) {
+    return type instanceof Class &&
+            (type == Object.class
+                    || JsonElement.class.isAssignableFrom((Class<?>) type));
+  }
+
   /**
    * Register a factory for type adapters. Registering a factory is useful when the type
    * adapter needs to be configured based on the type of the field being processed. Gson
@@ -718,6 +731,7 @@ public GsonBuilder registerTypeAdapterFactory(TypeAdapterFactory factory) {
    * @param typeAdapter This object must implement at least one of {@link TypeAdapter},
    *        {@link JsonSerializer} or {@link JsonDeserializer} interfaces.
    * @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
+   * @throws IllegalArgumentException if the type adapter being registered is for {@link JsonElement} or any of its subclasses
    * @since 1.7
    */
   @CanIgnoreReturnValue
@@ -726,6 +740,11 @@ public GsonBuilder registerTypeHierarchyAdapter(Class<?> baseType, Object typeAd
     $Gson$Preconditions.checkArgument(typeAdapter instanceof JsonSerializer<?>
         || typeAdapter instanceof JsonDeserializer<?>
         || typeAdapter instanceof TypeAdapter<?>);
+
+    if (JsonElement.class.isAssignableFrom(baseType)) {
+      throw new IllegalArgumentException("Cannot override built-in adapter for " + baseType);
+    }
+
     if (typeAdapter instanceof JsonDeserializer || typeAdapter instanceof JsonSerializer) {
       hierarchyFactories.add(TreeTypeAdapter.newTypeHierarchyFactory(baseType, typeAdapter));
     }

diff --git a/gson/src/test/java/com/google/gson/GsonBuilderTest.java b/gson/src/test/java/com/google/gson/GsonBuilderTest.java
@@ -17,6 +17,7 @@
 package com.google.gson;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.gson.stream.JsonReader;
@@ -254,4 +255,39 @@ public void testSetStrictness() throws IOException {
     assertThat(gson.newJsonReader(new StringReader("{}")).getStrictness()).isEqualTo(STRICTNESS);
     assertThat(gson.newJsonWriter(new StringWriter()).getStrictness()).isEqualTo(STRICTNESS);
   }
+
+  @Test
+  public void testRegisterTypeAdapterForObjectAndJsonElements() {
+    final String ERROR_MESSAGE = "Cannot override built-in adapter for ";
+    Type[] types = {
+        Object.class,
+        JsonElement.class,
+        JsonArray.class,
+    };
+    GsonBuilder gsonBuilder = new GsonBuilder();
+    for (Type type : types) {
+      IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
+          () -> gsonBuilder.registerTypeAdapter(type, NULL_TYPE_ADAPTER));
+      assertThat(e).hasMessageThat().isEqualTo(ERROR_MESSAGE + type);
+    }
+  }
+
+
+  @Test
+  public void testRegisterTypeHierarchyAdapterJsonElements() {
+    final String ERROR_MESSAGE = "Cannot override built-in adapter for ";
+    Class<?>[] types = {
+        JsonElement.class,
+        JsonArray.class,
+    };
+    GsonBuilder gsonBuilder = new GsonBuilder();
+    for (Class<?> type : types) {
+      IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
+          () -> gsonBuilder.registerTypeHierarchyAdapter(type, NULL_TYPE_ADAPTER));
+
+      assertThat(e).hasMessageThat().isEqualTo(ERROR_MESSAGE + type);
+    }
+    // But registering type hierarchy adapter for Object should be allowed
+    gsonBuilder.registerTypeHierarchyAdapter(Object.class, NULL_TYPE_ADAPTER);
+  }
 }
diff --git a/gson/src/test/java/com/google/gson/GsonTest.java b/gson/src/test/java/com/google/gson/GsonTest.java
@@ -93,7 +93,7 @@ public void testClonedTypeAdapterFactoryListsAreIndependent() {
         Collections.<ReflectionAccessFilter>emptyList());
 
     Gson clone = original.newBuilder()
-        .registerTypeAdapter(Object.class, new TestTypeAdapter())
+        .registerTypeAdapter(int.class, new TestTypeAdapter())
         .create();
 
     assertThat(clone.factories).hasSize(original.factories.size() + 1);