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

Support serializing anonymous and local class with custom adapter #2498

60 changes: 31 additions & 29 deletions gson/src/main/java/com/google/gson/internal/Excluder.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.gson.annotations.Expose;
import com.google.gson.annotations.Since;
import com.google.gson.annotations.Until;
import com.google.gson.internal.reflect.ReflectionHelper;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
Expand Down Expand Up @@ -109,18 +110,20 @@ public Excluder withExclusionStrategy(
@Override
public <T> TypeAdapter<T> create(final Gson gson, final TypeToken<T> type) {
Class<?> rawType = type.getRawType();
boolean excludeClass = excludeClassChecks(rawType);

final boolean skipSerialize = excludeClass || excludeClassInStrategy(rawType, true);
final boolean skipDeserialize = excludeClass || excludeClassInStrategy(rawType, false);
final boolean skipSerialize = excludeClass(rawType, true);
final boolean skipDeserialize = excludeClass(rawType, false);

if (!skipSerialize && !skipDeserialize) {
return null;
}

return new TypeAdapter<T>() {
/** The delegate is lazily created because it may not be needed, and creating it may fail. */
private TypeAdapter<T> delegate;
/**
* The delegate is lazily created because it may not be needed, and creating it may fail.
* Field has to be {@code volatile} because {@link Gson} guarantees to be thread-safe.
*/
private volatile TypeAdapter<T> delegate;

@Override
public T read(JsonReader in) throws IOException {
Expand All @@ -141,6 +144,8 @@ public void write(JsonWriter out, T value) throws IOException {
}

private TypeAdapter<T> delegate() {
// A race might lead to `delegate` being assigned by multiple threads but the last
// assignment will stick
TypeAdapter<T> d = delegate;
return d != null ? d : (delegate = gson.getDelegateAdapter(Excluder.this, type));
}
Expand Down Expand Up @@ -168,11 +173,7 @@ public boolean excludeField(Field field, boolean serialize) {
}
}

if (!serializeInnerClasses && isInnerClass(field.getType())) {
return true;
}

if (isAnonymousOrNonStaticLocal(field.getType())) {
if (excludeClass(field.getType(), serialize)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed this to avoid code duplication. This should be identical to the previous behavior, but previously excludeClass was called separately:

private boolean includeField(Field f, boolean serialize) {
return !excluder.excludeClass(f.getType(), serialize) && !excluder.excludeField(f, serialize);
}

(and it looks like there are no other callers of excludeField)

return true;
}

Expand All @@ -189,7 +190,8 @@ public boolean excludeField(Field field, boolean serialize) {
return false;
}

private boolean excludeClassChecks(Class<?> clazz) {
// public for unit tests; can otherwise be private
public boolean excludeClass(Class<?> clazz, boolean serialize) {
if (version != Excluder.IGNORE_VERSIONS
&& !isValidVersion(clazz.getAnnotation(Since.class), clazz.getAnnotation(Until.class))) {
return true;
Expand All @@ -199,14 +201,24 @@ private boolean excludeClassChecks(Class<?> clazz) {
return true;
}

return isAnonymousOrNonStaticLocal(clazz);
}

public boolean excludeClass(Class<?> clazz, boolean serialize) {
return excludeClassChecks(clazz) || excludeClassInStrategy(clazz, serialize);
}
/*
* Exclude anonymous and local classes because they can have synthetic fields capturing enclosing
* values which makes serialization and deserialization unreliable.
* Don't exclude anonymous enum subclasses because enum types have a built-in adapter.
*
* Exclude only for deserialization; for serialization allow because custom adapter might be
* used; if no custom adapter exists reflection-based adapter otherwise excludes value.
*
* Cannot allow deserialization reliably here because some custom adapters like Collection adapter
* fall back to creating instances using Unsafe, which would likely lead to runtime exceptions
* for anonymous and local classes if they capture values.
*/
if (!serialize
&& !Enum.class.isAssignableFrom(clazz)
&& ReflectionHelper.isAnonymousOrNonStaticLocal(clazz)) {
return true;
}

private boolean excludeClassInStrategy(Class<?> clazz, boolean serialize) {
List<ExclusionStrategy> list = serialize ? serializationStrategies : deserializationStrategies;
for (ExclusionStrategy exclusionStrategy : list) {
if (exclusionStrategy.shouldSkipClass(clazz)) {
Expand All @@ -216,18 +228,8 @@ private boolean excludeClassInStrategy(Class<?> clazz, boolean serialize) {
return false;
}

private static boolean isAnonymousOrNonStaticLocal(Class<?> clazz) {
return !Enum.class.isAssignableFrom(clazz)
&& !isStatic(clazz)
&& (clazz.isAnonymousClass() || clazz.isLocalClass());
}

private static boolean isInnerClass(Class<?> clazz) {
return clazz.isMemberClass() && !isStatic(clazz);
}

private static boolean isStatic(Class<?> clazz) {
return (clazz.getModifiers() & Modifier.STATIC) != 0;
return clazz.isMemberClass() && !ReflectionHelper.isStatic(clazz);
}

private boolean isValidVersion(Since since, Until until) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public ReflectiveTypeAdapterFactory(
}

private boolean includeField(Field f, boolean serialize) {
return !excluder.excludeClass(f.getType(), serialize) && !excluder.excludeField(f, serialize);
return !excluder.excludeField(f, serialize);
}

/** first element holds the default name */
Expand Down Expand Up @@ -110,6 +110,31 @@ public <T> TypeAdapter<T> create(Gson gson, final TypeToken<T> type) {
return null; // it's a primitive!
}

// Don't allow using reflection on anonymous and local classes because synthetic fields for
// captured enclosing values make this unreliable
if (ReflectionHelper.isAnonymousOrNonStaticLocal(raw)) {
// This adapter just serializes and deserializes null, ignoring the actual values
// This is done for backward compatibility; troubleshooting-wise it might be better to throw
// exceptions
return new TypeAdapter<T>() {
@Override
public T read(JsonReader in) throws IOException {
in.skipValue();
return null;
}

@Override
public void write(JsonWriter out, T value) throws IOException {
out.nullValue();
}

@Override
public String toString() {
return "AnonymousOrNonStaticLocalClassAdapter";
}
};
}

FilterResult filterResult =
ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw);
if (filterResult == FilterResult.BLOCK_ALL) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;

public class ReflectionHelper {

Expand Down Expand Up @@ -146,6 +147,15 @@ private static void appendExecutableParameters(
stringBuilder.append(')');
}

public static boolean isStatic(Class<?> clazz) {
return Modifier.isStatic(clazz.getModifiers());
}

/** Returns whether the class is anonymous or a non-static local class. */
public static boolean isAnonymousOrNonStaticLocal(Class<?> clazz) {
return !isStatic(clazz) && (clazz.isAnonymousClass() || clazz.isLocalClass());
}

/**
* Tries making the constructor accessible, returning an exception message if this fails.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,38 +31,48 @@
public class ExposeAnnotationExclusionStrategyTest {
private Excluder excluder = Excluder.DEFAULT.excludeFieldsWithoutExposeAnnotation();

private void assertIncludesClass(Class<?> c) {
assertThat(excluder.excludeClass(c, true)).isFalse();
assertThat(excluder.excludeClass(c, false)).isFalse();
}

private void assertIncludesField(Field f) {
assertThat(excluder.excludeField(f, true)).isFalse();
assertThat(excluder.excludeField(f, false)).isFalse();
}

private void assertExcludesField(Field f) {
assertThat(excluder.excludeField(f, true)).isTrue();
assertThat(excluder.excludeField(f, false)).isTrue();
}

@Test
public void testNeverSkipClasses() {
assertThat(excluder.excludeClass(MockObject.class, true)).isFalse();
assertThat(excluder.excludeClass(MockObject.class, false)).isFalse();
assertIncludesClass(MockObject.class);
}

@Test
public void testSkipNonAnnotatedFields() throws Exception {
Field f = createFieldAttributes("hiddenField");
assertThat(excluder.excludeField(f, true)).isTrue();
assertThat(excluder.excludeField(f, false)).isTrue();
assertExcludesField(f);
}

@Test
public void testSkipExplicitlySkippedFields() throws Exception {
Field f = createFieldAttributes("explicitlyHiddenField");
assertThat(excluder.excludeField(f, true)).isTrue();
assertThat(excluder.excludeField(f, false)).isTrue();
assertExcludesField(f);
}

@Test
public void testNeverSkipExposedAnnotatedFields() throws Exception {
Field f = createFieldAttributes("exposedField");
assertThat(excluder.excludeField(f, true)).isFalse();
assertThat(excluder.excludeField(f, false)).isFalse();
assertIncludesField(f);
}

@Test
public void testNeverSkipExplicitlyExposedAnnotatedFields() throws Exception {
Field f = createFieldAttributes("explicitlyExposedField");
assertThat(excluder.excludeField(f, true)).isFalse();
assertThat(excluder.excludeField(f, false)).isFalse();
assertIncludesField(f);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,48 @@ public class InnerClassExclusionStrategyTest {
public StaticNestedClass staticNestedClass = new StaticNestedClass();
private Excluder excluder = Excluder.DEFAULT.disableInnerClassSerialization();

private void assertIncludesClass(Class<?> c) {
assertThat(excluder.excludeClass(c, true)).isFalse();
assertThat(excluder.excludeClass(c, false)).isFalse();
}

private void assertExcludesClass(Class<?> c) {
assertThat(excluder.excludeClass(c, true)).isTrue();
assertThat(excluder.excludeClass(c, false)).isTrue();
}

private void assertIncludesField(Field f) {
assertThat(excluder.excludeField(f, true)).isFalse();
assertThat(excluder.excludeField(f, false)).isFalse();
}

private void assertExcludesField(Field f) {
assertThat(excluder.excludeField(f, true)).isTrue();
assertThat(excluder.excludeField(f, false)).isTrue();
}

@Test
public void testExcludeInnerClassObject() {
Class<?> clazz = innerClass.getClass();
assertThat(excluder.excludeClass(clazz, true)).isTrue();
assertExcludesClass(clazz);
}

@Test
public void testExcludeInnerClassField() throws Exception {
Field f = getClass().getField("innerClass");
assertThat(excluder.excludeField(f, true)).isTrue();
assertExcludesField(f);
}

@Test
public void testIncludeStaticNestedClassObject() {
Class<?> clazz = staticNestedClass.getClass();
assertThat(excluder.excludeClass(clazz, true)).isFalse();
assertIncludesClass(clazz);
}

@Test
public void testIncludeStaticNestedClassField() throws Exception {
Field f = getClass().getField("staticNestedClass");
assertThat(excluder.excludeField(f, true)).isFalse();
assertIncludesField(f);
}

@SuppressWarnings("ClassCanBeStatic")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.gson.annotations.Since;
import com.google.gson.annotations.Until;
import com.google.gson.internal.Excluder;
import java.lang.reflect.Field;
import org.junit.Test;

/**
Expand All @@ -32,44 +33,64 @@
public class VersionExclusionStrategyTest {
private static final double VERSION = 5.0D;

private static void assertIncludesClass(Excluder excluder, Class<?> c) {
assertThat(excluder.excludeClass(c, true)).isFalse();
assertThat(excluder.excludeClass(c, false)).isFalse();
}

private static void assertExcludesClass(Excluder excluder, Class<?> c) {
assertThat(excluder.excludeClass(c, true)).isTrue();
assertThat(excluder.excludeClass(c, false)).isTrue();
}

private static void assertIncludesField(Excluder excluder, Field f) {
assertThat(excluder.excludeField(f, true)).isFalse();
assertThat(excluder.excludeField(f, false)).isFalse();
}

private static void assertExcludesField(Excluder excluder, Field f) {
assertThat(excluder.excludeField(f, true)).isTrue();
assertThat(excluder.excludeField(f, false)).isTrue();
}

@Test
public void testSameVersion() throws Exception {
Excluder excluder = Excluder.DEFAULT.withVersion(VERSION);
assertThat(excluder.excludeClass(MockClassSince.class, true)).isFalse();
assertThat(excluder.excludeField(MockClassSince.class.getField("someField"), true)).isFalse();
assertIncludesClass(excluder, MockClassSince.class);
assertIncludesField(excluder, MockClassSince.class.getField("someField"));

// Until version is exclusive
assertThat(excluder.excludeClass(MockClassUntil.class, true)).isTrue();
assertThat(excluder.excludeField(MockClassUntil.class.getField("someField"), true)).isTrue();
assertExcludesClass(excluder, MockClassUntil.class);
assertExcludesField(excluder, MockClassUntil.class.getField("someField"));

assertThat(excluder.excludeClass(MockClassBoth.class, true)).isFalse();
assertThat(excluder.excludeField(MockClassBoth.class.getField("someField"), true)).isFalse();
assertIncludesClass(excluder, MockClassBoth.class);
assertIncludesField(excluder, MockClassBoth.class.getField("someField"));
}

@Test
public void testNewerVersion() throws Exception {
Excluder excluder = Excluder.DEFAULT.withVersion(VERSION + 5);
assertThat(excluder.excludeClass(MockClassSince.class, true)).isFalse();
assertThat(excluder.excludeField(MockClassSince.class.getField("someField"), true)).isFalse();
assertIncludesClass(excluder, MockClassSince.class);
assertIncludesField(excluder, MockClassSince.class.getField("someField"));

assertThat(excluder.excludeClass(MockClassUntil.class, true)).isTrue();
assertThat(excluder.excludeField(MockClassUntil.class.getField("someField"), true)).isTrue();
assertExcludesClass(excluder, MockClassUntil.class);
assertExcludesField(excluder, MockClassUntil.class.getField("someField"));

assertThat(excluder.excludeClass(MockClassBoth.class, true)).isTrue();
assertThat(excluder.excludeField(MockClassBoth.class.getField("someField"), true)).isTrue();
assertExcludesClass(excluder, MockClassBoth.class);
assertExcludesField(excluder, MockClassBoth.class.getField("someField"));
}

@Test
public void testOlderVersion() throws Exception {
Excluder excluder = Excluder.DEFAULT.withVersion(VERSION - 5);
assertThat(excluder.excludeClass(MockClassSince.class, true)).isTrue();
assertThat(excluder.excludeField(MockClassSince.class.getField("someField"), true)).isTrue();
assertExcludesClass(excluder, MockClassSince.class);
assertExcludesField(excluder, MockClassSince.class.getField("someField"));

assertThat(excluder.excludeClass(MockClassUntil.class, true)).isFalse();
assertThat(excluder.excludeField(MockClassUntil.class.getField("someField"), true)).isFalse();
assertIncludesClass(excluder, MockClassUntil.class);
assertIncludesField(excluder, MockClassUntil.class.getField("someField"));

assertThat(excluder.excludeClass(MockClassBoth.class, true)).isTrue();
assertThat(excluder.excludeField(MockClassBoth.class.getField("someField"), true)).isTrue();
assertExcludesClass(excluder, MockClassBoth.class);
assertExcludesField(excluder, MockClassBoth.class.getField("someField"));
}

@Since(VERSION)
Expand Down
Loading
Loading