diff options
4 files changed, 166 insertions, 70 deletions
diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java index cc397078ef..16a7415090 100644 --- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java +++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java @@ -284,64 +284,6 @@ public class ObjectStreamClassTest extends TestCaseWithRules { return newInstance; } - // 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 */)); - } - // http://b/29721023 public void testClassWithSameFieldName() throws Exception { // Load class from dex, it's not possible to create a class with same-named diff --git a/luni/src/test/java/libcore/java/io/ObjectStreamClassTest.java b/luni/src/test/java/libcore/java/io/ObjectStreamClassTest.java new file mode 100644 index 0000000000..612b7d8798 --- /dev/null +++ b/luni/src/test/java/libcore/java/io/ObjectStreamClassTest.java @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * 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 libcore.java.io; + +import java.io.ObjectStreamClass; +import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; +import libcore.junit.util.SwitchTargetSdkVersionRule; +import libcore.junit.util.SwitchTargetSdkVersionRule.TargetSdkVersion; +import org.junit.FixMethodOrder; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestRule; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; + +import static org.junit.Assert.assertEquals; + +@RunWith(JUnitParamsRunner.class) +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +public class ObjectStreamClassTest { + + @Rule + public TestRule switchTargetSdkVersionRule = SwitchTargetSdkVersionRule.getInstance(); + + /** + * The default SUID for this should not be affected by the b/29064453 patch. + */ + private static class BaseWithStaticInitializer implements Serializable { + static { + System.out.println( + "Static initializer for " + BaseWithoutStaticInitializer.class.getCanonicalName()); + } + } + + /** + * The default SUID for this should not be affected by the b/29064453 patch. + */ + private static class BaseWithoutStaticInitializer implements Serializable { + } + + /** + * The default SUID for this should not be affected by the b/29064453 patch. + */ + private static class WithStaticInitializer extends BaseWithoutStaticInitializer { + static { + System.out.println( + "Static initializer for " + WithStaticInitializer.class.getCanonicalName()); + } + } + + /** + * The default SUID for this should not be affected by the b/29064453 patch. + */ + private static class WithoutStaticInitializer extends BaseWithoutStaticInitializer { + } + + /** + * The default SUID for this should be affected by the b/29064453 patch and so should differ + * between version <= 23 and version > 23. + */ + private static class InheritStaticInitializer extends BaseWithStaticInitializer { + } + + public static Object[][] defaultSUIDs() { + return new Object[][] { + // The default SUID for BaseWithStaticInitializer should not be affected by the b/29064453 + // patch. + { BaseWithStaticInitializer.class, -4971959491244110788L, -4971959491244110788L }, + + // The default SUID for BaseWithoutStaticInitializer should not be affected by the + // b/29064453 patch. + { BaseWithoutStaticInitializer.class, -245652310465925293L, -245652310465925293L }, + + // The default SUID for WithStaticInitializer should not be affected by the b/29064453 + // patch. + { WithStaticInitializer.class, -3581047773254885060L, -3581047773254885060L }, + + // The default SUID for WithStaticInitializer should not be affected by the + // b/29064453 patch. + { WithoutStaticInitializer.class, -975889567651927545L, -975889567651927545L }, + + // The default SUID for the InheritStaticInitializer should be affected by the b/29064453 + // patch and so should differ between version <= 23 and version > 23. + { InheritStaticInitializer.class, 4188245044387716731L, 992629205079295334L }, + + + }; + } + + @Parameters(method = "defaultSUIDs") + @Test + public void computeDefaultSUID_current(Class<?> clazz, long suid, + @SuppressWarnings("unused") long suid23) { + checkSerialVersionUID(suid, clazz); + } + + @Parameters(method = "defaultSUIDs") + @Test + @TargetSdkVersion(23) + public void computeDefaultSUID_targetSdkVersion_23(Class<?> clazz, + @SuppressWarnings("unused") long suid, long suid23) { + checkSerialVersionUID(suid23, clazz); + } + + private static void checkSerialVersionUID(long expectedSUID, Class<?> clazz) { + // Use reflection to access the private static computeDefaultSUID method. + long defaultSUID; + try { + Method computeDefaultSUIDMethod = + ObjectStreamClass.class.getDeclaredMethod("computeDefaultSUID", Class.class); + computeDefaultSUIDMethod.setAccessible(true); + defaultSUID = (Long) computeDefaultSUIDMethod.invoke(null, clazz); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException(e); + } + assertEquals(expectedSUID, defaultSUID); + } +} diff --git a/ojluni/src/main/java/java/io/ObjectStreamClass.java b/ojluni/src/main/java/java/io/ObjectStreamClass.java index 4a183f807a..7561aa89da 100644 --- a/ojluni/src/main/java/java/io/ObjectStreamClass.java +++ b/ojluni/src/main/java/java/io/ObjectStreamClass.java @@ -1798,9 +1798,14 @@ public class ObjectStreamClass implements Serializable { } // Android-changed: Clinit serialization workaround b/29064453 - boolean checkSuperclass = !(VMRuntime.getRuntime().getTargetSdkVersion() - <= MAX_SDK_TARGET_FOR_CLINIT_UIDGEN_WORKAROUND); - if (hasStaticInitializer(cl, checkSuperclass)) { + // Prior to SDK 24 hasStaticInitializer() would return true if the superclass had a + // static initializer, that was contrary to the specification. In SDK 24 the default + // behavior was corrected but the old behavior was preserved for apps that targeted 23 + // or below in order to maintain backwards compatibility. + boolean inheritStaticInitializer = + (VMRuntime.getRuntime().getTargetSdkVersion() + <= MAX_SDK_TARGET_FOR_CLINIT_UIDGEN_WORKAROUND); + if (hasStaticInitializer(cl, inheritStaticInitializer)) { dout.writeUTF("<clinit>"); dout.writeInt(Modifier.STATIC); dout.writeUTF("()V"); @@ -1880,10 +1885,14 @@ public class ObjectStreamClass implements Serializable { /** * 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. + * + * @param inheritStaticInitializer if false then this method will return true iff the given + * class has its own static initializer, if true (used for backwards compatibility for apps + * that target SDK version <= {@link #MAX_SDK_TARGET_FOR_CLINIT_UIDGEN_WORKAROUND}) it will + * return true if the given class or any of its ancestor classes have a static initializer. */ - private native static boolean hasStaticInitializer(Class<?> cl, boolean checkSuperclass); + private native static boolean hasStaticInitializer( + Class<?> cl, boolean inheritStaticInitializer); // END Android-changed: Clinit serialization workaround b/29064453 /** diff --git a/ojluni/src/main/native/ObjectStreamClass.c b/ojluni/src/main/native/ObjectStreamClass.c index 95cd4f0247..3d3fdefa33 100644 --- a/ojluni/src/main/native/ObjectStreamClass.c +++ b/ojluni/src/main/native/ObjectStreamClass.c @@ -51,12 +51,19 @@ static void ObjectStreamClass_initNative(JNIEnv *env) * otherwise. */ JNIEXPORT jboolean JNICALL +// Android-changed: Added inheritStaticInitializer parameter. +// The inheritStaticInitializer parameter is set to JNI_TRUE when behavior compatible with +// Android version 23 is required. ObjectStreamClass_hasStaticInitializer(JNIEnv *env, jclass this, jclass clazz, - jboolean checkSuperclass) + jboolean inheritStaticInitializer) { jclass superCl = NULL; jmethodID superClinitId = NULL; + + // Android-changed: Added comment to explain behavior. + // Search for a static initializer method in this class and up through its + // ancestor super classes, returning the id of the first method found. jmethodID clinitId = (*env)->GetStaticMethodID(env, clazz, "<clinit>", "()V"); if (clinitId == NULL) { /* error thrown */ @@ -68,13 +75,15 @@ 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) { + // BEGIN Android-changed: Exit immediately if version 23 behavior is required. + // At this point the class or one of its ancestor super classes has a + // static initializer. If inheritStaticInitializer is true then this method + // can simply return true. Otherwise, it needs to check that the static + // initializer was not inherited (b/29064453). + if (inheritStaticInitializer == JNI_TRUE) { return JNI_TRUE; } + // END Android-changed: Exit immediately if version 23 behavior is required. /* * Check superclass for static initializer as well--if the same method ID @@ -83,6 +92,7 @@ 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; } |