Commit fd17eabf authored by Przemyslaw Szczepaniak's avatar Przemyslaw Szczepaniak
Browse files

Fix clinit-related serialization incompatibility.

For the code:
class Parent {
  some code that will create <clinit>
}
class Child extends Parent {}

java.io.ObjectStreamClass.hasStaticInitializer(Child.class)
was returning false. This is technically valid, but historically
android was returning true in this case. This causes
problems with deserializing classes like Child without
explicit serialVersionUID. To mitigate the problem, this
change reverts to old behavior if sdk target <= 23.

Bug: 29064453
Change-Id: Iaff269ced01543469a30b50673a04d3d8722d261
parent a4fcb95b
......@@ -236,4 +236,62 @@ public class ObjectStreamClassTest extends TestCase {
assertNotNull(obj);
assertTrue(obj instanceof String);
}
// Class without <clinit> method
public static class NoClinitParent {
}
// Class without <clinit> method
public static class NoClinitChildWithNoClinitParent extends NoClinitParent {
}
// Class with <clinit> method
public static class ClinitParent {
// This field will trigger creation of <clinit> method for this class
private static final String TAG = ClinitParent.class.getName();
static {
}
}
// Class without <clinit> but with parent that has <clinit> method
public static class NoClinitChildWithClinitParent extends ClinitParent {
}
// http://b/29064453
public void testHasClinit() throws Exception {
Method hasStaticInitializer =
ObjectStreamClass.class.getDeclaredMethod("hasStaticInitializer", Class.class,
boolean.class);
hasStaticInitializer.setAccessible(true);
assertTrue((Boolean)
hasStaticInitializer.invoke(null, ClinitParent.class,
false /* checkSuperclass */));
// RI will return correctly False in this case, but android has been returning true
// in this particular case. We're returning true to enable deserializing classes
// like NoClinitChildWithClinitParent without explicit serialVersionID field.
assertTrue((Boolean)
hasStaticInitializer.invoke(null, NoClinitChildWithClinitParent.class,
false /* checkSuperclass */));
assertFalse((Boolean)
hasStaticInitializer.invoke(null, NoClinitParent.class,
false /* checkSuperclass */));
assertFalse((Boolean)
hasStaticInitializer.invoke(null, NoClinitChildWithNoClinitParent.class,
false /* checkSuperclass */));
assertTrue((Boolean)
hasStaticInitializer.invoke(null, ClinitParent.class,
true /* checkSuperclass */));
assertFalse((Boolean)
hasStaticInitializer.invoke(null, NoClinitChildWithClinitParent.class,
true /* checkSuperclass */));
assertFalse((Boolean)
hasStaticInitializer.invoke(null, NoClinitParent.class,
true /* checkSuperclass */));
assertFalse((Boolean)
hasStaticInitializer.invoke(null, NoClinitChildWithNoClinitParent.class,
true /* checkSuperclass */));
}
}
......@@ -52,8 +52,8 @@ import sun.misc.Unsafe;
import sun.reflect.CallerSensitive;
import sun.reflect.Reflection;
import sun.reflect.misc.ReflectUtil;
import dalvik.system.VMRuntime;
import dalvik.system.VMStack;
/**
* Serialization's descriptor for classes. It contains the name and
* serialVersionUID of the class. The ObjectStreamClass for a specific class
......@@ -1731,7 +1731,9 @@ public class ObjectStreamClass implements Serializable {
}
}
if (hasStaticInitializer(cl)) {
boolean checkSuperclass = !(VMRuntime.getRuntime().getTargetSdkVersion()
<= MAX_SDK_TARGET_FOR_CLINIT_UIDGEN_WORKAROUND);
if (hasStaticInitializer(cl, checkSuperclass)) {
dout.writeUTF("<clinit>");
dout.writeInt(Modifier.STATIC);
dout.writeUTF("()V");
......@@ -1804,11 +1806,17 @@ public class ObjectStreamClass implements Serializable {
}
}
/** Max SDK target version for which we use buggy hasStaticIntializier implementation. */
static final int MAX_SDK_TARGET_FOR_CLINIT_UIDGEN_WORKAROUND = 23;
/**
* Returns true if the given class defines a static initializer method,
* false otherwise.
* if checkSuperclass is false, we use a buggy version (for compatibility reason) that
* will return true even if only the superclass has a static initializer method.
*/
private native static boolean hasStaticInitializer(Class<?> cl);
private native static boolean hasStaticInitializer(Class<?> cl, boolean checkSuperclass);
/**
* Class for computing and caching field/constructor/method signatures
......
......@@ -158,6 +158,12 @@ package java.io;
* the default computed value, but the requirement for matching
* serialVersionUID values is waived for array classes.
*
* Android implementation of serialVersionUID computation will change slightly
* for some classes if you're targeting android N. In order to preserve compatibility,
* this change is only enabled is the application target SDK version is set to
* 24 or higher. It is highly recommended to use an explicit serialVersionUID
* field to avoid compatibility issues.
*
* <h3>Implement Serializable Judiciously</h3>
* Refer to <i>Effective Java</i>'s chapter on serialization for thorough
* coverage of the serialization API. The book explains how to use this
......
......@@ -45,14 +45,15 @@ static void ObjectStreamClass_initNative(JNIEnv *env)
/*
* Class: java_io_ObjectStreamClass
* Method: hasStaticInitializer
* Signature: (Ljava/lang/Class;)Z
* Signature: (Ljava/lang/Class;Z)Z
*
* Returns true if the given class defines a <clinit>()V method; returns false
* otherwise.
*/
JNIEXPORT jboolean JNICALL
ObjectStreamClass_hasStaticInitializer(JNIEnv *env, jclass this,
jclass clazz)
jclass clazz,
jboolean checkSuperclass)
{
jclass superCl = NULL;
jmethodID superClinitId = NULL;
......@@ -67,6 +68,14 @@ ObjectStreamClass_hasStaticInitializer(JNIEnv *env, jclass this,
return JNI_FALSE;
}
// Android-changed, if checkSuperclass == true, remove check for
// superclass clinitId != child clinitId.
// We're returning true to enable deserializing classes without explicit serialVersionID
// that would fail in this check (b/29064453).
if (checkSuperclass == JNI_FALSE) {
return JNI_TRUE;
}
/*
* Check superclass for static initializer as well--if the same method ID
* is returned, then the static initializer is from a superclass.
......@@ -74,7 +83,6 @@ ObjectStreamClass_hasStaticInitializer(JNIEnv *env, jclass this,
* JNI spec makes no guarantee that GetStaticMethodID will not return the
* ID for a superclass initializer.
*/
if ((superCl = (*env)->GetSuperclass(env, clazz)) == NULL) {
return JNI_TRUE;
}
......@@ -93,7 +101,7 @@ ObjectStreamClass_hasStaticInitializer(JNIEnv *env, jclass this,
}
static JNINativeMethod gMethods[] = {
NATIVE_METHOD(ObjectStreamClass, hasStaticInitializer, "(Ljava/lang/Class;)Z"),
NATIVE_METHOD(ObjectStreamClass, hasStaticInitializer, "(Ljava/lang/Class;Z)Z"),
};
void register_java_io_ObjectStreamClass(JNIEnv* env) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment