-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Throw exception when serializing or deserializing anonymous or local classes #2189
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interestingly this method affects both serialization and deserialization. |
||
* 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. | ||
* | ||
* <p>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. | ||
* | ||
* <p>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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,13 @@ private List<String> 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<T> 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<T> extends TypeAdapter<T> { | ||
/** | ||
* 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<T> extends TypeAdapter<T> { | ||
} | ||
|
||
public static class Adapter<T> extends ReflectiveAdapter<T> { | ||
private final ObjectConstructor<T> constructor; | ||
private final Map<String, BoundField> boundFields; | ||
|
||
|
@@ -292,4 +306,27 @@ public static final class Adapter<T> extends TypeAdapter<T> { | |
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<T> extends ReflectiveAdapter<T> { | ||
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. " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if At least the exact exception type is not specified so I guess we can change it later, if necessary. |
||
+ "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."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed this because it only seems to cover inner classes. |
||
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; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notes below for non-
static
inner classes a few lines below are a bit incorrect, should they be adjusted?