Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

### <a name="TOC-Nested-Classes-including-Inner-Classes-"></a>Nested Classes (including Inner Classes)

Copy link
Collaborator Author

@Marcono1234 Marcono1234 Sep 2, 2022

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?

Expand Down
15 changes: 14 additions & 1 deletion gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
Expand Down
13 changes: 0 additions & 13 deletions gson/src/main/java/com/google/gson/internal/Excluder.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,6 @@ public boolean excludeField(Field field, boolean serialize) {
return true;
}

if (isAnonymousOrNonStaticLocal(field.getType())) {
return true;
}

List<ExclusionStrategy> list = serialize ? serializationStrategies : deserializationStrategies;
if (!list.isEmpty()) {
FieldAttributes fieldAttributes = new FieldAttributes(field);
Expand All @@ -198,10 +194,6 @@ private boolean excludeClassChecks(Class<?> clazz) {
return true;
}

if (isAnonymousOrNonStaticLocal(clazz)) {
return true;
}

return false;
}

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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. "
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if UnsupportedOperationException fits well here. Ideally we should really create a proper exception hierarchy for Gson at some point.

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
Expand Up @@ -53,10 +53,10 @@ public void write(JsonWriter out, T value) throws IOException {
if (runtimeType != type) {
@SuppressWarnings("unchecked")
TypeAdapter<T> runtimeTypeAdapter = (TypeAdapter<T>) 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;
Expand Down

This file was deleted.

112 changes: 112 additions & 0 deletions gson/src/test/java/com/google/gson/functional/InnerClassesTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright (C) 2008 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

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 org.junit.Test;

public class InnerClassesTest {
private static final String VALUE = "blah_1234";

private Outer outer = new Outer();

@Test
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);
}

@Test
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);
}

@Test
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)));
}

@Test
public void testInnerClassExclusionDeserialization() {
Gson gson = new GsonBuilder().disableInnerClassSerialization().create();
Outer.Inner deserialized = gson.fromJson("{\"value\":\"a\"}", Outer.Inner.class);
assertNull(deserialized);

WithInnerClassField deserializedWithField = gson.fromJson("{\"inner\":{\"value\":\"a\"}}", WithInnerClassField.class);
deserialized = deserializedWithField.inner;
assertNull(deserialized);
}

private static class Outer {
private class Inner extends NestedClass {
public Inner(String value) {
super(value);
}
}
}

private static class NestedClass {
final String value;
public NestedClass(String value) {
this.value = value;
}

public String toJson() {
return "{\"value\":\"" + value + "\"}";
}
}

private static class WithInnerClassField {
Outer.Inner inner;

WithInnerClassField(Outer.Inner inner) {
this.inner = inner;
}
}
}
Loading