From f959dec6ac59fa0e23306a34077cbd7e67f4fc6b Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Fri, 2 Sep 2022 01:02:44 +0200 Subject: [PATCH 1/2] Throw exception when serializing or deserializing anonymous or local classes Backward incompatible changes: - Previously Gson did not throw an exception but simply used null; this was pretty error-prone and difficult to debug for users - Previously Gson ignored whether the type had a custom adapter registered; now Gson only throws an exception when no adapter has been registered and reflection would be used --- UserGuide.md | 3 +- .../java/com/google/gson/GsonBuilder.java | 15 +- .../com/google/gson/internal/Excluder.java | 13 -- .../bind/ReflectiveTypeAdapterFactory.java | 39 ++++- .../bind/TypeAdapterRuntimeTypeWrapper.java | 4 +- ...clusionTest.java => InnerClassesTest.java} | 62 +++++--- .../google/gson/functional/ObjectTest.java | 134 +++++++++++++++++- .../gson/functional/ReflectionAccessTest.java | 3 + 8 files changed, 226 insertions(+), 47 deletions(-) rename gson/src/test/java/com/google/gson/functional/{FieldExclusionTest.java => InnerClassesTest.java} (53%) diff --git a/UserGuide.md b/UserGuide.md index ec7fb7ebad..f6bc669fc8 100644 --- a/UserGuide.md +++ b/UserGuide.md @@ -155,7 +155,8 @@ BagOfPrimitives obj2 = gson.fromJson(json, BagOfPrimitives.class); * While serializing, a null field is omitted from the output. * While deserializing, a missing entry in JSON results in setting the corresponding field in the object to its default value: null for object types, zero for numeric types, and false for booleans. * If a field is _synthetic_, it is ignored and not included in JSON serialization or deserialization. -* Fields corresponding to the outer classes in inner classes, anonymous classes, and local classes are ignored and not included in serialization or deserialization. +* Fields corresponding to the outer classes in inner classes are ignored and not included in serialization or deserialization. +* Reflection-based serialization and deserialization of anonymous and local classes is not supported; an exception will be thrown. Convert the classes to `static` nested classes or register a custom `TypeAdapter` for them to enable serialization and deserialization. ### Nested Classes (including Inner Classes) diff --git a/gson/src/main/java/com/google/gson/GsonBuilder.java b/gson/src/main/java/com/google/gson/GsonBuilder.java index e28da7c7f3..aec67198b9 100644 --- a/gson/src/main/java/com/google/gson/GsonBuilder.java +++ b/gson/src/main/java/com/google/gson/GsonBuilder.java @@ -291,7 +291,20 @@ public GsonBuilder enableComplexMapKeySerialization() { } /** - * Configures Gson to exclude inner classes during serialization. + * Configures Gson to exclude inner classes (= non-{@code static} nested classes) during serialization + * and deserialization. This is a convenience method which behaves as if an {@link ExclusionStrategy} + * which excludes inner classes was registered with this builder. This means inner classes will be + * serialized as JSON {@code null}, and will be deserialized as Java {@code null} with their JSON data + * being ignored. And fields with an inner class as type will be ignored during serialization and + * deserialization. + * + *

By default Gson serializes and deserializes inner classes, but ignores references to the + * enclosing instance. Deserialization might not be possible at all when {@link #disableJdkUnsafe()} + * is used (and no custom {@link InstanceCreator} is registered), or it can lead to unexpected + * {@code NullPointerException}s when the deserialized instance is used. + * + *

In general using inner classes with Gson should be avoided; they should be converted to {@code static} + * nested classes if possible. * * @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern * @since 1.3 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 8d8a25f483..827845c190 100644 --- a/gson/src/main/java/com/google/gson/internal/Excluder.java +++ b/gson/src/main/java/com/google/gson/internal/Excluder.java @@ -172,10 +172,6 @@ public boolean excludeField(Field field, boolean serialize) { return true; } - if (isAnonymousOrNonStaticLocal(field.getType())) { - return true; - } - List list = serialize ? serializationStrategies : deserializationStrategies; if (!list.isEmpty()) { FieldAttributes fieldAttributes = new FieldAttributes(field); @@ -198,10 +194,6 @@ private boolean excludeClassChecks(Class clazz) { return true; } - if (isAnonymousOrNonStaticLocal(clazz)) { - return true; - } - return false; } @@ -220,11 +212,6 @@ private boolean excludeClassInStrategy(Class clazz, boolean serialize) { return false; } - private boolean isAnonymousOrNonStaticLocal(Class clazz) { - return !Enum.class.isAssignableFrom(clazz) && !isStatic(clazz) - && (clazz.isAnonymousClass() || clazz.isLocalClass()); - } - private boolean isInnerClass(Class clazz) { return clazz.isMemberClass() && !isStatic(clazz); } 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 31a44e1a8a..3217d907f2 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 @@ -106,6 +106,13 @@ private List getFieldNames(Field f) { throw new JsonIOException("ReflectionAccessFilter does not permit using reflection for " + raw + ". Register a TypeAdapter for this type or adjust the access filter."); } + + // Check isStatic to allow serialization for static local classes, e.g. record classes (Java 16+) + boolean isAnonymousOrLocal = !Modifier.isStatic(raw.getModifiers()) && (raw.isAnonymousClass() || raw.isLocalClass()); + if (isAnonymousOrLocal) { + return new AnonymousLocalClassAdapter<>(raw); + } + boolean blockInaccessible = filterResult == FilterResult.BLOCK_INACCESSIBLE; ObjectConstructor constructor = constructorConstructor.get(type); @@ -238,7 +245,14 @@ protected BoundField(String name, boolean serialized, boolean deserialized) { abstract void read(JsonReader reader, Object value) throws IOException, IllegalAccessException; } - public static final class Adapter extends TypeAdapter { + /** + * Base class for reflection-based adapters; can be tested for to detect when reflection is used to + * serialize or deserialize a type. + */ + public abstract static class ReflectiveAdapter extends TypeAdapter { + } + + public static class Adapter extends ReflectiveAdapter { private final ObjectConstructor constructor; private final Map boundFields; @@ -292,4 +306,27 @@ public static final class Adapter extends TypeAdapter { out.endObject(); } } + + /** + * Adapter which throws an exception for anonymous and local classes. These types of classes are problematic + * because they might capture values of the enclosing context, which prevents proper deserialization and might + * also be missing information on serialization since synthetic fields are ignored by Gson. + */ + static class AnonymousLocalClassAdapter extends ReflectiveAdapter { + private final Class type; + + AnonymousLocalClassAdapter(Class type) { + this.type = type; + } + + @Override public void write(JsonWriter out, T value) throws IOException { + throw new UnsupportedOperationException("Serialization of anonymous or local class " + type.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class."); + } + + @Override public T read(JsonReader in) throws IOException { + throw new UnsupportedOperationException("Deserialization of anonymous or local class " + type.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class."); + } + } } diff --git a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java index 6a6909191d..dc50a33212 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java +++ b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapterRuntimeTypeWrapper.java @@ -53,10 +53,10 @@ public void write(JsonWriter out, T value) throws IOException { if (runtimeType != type) { @SuppressWarnings("unchecked") TypeAdapter runtimeTypeAdapter = (TypeAdapter) context.getAdapter(TypeToken.get(runtimeType)); - if (!(runtimeTypeAdapter instanceof ReflectiveTypeAdapterFactory.Adapter)) { + if (!(runtimeTypeAdapter instanceof ReflectiveTypeAdapterFactory.ReflectiveAdapter)) { // The user registered a type adapter for the runtime type, so we will use that chosen = runtimeTypeAdapter; - } else if (!(delegate instanceof ReflectiveTypeAdapterFactory.Adapter)) { + } else if (!(delegate instanceof ReflectiveTypeAdapterFactory.ReflectiveAdapter)) { // The user registered a type adapter for Base class, so we prefer it over the // reflective type adapter for the runtime type chosen = delegate; diff --git a/gson/src/test/java/com/google/gson/functional/FieldExclusionTest.java b/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java similarity index 53% rename from gson/src/test/java/com/google/gson/functional/FieldExclusionTest.java rename to gson/src/test/java/com/google/gson/functional/InnerClassesTest.java index 080a8234fe..a71c782e67 100644 --- a/gson/src/test/java/com/google/gson/functional/FieldExclusionTest.java +++ b/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java @@ -18,17 +18,9 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; - import junit.framework.TestCase; -/** - * Performs some functional testing to ensure GSON infrastructure properly serializes/deserializes - * fields that either should or should not be included in the output based on the GSON - * configuration. - * - * @author Joel Leitch - */ -public class FieldExclusionTest extends TestCase { +public class InnerClassesTest extends TestCase { private static final String VALUE = "blah_1234"; private Outer outer; @@ -39,35 +31,54 @@ protected void setUp() throws Exception { outer = new Outer(); } - public void testDefaultInnerClassExclusion() throws Exception { + public void testDefaultInnerClassExclusionSerialization() { Gson gson = new Gson(); Outer.Inner target = outer.new Inner(VALUE); String result = gson.toJson(target); assertEquals(target.toJson(), result); + assertEquals("{\"inner\":" + target.toJson() + "}", gson.toJson(new WithInnerClassField(target))); + gson = new GsonBuilder().create(); target = outer.new Inner(VALUE); result = gson.toJson(target); assertEquals(target.toJson(), result); } - public void testInnerClassExclusion() throws Exception { + public void testDefaultInnerClassExclusionDeserialization() { + Gson gson = new Gson(); + Outer.Inner deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class); + assertNotNull(deserialized); + assertEquals("a", deserialized.value); + + WithInnerClassField deserializedWithField = gson.fromJson("{\"inner\":{\"value\":\"a\"}}", WithInnerClassField.class); + deserialized = deserializedWithField.inner; + assertNotNull(deserialized); + assertEquals("a", deserialized.value); + + gson = new GsonBuilder().create(); + deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class); + assertNotNull(deserialized); + assertEquals("a", deserialized.value); + } + + public void testInnerClassExclusionSerialization() { Gson gson = new GsonBuilder().disableInnerClassSerialization().create(); Outer.Inner target = outer.new Inner(VALUE); String result = gson.toJson(target); assertEquals("null", result); + + assertEquals("{}", gson.toJson(new WithInnerClassField(target))); } - public void testDefaultNestedStaticClassIncluded() throws Exception { - Gson gson = new Gson(); - Outer.Inner target = outer.new Inner(VALUE); - String result = gson.toJson(target); - assertEquals(target.toJson(), result); + public void testInnerClassExclusionDeserialization() { + Gson gson = new GsonBuilder().disableInnerClassSerialization().create(); + Outer.Inner deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class); + assertNull(deserialized); - gson = new GsonBuilder().create(); - target = outer.new Inner(VALUE); - result = gson.toJson(target); - assertEquals(target.toJson(), result); + WithInnerClassField deserializedWithField = gson.fromJson("{\"inner\":{\"value\":\"a\"}}", WithInnerClassField.class); + deserialized = deserializedWithField.inner; + assertNull(deserialized); } private static class Outer { @@ -76,11 +87,10 @@ public Inner(String value) { super(value); } } - } private static class NestedClass { - private final String value; + final String value; public NestedClass(String value) { this.value = value; } @@ -89,4 +99,12 @@ public String toJson() { return "{\"value\":\"" + value + "\"}"; } } + + private static class WithInnerClassField { + Outer.Inner inner; + + WithInnerClassField(Outer.Inner inner) { + this.inner = inner; + } + } } 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 de384879ce..d2e583938c 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -22,6 +22,7 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParseException; +import com.google.gson.JsonPrimitive; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; import com.google.gson.common.TestTypes.ArrayOfObjects; @@ -285,25 +286,144 @@ public void testPrivateNoArgConstructorDeserialization() throws Exception { assertEquals(20, target.a); } - public void testAnonymousLocalClassesSerialization() throws Exception { - assertEquals("null", gson.toJson(new ClassWithNoFields() { + public void testAnonymousClasses() { + Object anonymousClassInstance = new ClassWithNoFields() { // empty anonymous class - })); + }; + Class anonymousClass = anonymousClassInstance.getClass(); + + try { + gson.toJson(anonymousClassInstance); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Serialization of anonymous or local class " + anonymousClass.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } + + try { + gson.fromJson("{}", anonymousClass); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Deserialization of anonymous or local class " + anonymousClass.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } } - public void testAnonymousLocalClassesCustomSerialization() throws Exception { + public void testAnonymousClassesCustomSerialization() { gson = new GsonBuilder() .registerTypeHierarchyAdapter(ClassWithNoFields.class, new JsonSerializer() { @Override public JsonElement serialize( ClassWithNoFields src, Type typeOfSrc, JsonSerializationContext context) { - return new JsonObject(); + return new JsonPrimitive("custom-serializer"); } }).create(); - assertEquals("null", gson.toJson(new ClassWithNoFields() { + Object anonymousClassInstance = new ClassWithNoFields() { // empty anonymous class - })); + }; + Class anonymousClass = anonymousClassInstance.getClass(); + + assertEquals("\"custom-serializer\"", gson.toJson(anonymousClassInstance)); + + // But deserialization should still fail + try { + gson.fromJson("{}", anonymousClass); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Deserialization of anonymous or local class " + anonymousClass.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } + } + + public void testLocalClasses() { + class Local extends ClassWithNoFields { + } + + try { + gson.toJson(new Local()); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Serialization of anonymous or local class " + Local.class.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } + + try { + gson.fromJson("{}", Local.class); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Deserialization of anonymous or local class " + Local.class.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } + } + + public void testLocalClassesCustomSerialization() { + gson = new GsonBuilder() + .registerTypeHierarchyAdapter(ClassWithNoFields.class, + new JsonSerializer() { + @Override public JsonElement serialize( + ClassWithNoFields src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("custom-serializer"); + } + }).create(); + + class Local extends ClassWithNoFields { + } + + assertEquals("\"custom-serializer\"", gson.toJson(new Local())); + + // But deserialization should still fail + try { + gson.fromJson("{}", Local.class); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Deserialization of anonymous or local class " + Local.class.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } + } + + private static class ClassWithNoFieldsContainer { + @SuppressWarnings("unused") + ClassWithNoFields f; + + ClassWithNoFieldsContainer(ClassWithNoFields f) { + this.f = f; + } + } + + public void testLocalClassesCustomSerializationForBaseClass() { + gson = new GsonBuilder() + // Only register adapter for base class + .registerTypeAdapter(ClassWithNoFields.class, + new JsonSerializer() { + @Override public JsonElement serialize( + ClassWithNoFields src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("custom-serializer"); + } + }).create(); + + class Local extends ClassWithNoFields { + } + + // TypeAdapterRuntimeTypeWrapper should prefer adapter for base class over reflective + // adapter for runtime class (= local class) + assertEquals("{\"f\":\"custom-serializer\"}", gson.toJson(new ClassWithNoFieldsContainer(new Local()))); + + // But deserialization should still fail + try { + gson.fromJson("{}", Local.class); + fail(); + } catch (UnsupportedOperationException e) { + assertEquals("Deserialization of anonymous or local class " + Local.class.getName() + " is not supported. " + + "Register a TypeAdapter for the class or convert it to a static nested class.", + e.getMessage()); + } } public void testPrimitiveArrayFieldSerialization() { diff --git a/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java b/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java index ece351240a..fa7eadf83e 100644 --- a/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java +++ b/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java @@ -8,6 +8,7 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonIOException; +import com.google.gson.JsonSyntaxException; import com.google.gson.TypeAdapter; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; @@ -113,6 +114,8 @@ public void testSerializeInternalImplementationObject() { try { gson.fromJson("{}", internalClass); fail("Missing exception; test has to be run with `--illegal-access=deny`"); + } catch (JsonSyntaxException e) { + fail("Unexpected exception; test has to be run with `--illegal-access=deny`"); } catch (JsonIOException expected) { assertTrue(expected.getMessage().startsWith( "Failed making constructor 'java.util.Collections$EmptyList#EmptyList()' accessible; " From 044be429ec3ad8ba80de930e72ba02ab4303cc37 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Fri, 2 Sep 2022 15:41:59 +0200 Subject: [PATCH 2/2] Convert InnerClassesTest to JUnit 4 test --- .../gson/functional/InnerClassesTest.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java b/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java index a71c782e67..793e49e1e6 100644 --- a/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java +++ b/gson/src/test/java/com/google/gson/functional/InnerClassesTest.java @@ -16,21 +16,20 @@ package com.google.gson.functional; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + import com.google.gson.Gson; import com.google.gson.GsonBuilder; -import junit.framework.TestCase; +import org.junit.Test; -public class InnerClassesTest extends TestCase { +public class InnerClassesTest { private static final String VALUE = "blah_1234"; - private Outer outer; - - @Override - protected void setUp() throws Exception { - super.setUp(); - outer = new Outer(); - } + private Outer outer = new Outer(); + @Test public void testDefaultInnerClassExclusionSerialization() { Gson gson = new Gson(); Outer.Inner target = outer.new Inner(VALUE); @@ -45,6 +44,7 @@ public void testDefaultInnerClassExclusionSerialization() { assertEquals(target.toJson(), result); } + @Test public void testDefaultInnerClassExclusionDeserialization() { Gson gson = new Gson(); Outer.Inner deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class); @@ -62,6 +62,7 @@ public void testDefaultInnerClassExclusionDeserialization() { assertEquals("a", deserialized.value); } + @Test public void testInnerClassExclusionSerialization() { Gson gson = new GsonBuilder().disableInnerClassSerialization().create(); Outer.Inner target = outer.new Inner(VALUE); @@ -71,6 +72,7 @@ public void testInnerClassExclusionSerialization() { assertEquals("{}", gson.toJson(new WithInnerClassField(target))); } + @Test public void testInnerClassExclusionDeserialization() { Gson gson = new GsonBuilder().disableInnerClassSerialization().create(); Outer.Inner deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class);