Improve handling of exception thrown by accessor

Such an exception is not 'unexpected' (which was claimed by the previous exception handling) because user code could throw it.
google · Oct 18, 2022 · e28ad83 · e28ad83
1 parent f499528
commit e28ad83
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 8 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
@@ -42,6 +42,7 @@
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -162,7 +163,7 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField(
     final TypeAdapter<Object> typeAdapter = (TypeAdapter<Object>) mapped;
     return new ReflectiveTypeAdapterFactory.BoundField(name, field.getName(), serialize, deserialize) {
       @Override void write(JsonWriter writer, Object source)
-          throws IOException, ReflectiveOperationException {
+          throws IOException, IllegalAccessException {
         if (!serialized) return;
         if (blockInaccessible) {
           if (accessor == null) {
@@ -174,9 +175,17 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField(
           }
         }
 
-        Object fieldValue = (accessor != null)
-          ? accessor.invoke(source)
-          : field.get(source);
+        Object fieldValue;
+        if (accessor != null) {
+          try {
+            fieldValue = accessor.invoke(source);
+          } catch (InvocationTargetException e) {
+            String accessorDescription = ReflectionHelper.getAccessibleObjectDescription(accessor, false);
+            throw new JsonIOException("Accessor " + accessorDescription + " threw exception", e.getCause());
+          }
+        } else {
+          fieldValue = field.get(source);
+        }
         if (fieldValue == source) {
           // avoid direct recursion
           return;
@@ -314,7 +323,7 @@ protected BoundField(String name, String fieldName, boolean serialized, boolean
     }
 
     /** Read this field value from the source, and append its JSON value to the writer */
-    abstract void write(JsonWriter writer, Object source) throws IOException, ReflectiveOperationException;
+    abstract void write(JsonWriter writer, Object source) throws IOException, IllegalAccessException;
 
     /** Read the value into the target array, used to provide constructor arguments for records */
     abstract void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException, JsonParseException;
@@ -358,8 +367,6 @@ public void write(JsonWriter out, T value) throws IOException {
         }
       } catch (IllegalAccessException e) {
         throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
-      } catch (ReflectiveOperationException e) {
-        throw ReflectionHelper.createExceptionForRecordReflectionException(e);
       }
       out.endObject();
     }

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
@@ -150,7 +150,7 @@ public static RuntimeException createExceptionForUnexpectedIllegalAccess(
   }
 
 
-  public static RuntimeException createExceptionForRecordReflectionException(
+  private static RuntimeException createExceptionForRecordReflectionException(
           ReflectiveOperationException exception) {
     throw new RuntimeException("Unexpected ReflectiveOperationException occurred"
             + " (Gson " + GsonBuildConfig.VERSION + ")."

diff --git a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java
@@ -18,7 +18,9 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.fail;
 
 import com.google.gson.ExclusionStrategy;
 import com.google.gson.FieldAttributes;
@@ -146,6 +148,28 @@ public String s() {
     assertEquals("{\"s\":\"accessor-value\"}", gson.toJson(new LocalRecord(null)));
   }
 
+  /** Tests behavior when a record accessor method throws an exception */
+  @Test
+  public void testThrowingAccessor() {
+    record LocalRecord(String s) {
+      static final RuntimeException thrownException = new RuntimeException("Custom exception");
+
+      @Override
+      public String s() {
+        throw thrownException;
+      }
+    }
+
+    try {
+      gson.toJson(new LocalRecord("a"));
+      fail();
+    } catch (JsonIOException e) {
+      assertEquals("Accessor method '" + LocalRecord.class.getName() + "#s()' threw exception",
+          e.getMessage());
+      assertSame(LocalRecord.thrownException, e.getCause());
+    }
+  }
+
   /** Tests behavior for a record without components */
   @Test
   public void testEmptyRecord() {