Fix failing to serialize Collection or Map with inaccessible constructor (#1902)

* Remove UnsafeReflectionAccessor

Revert #1218

Usage of sun.misc.Unsafe to change internal AccessibleObject.override field
to suppress JPMS warnings goes against the intentions of the JPMS and does not
work anymore in newer versions, see #1540.
Therefore remove it and instead create a descriptive exception when making a
member accessible fails. If necessary users can also still use `java` command
line flags to open external modules.

* Fix failing to serialize Collection or Map with inaccessible constructor

Also remove tests which rely on Java implementation details.

* Don't keep reference to access exception of ConstructorConstructor

This also avoids a confusing stack trace, since the previously caught
exception might have had a complete unrelated stack trace.

* Remove Maven toolchain requirement

* Address review feedback

* Add back test for Security Manager

gson/pom.xml

@@ -1,4 +1,6 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -64,6 +66,21 @@
           <target>1.6</target>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <version>3.0.0-M5</version>
+        <configuration>
+          <!-- Deny illegal access, this is required for ReflectionAccessTest -->
+          <!-- Requires Java >= 9; Important: In case future Java versions 
+            don't support this flag anymore, don't remove it unless CI also runs with 
+            that Java version. Ideally would use toolchain to specify that this should 
+            run with e.g. Java 11, but Maven toolchain requirements (unlike Gradle ones) 
+            don't seem to be portable (every developer would have to set up toolchain 
+            configuration locally). -->
+          <argLine>--illegal-access=deny</argLine>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-javadoc-plugin</artifactId>

gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java

@@ -40,7 +40,7 @@ import java.util.concurrent.ConcurrentSkipListMap;
 
 import com.google.gson.InstanceCreator;
 import com.google.gson.JsonIOException;
-import com.google.gson.internal.reflect.ReflectionAccessor;
+import com.google.gson.internal.reflect.ReflectionHelper;
 import com.google.gson.reflect.TypeToken;
 
 /**
@@ -48,7 +48,6 @@ import com.google.gson.reflect.TypeToken;
  */
 public final class ConstructorConstructor {
   private final Map<Type, InstanceCreator<?>> instanceCreators;
-  private final ReflectionAccessor accessor = ReflectionAccessor.getInstance();
 
   public ConstructorConstructor(Map<Type, InstanceCreator<?>> instanceCreators) {
     this.instanceCreators = instanceCreators;
@@ -97,33 +96,52 @@ public final class ConstructorConstructor {
   }
 
   private <T> ObjectConstructor<T> newDefaultConstructor(Class<? super T> rawType) {
+    final Constructor<? super T> constructor;
     try {
-      final Constructor<? super T> constructor = rawType.getDeclaredConstructor();
+      constructor = rawType.getDeclaredConstructor();
-      if (!constructor.isAccessible()) {
-        accessor.makeAccessible(constructor);
-      }
-      return new ObjectConstructor<T>() {
-        @SuppressWarnings("unchecked") // T is the same raw type as is requested
-        @Override public T construct() {
-          try {
-            Object[] args = null;
-            return (T) constructor.newInstance(args);
-          } catch (InstantiationException e) {
-            // TODO: JsonParseException ?
-            throw new RuntimeException("Failed to invoke " + constructor + " with no args", e);
-          } catch (InvocationTargetException e) {
-            // TODO: don't wrap if cause is unchecked!
-            // TODO: JsonParseException ?
-            throw new RuntimeException("Failed to invoke " + constructor + " with no args",
-                e.getTargetException());
-          } catch (IllegalAccessException e) {
-            throw new AssertionError(e);
-          }
-        }
-      };
     } catch (NoSuchMethodException e) {
       return null;
     }
+
+    final String exceptionMessage = ReflectionHelper.tryMakeAccessible(constructor);
+    if (exceptionMessage != null) {
+      /*
+       * Create ObjectConstructor which throws exception.
+       * This keeps backward compatibility (compared to returning `null` which
+       * would then choose another way of creating object).
+       * And it supports types which are only serialized but not deserialized
+       * (compared to directly throwing exception here), e.g. when runtime type
+       * of object is inaccessible, but compile-time type is accessible.
+       */
+      return new ObjectConstructor<T>() {
+        @Override
+        public T construct() {
+          // New exception is created every time to avoid keeping reference
+          // to exception with potentially long stack trace, causing a
+          // memory leak
+          throw new JsonIOException(exceptionMessage);
+        }
+      };
+    }
+
+    return new ObjectConstructor<T>() {
+      @SuppressWarnings("unchecked") // T is the same raw type as is requested
+      @Override public T construct() {
+        try {
+          return (T) constructor.newInstance();
+        } catch (InstantiationException e) {
+          // TODO: JsonParseException ?
+          throw new RuntimeException("Failed to invoke " + constructor + " with no args", e);
+        } catch (InvocationTargetException e) {
+          // TODO: don't wrap if cause is unchecked!
+          // TODO: JsonParseException ?
+          throw new RuntimeException("Failed to invoke " + constructor + " with no args",
+              e.getTargetException());
+        } catch (IllegalAccessException e) {
+          throw new AssertionError(e);
+        }
+      }
+    };
   }
 
   /**

gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java

@@ -28,7 +28,7 @@ import com.google.gson.internal.ConstructorConstructor;
 import com.google.gson.internal.Excluder;
 import com.google.gson.internal.ObjectConstructor;
 import com.google.gson.internal.Primitives;
-import com.google.gson.internal.reflect.ReflectionAccessor;
+import com.google.gson.internal.reflect.ReflectionHelper;
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonToken;
@@ -50,7 +50,6 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
   private final FieldNamingStrategy fieldNamingPolicy;
   private final Excluder excluder;
   private final JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory;
-  private final ReflectionAccessor accessor = ReflectionAccessor.getInstance();
 
   public ReflectiveTypeAdapterFactory(ConstructorConstructor constructorConstructor,
       FieldNamingStrategy fieldNamingPolicy, Excluder excluder,
@@ -156,7 +155,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
         if (!serialize && !deserialize) {
           continue;
         }
-        accessor.makeAccessible(field);
+        ReflectionHelper.makeAccessible(field);
         Type fieldType = $Gson$Types.resolve(type.getType(), raw, field.getGenericType());
         List<String> fieldNames = getFieldNames(field);
         BoundField previous = null;

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

@@ -17,6 +17,7 @@
 package com.google.gson.internal.bind;
 
 import java.io.IOException;
+import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -759,22 +760,31 @@ public final class TypeAdapters {
     private final Map<String, T> nameToConstant = new HashMap<String, T>();
     private final Map<T, String> constantToName = new HashMap<T, String>();
 
-    public EnumTypeAdapter(Class<T> classOfT) {
+    public EnumTypeAdapter(final Class<T> classOfT) {
       try {
-        for (final Field field : classOfT.getDeclaredFields()) {
+        // Uses reflection to find enum constants to work around name mismatches for obfuscated classes
-          if (!field.isEnumConstant()) {
+        // Reflection access might throw SecurityException, therefore run this in privileged context;
-            continue;
+        // should be acceptable because this only retrieves enum constants, but does not expose anything else
-          }
+        Field[] constantFields = AccessController.doPrivileged(new PrivilegedAction<Field[]>() {
-          AccessController.doPrivileged(new PrivilegedAction<Void>() {
+          @Override public Field[] run() {
-            @Override public Void run() {
+            Field[] fields = classOfT.getDeclaredFields();
-              field.setAccessible(true);
+            ArrayList<Field> constantFieldsList = new ArrayList<Field>(fields.length);
-              return null;
+            for (Field f : fields) {
+              if (f.isEnumConstant()) {
+                constantFieldsList.add(f);
+              }
             }
-          });
+
+            Field[] constantFields = constantFieldsList.toArray(new Field[0]);
+            AccessibleObject.setAccessible(constantFields, true);
+            return constantFields;
+          }
+        });
+        for (Field constantField : constantFields) {
           @SuppressWarnings("unchecked")
-          T constant = (T)(field.get(null));
+          T constant = (T)(constantField.get(null));
           String name = constant.name();
-          SerializedName annotation = field.getAnnotation(SerializedName.class);
+          SerializedName annotation = constantField.getAnnotation(SerializedName.class);
           if (annotation != null) {
             name = annotation.value();
             for (String alternate : annotation.alternate()) {

gson/src/main/java/com/google/gson/internal/reflect/PreJava9ReflectionAccessor.java

@@ -1,33 +0,0 @@
-/*
- * Copyright (C) 2017 The Gson authors
- *
- * 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.internal.reflect;
-
-import java.lang.reflect.AccessibleObject;
-
-/**
- * A basic implementation of {@link ReflectionAccessor} which is suitable for Java 8 and below.
- * <p>
- * This implementation just calls {@link AccessibleObject#setAccessible(boolean) setAccessible(true)}, which worked
- * fine before Java 9.
- */
-final class PreJava9ReflectionAccessor extends ReflectionAccessor {
-
-  /** {@inheritDoc} */
-  @Override
-  public void makeAccessible(AccessibleObject ao) {
-    ao.setAccessible(true);
-  }
-}

gson/src/main/java/com/google/gson/internal/reflect/ReflectionAccessor.java

@@ -1,54 +0,0 @@
-/*
- * Copyright (C) 2017 The Gson authors
- *
- * 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.internal.reflect;
-
-import java.lang.reflect.AccessibleObject;
-
-import com.google.gson.internal.JavaVersion;
-
-/**
- * Provides a replacement for {@link AccessibleObject#setAccessible(boolean)}, which may be used to
- * avoid reflective access issues appeared in Java 9, like {@link java.lang.reflect.InaccessibleObjectException}
- * thrown or warnings like
- * <pre>
- *   WARNING: An illegal reflective access operation has occurred
- *   WARNING: Illegal reflective access by ...
- * </pre>
- * <p/>
- * Works both for Java 9 and earlier Java versions.
- */
-public abstract class ReflectionAccessor {
-
-  // the singleton instance, use getInstance() to obtain
-  private static final ReflectionAccessor instance = JavaVersion.getMajorJavaVersion() < 9 ? new PreJava9ReflectionAccessor() : new UnsafeReflectionAccessor();
-
-  /**
-   * Does the same as {@code ao.setAccessible(true)}, but never throws
-   * {@link java.lang.reflect.InaccessibleObjectException}
-   */
-  public abstract void makeAccessible(AccessibleObject ao);
-
-  /**
-   * Obtains a {@link ReflectionAccessor} instance suitable for the current Java version.
-   * <p>
-   * You may need one a reflective operation in your code throws {@link java.lang.reflect.InaccessibleObjectException}.
-   * In such a case, use {@link ReflectionAccessor#makeAccessible(AccessibleObject)} on a field, method or constructor
-   * (instead of basic {@link AccessibleObject#setAccessible(boolean)}).
-   */
-  public static ReflectionAccessor getInstance() {
-    return instance;
-  }
-}

gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java

@@ -0,0 +1,66 @@
+package com.google.gson.internal.reflect;
+
+import com.google.gson.JsonIOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+
+public class ReflectionHelper {
+  private ReflectionHelper() { }
+
+  /**
+   * Tries making the field accessible, wrapping any thrown exception in a
+   * {@link JsonIOException} with descriptive message.
+   *
+   * @param field field to make accessible
+   * @throws JsonIOException if making the field accessible fails
+   */
+  public static void makeAccessible(Field field) throws JsonIOException {
+    try {
+      field.setAccessible(true);
+    } catch (Exception exception) {
+      throw new JsonIOException("Failed making field '" + field.getDeclaringClass().getName() + "#"
+          + field.getName() + "' accessible; either change its visibility or write a custom "
+          + "TypeAdapter for its declaring type", exception);
+    }
+  }
+
+  /**
+   * Creates a string representation for a constructor.
+   * E.g.: {@code java.lang.String#String(char[], int, int)}
+   */
+  private static String constructorToString(Constructor<?> constructor) {
+    StringBuilder stringBuilder = new StringBuilder(constructor.getDeclaringClass().getName())
+      .append('#')
+      .append(constructor.getDeclaringClass().getSimpleName())
+      .append('(');
+    Class<?>[] parameters = constructor.getParameterTypes();
+    for (int i = 0; i < parameters.length; i++) {
+      if (i > 0) {
+        stringBuilder.append(", ");
+      }
+      stringBuilder.append(parameters[i].getSimpleName());
+    }
+
+    return stringBuilder.append(')').toString();
+  }
+
+  /**
+   * Tries making the constructor accessible, returning an exception message
+   * if this fails.
+   *
+   * @param constructor constructor to make accessible
+   * @return exception message; {@code null} if successful, non-{@code null} if
+   *    unsuccessful
+   */
+  public static String tryMakeAccessible(Constructor<?> constructor) {
+    try {
+      constructor.setAccessible(true);
+      return null;
+    } catch (Exception exception) {
+      return "Failed making constructor '" + constructorToString(constructor) + "' accessible; "
+          + "either change its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type: "
+          // Include the message since it might contain more detailed information
+          + exception.getMessage();
+    }
+  }
+}

gson/src/main/java/com/google/gson/internal/reflect/UnsafeReflectionAccessor.java

@@ -1,86 +0,0 @@
-/*
- * Copyright (C) 2017 The Gson authors
- *
- * 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.internal.reflect;
-
-import java.lang.reflect.AccessibleObject;
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-
-import com.google.gson.JsonIOException;
-
-/**
- * An implementation of {@link ReflectionAccessor} based on {@link Unsafe}.
- * <p>
- * NOTE: This implementation is designed for Java 9. Although it should work with earlier Java releases, it is better to
- * use {@link PreJava9ReflectionAccessor} for them.
- */
-@SuppressWarnings({"unchecked", "rawtypes"})
-final class UnsafeReflectionAccessor extends ReflectionAccessor {
-
-  private static Class unsafeClass;
-  private final Object theUnsafe = getUnsafeInstance();
-  private final Field overrideField = getOverrideField();
-
-  /** {@inheritDoc} */
-  @Override
-  public void makeAccessible(AccessibleObject ao) {
-    boolean success = makeAccessibleWithUnsafe(ao);
-    if (!success) {
-      try {
-        // unsafe couldn't be found, so try using accessible anyway
-        ao.setAccessible(true);
-      } catch (SecurityException e) {
-        throw new JsonIOException("Gson couldn't modify fields for " + ao
-          + "\nand sun.misc.Unsafe not found.\nEither write a custom type adapter,"
-          + " or make fields accessible, or include sun.misc.Unsafe.", e);
-      }
-    }
-  }
-
-  // Visible for testing only
-  boolean makeAccessibleWithUnsafe(AccessibleObject ao) {
-    if (theUnsafe != null && overrideField != null) {
-      try {
-        Method method = unsafeClass.getMethod("objectFieldOffset", Field.class);
-        long overrideOffset = (Long) method.invoke(theUnsafe, overrideField);  // long overrideOffset = theUnsafe.objectFieldOffset(overrideField);
-        Method putBooleanMethod = unsafeClass.getMethod("putBoolean",  Object.class, long.class, boolean.class);
-        putBooleanMethod.invoke(theUnsafe, ao, overrideOffset, true); // theUnsafe.putBoolean(ao, overrideOffset, true);
-        return true;
-      } catch (Exception ignored) { // do nothing
-      }
-    }
-    return false;
-  }
-
-  private static Object getUnsafeInstance() {
-    try {
-      unsafeClass = Class.forName("sun.misc.Unsafe");
-      Field unsafeField = unsafeClass.getDeclaredField("theUnsafe");
-      unsafeField.setAccessible(true);
-      return unsafeField.get(null);
-    } catch (Exception e) {
-      return null;
-    }
-  }
-
-  private static Field getOverrideField() {
-    try {
-      return AccessibleObject.class.getDeclaredField("override");
-    } catch (Exception e) {
-      return null;
-    }
-  }
-}

gson/src/test/java/com/google/gson/functional/DefaultTypeAdaptersTest.java

@@ -180,7 +180,6 @@ public class DefaultTypeAdaptersTest extends TestCase {
     testNullSerializationAndDeserialization(Date.class);
     testNullSerializationAndDeserialization(GregorianCalendar.class);
     testNullSerializationAndDeserialization(Calendar.class);
-    testNullSerializationAndDeserialization(Enum.class);
     testNullSerializationAndDeserialization(Class.class);
   }
 

gson/src/test/java/com/google/gson/functional/EnumTest.java

@@ -16,12 +16,6 @@
 
 package com.google.gson.functional;
 
-import java.lang.reflect.Type;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.EnumSet;
-import java.util.Set;
-
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonDeserializationContext;
@@ -34,7 +28,11 @@ import com.google.gson.JsonSerializer;
 import com.google.gson.annotations.SerializedName;
 import com.google.gson.common.MoreAsserts;
 import com.google.gson.reflect.TypeToken;
-
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.EnumSet;
+import java.util.Set;
 import junit.framework.TestCase;
 /**
  * Functional tests for Java 5.0 enums.
@@ -200,17 +198,17 @@ public class EnumTest extends TestCase {
   }
 
   public void testEnumClassWithFields() {
-	  assertEquals("\"RED\"", gson.toJson(Color.RED));
+    assertEquals("\"RED\"", gson.toJson(Color.RED));
-	  assertEquals("red", gson.fromJson("RED", Color.class).value);
+    assertEquals("red", gson.fromJson("RED", Color.class).value);
   }
 
   public enum Color {
-	  RED("red", 1), BLUE("blue", 2), GREEN("green", 3);
+    RED("red", 1), BLUE("blue", 2), GREEN("green", 3);
-	  String value;
+    String value;
-	  int index;
+    int index;
-	  private Color(String value, int index) {
+    private Color(String value, int index) {
-		  this.value = value;
+      this.value = value;
-		  this.index = index;
+      this.index = index;
-	  }
+    }
   }
 }

gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java

@@ -0,0 +1,123 @@
+package com.google.gson.functional;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonIOException;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
+import java.io.IOException;
+import java.lang.reflect.ReflectPermission;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.security.Permission;
+import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.junit.Test;
+
+public class ReflectionAccessTest {
+  @SuppressWarnings("unused")
+  private static class ClassWithPrivateMembers {
+    private String s;
+
+    private ClassWithPrivateMembers() {
+    }
+  }
+
+  private static Class<?> loadClassWithDifferentClassLoader(Class<?> c) throws Exception {
+    URL url = c.getProtectionDomain().getCodeSource().getLocation();
+    URLClassLoader classLoader = new URLClassLoader(new URL[] { url }, null);
+    return classLoader.loadClass(c.getName());
+  }
+
+  @Test
+  public void testRestrictiveSecurityManager() throws Exception {
+    // Must use separate class loader, otherwise permission is not checked, see Class.getDeclaredFields()
+    Class<?> clazz = loadClassWithDifferentClassLoader(ClassWithPrivateMembers.class);
+
+    final Permission accessDeclaredMembers = new RuntimePermission("accessDeclaredMembers");
+    final Permission suppressAccessChecks = new ReflectPermission("suppressAccessChecks");
+    SecurityManager original = System.getSecurityManager();
+    SecurityManager restrictiveManager = new SecurityManager() {
+      @Override
+      public void checkPermission(Permission perm) {
+        if (accessDeclaredMembers.equals(perm)) {
+          throw new SecurityException("Gson: no-member-access");
+        }
+        if (suppressAccessChecks.equals(perm)) {
+          throw new SecurityException("Gson: no-suppress-access-check");
+        }
+      }
+    };
+    System.setSecurityManager(restrictiveManager);
+
+    try {
+      Gson gson = new Gson();
+      try {
+        // Getting reflection based adapter should fail
+        gson.getAdapter(clazz);
+        fail();
+      } catch (SecurityException e) {
+        assertEquals("Gson: no-member-access", e.getMessage());
+      }
+
+      final AtomicBoolean wasReadCalled = new AtomicBoolean(false);
+      gson = new GsonBuilder()
+        .registerTypeAdapter(clazz, new TypeAdapter<Object>() {
+          @Override
+          public void write(JsonWriter out, Object value) throws IOException {
+            out.value("custom-write");
+          }
+
+          @Override
+          public Object read(JsonReader in) throws IOException {
+            in.skipValue();
+            wasReadCalled.set(true);
+            return null;
+          }}
+        )
+        .create();
+
+      assertEquals("\"custom-write\"", gson.toJson(null, clazz));
+      assertNull(gson.fromJson("{}", clazz));
+      assertTrue(wasReadCalled.get());
+    } finally {
+      System.setSecurityManager(original);
+    }
+  }
+
+  /**
+   * Test serializing an instance of a non-accessible internal class, but where
+   * Gson supports serializing one of its superinterfaces.
+   *
+   * <p>Here {@link Collections#emptyList()} is used which returns an instance
+   * of the internal class {@code java.util.Collections.EmptyList}. Gson should
+   * serialize the object as {@code List} despite the internal class not being
+   * accessible.
+   *
+   * <p>See https://github.com/google/gson/issues/1875
+   */
+  @Test
+  public void testSerializeInternalImplementationObject() {
+    Gson gson = new Gson();
+    String json = gson.toJson(Collections.emptyList());
+    assertEquals("[]", json);
+
+    // But deserialization should fail
+    Class<?> internalClass = Collections.emptyList().getClass();
+    try {
+      gson.fromJson("{}", internalClass);
+      fail("Missing 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; "
+          + "either change its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type"
+      ));
+    }
+  }
+}

gson/src/test/java/com/google/gson/functional/ThrowableFunctionalTest.java

@@ -1,65 +0,0 @@
-// Copyright (C) 2014 Trymph Inc.
-package com.google.gson.functional;
-
-import java.io.IOException;
-
-import junit.framework.TestCase;
-
-import com.google.gson.Gson;
-import com.google.gson.annotations.SerializedName;
-
-@SuppressWarnings("serial")
-public final class ThrowableFunctionalTest extends TestCase {
-  private final Gson gson = new Gson();
-
-  public void testExceptionWithoutCause() {
-    RuntimeException e = new RuntimeException("hello");
-    String json = gson.toJson(e);
-    assertTrue(json.contains("hello"));
-
-    e = gson.fromJson("{'detailMessage':'hello'}", RuntimeException.class);
-    assertEquals("hello", e.getMessage());
-  }
-
-  public void testExceptionWithCause() {
-    Exception e = new Exception("top level", new IOException("io error"));
-    String json = gson.toJson(e);
-    assertTrue(json.contains("{\"detailMessage\":\"top level\",\"cause\":{\"detailMessage\":\"io error\""));
-
-    e = gson.fromJson("{'detailMessage':'top level','cause':{'detailMessage':'io error'}}", Exception.class);
-    assertEquals("top level", e.getMessage());
-    assertTrue(e.getCause() instanceof Throwable); // cause is not parameterized so type info is lost
-    assertEquals("io error", e.getCause().getMessage());
-  }
-
-  public void testSerializedNameOnExceptionFields() {
-    MyException e = new MyException();
-    String json = gson.toJson(e);
-    assertTrue(json.contains("{\"my_custom_name\":\"myCustomMessageValue\""));
-  }
-
-  public void testErrorWithoutCause() {
-    OutOfMemoryError e = new OutOfMemoryError("hello");
-    String json = gson.toJson(e);
-    assertTrue(json.contains("hello"));
-
-    e = gson.fromJson("{'detailMessage':'hello'}", OutOfMemoryError.class);
-    assertEquals("hello", e.getMessage());
-  }
-
-  public void testErrornWithCause() {
-    Error e = new Error("top level", new IOException("io error"));
-    String json = gson.toJson(e);
-    assertTrue(json.contains("top level"));
-    assertTrue(json.contains("io error"));
-
-    e = gson.fromJson("{'detailMessage':'top level','cause':{'detailMessage':'io error'}}", Error.class);
-    assertEquals("top level", e.getMessage());
-    assertTrue(e.getCause() instanceof Throwable); // cause is not parameterized so type info is lost
-    assertEquals("io error", e.getCause().getMessage());
-  }
-
-  private static final class MyException extends Throwable {
-    @SerializedName("my_custom_name") String myCustomMessage = "myCustomMessageValue";
-  }
-}

gson/src/test/java/com/google/gson/internal/bind/RecursiveTypesResolveTest.java

@@ -53,21 +53,6 @@ public class RecursiveTypesResolveTest extends TestCase {
     assertNotNull(adapter);
   }
 
-  /**
-   * Real-world samples, found in Issues #603 and #440.
-   */
-
-  public void testIssue603PrintStream() {
-    TypeAdapter<PrintStream> adapter = new Gson().getAdapter(PrintStream.class);
-    assertNotNull(adapter);
-  }
-
-  public void testIssue440WeakReference() throws Exception {
-    @SuppressWarnings("rawtypes")
-    TypeAdapter<WeakReference> adapter = new Gson().getAdapter(WeakReference.class);
-    assertNotNull(adapter);
-  }
-
   /**
    * Tests belows check the behaviour of the methods changed for the fix.
    */

gson/src/test/java/com/google/gson/internal/reflect/UnsafeReflectionAccessorTest.java

@@ -1,77 +0,0 @@
-/*
- * Copyright (C) 2018 The Gson authors
- *
- * 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.internal.reflect;
-
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.lang.reflect.Field;
-import java.security.Permission;
-
-import org.junit.Test;
-
-/**
- * Unit tests for {@link UnsafeReflectionAccessor}
- *
- * @author Inderjeet Singh
- */
-public class UnsafeReflectionAccessorTest {
-
-  @Test
-  public void testMakeAccessibleWithUnsafe() throws Exception {
-    UnsafeReflectionAccessor accessor = new UnsafeReflectionAccessor();
-    Field field = ClassWithPrivateFinalFields.class.getDeclaredField("a");
-    try {
-      boolean success = accessor.makeAccessibleWithUnsafe(field);
-      assertTrue(success);
-    } catch (Exception e) {
-      fail("Unsafe didn't work on the JDK");
-    }
-  }
-
-  @Test
-  public void testMakeAccessibleWithRestrictiveSecurityManager() throws Exception {
-    final Permission accessDeclaredMembers = new RuntimePermission("accessDeclaredMembers");
-    final SecurityManager original = System.getSecurityManager();
-    SecurityManager restrictiveManager = new SecurityManager() {
-      @Override
-      public void checkPermission(Permission perm) {
-        if (accessDeclaredMembers.equals(perm)) {
-          throw new SecurityException("nope");
-        }
-      }
-    };
-    System.setSecurityManager(restrictiveManager);
-
-    try {
-      UnsafeReflectionAccessor accessor = new UnsafeReflectionAccessor();
-      Field field = ClassWithPrivateFinalFields.class.getDeclaredField("a");
-      assertFalse("override field should have been inaccessible", accessor.makeAccessibleWithUnsafe(field));
-      accessor.makeAccessible(field);
-    } finally {
-      System.setSecurityManager(original);
-    }
-  }
-
-  @SuppressWarnings("unused")
-  private static final class ClassWithPrivateFinalFields {
-    private final String a;
-    public ClassWithPrivateFinalFields(String a) {
-      this.a = a;
-    }
-  }
-}