Add checks for types in Parcel / avoid class initialization
Make Parcel more stringent to avoid initializing classes
that are not related to Parcelable.
Two new checks:
1) That the class found on the stream implements Parcelable.
2) That the type of the CREATOR field declared on the class
found on the stream actually implements Parcelable.Creator.
For (1) the new check that a class in the stream is actually
Parcelable. This will affect handling of invalid streams or
code that didn't obey the requirements.
For (2) this change could break some apps that had a CREATOR
field in a Parcelable class that was not declared to be
(at least) a Parcelable.Creator: it is no longer sufficient
for the type to implement Parcelable.Creator, the field
must be declared as such.
This change includes doc updates for Parcelable to make
the requirement around the declared type of the CREATOR
field more concrete.
This change also makes the generics slightly tidier/explicit,
annotates code as unchecked where needed and removes some
assumptions that can not be guaranteed with Java's type
system and the current definitions.
For example, there's no guarantee right now that
Parcelable.Creator returns objects that are actually
Parcelable, or that the CREATOR object associated
with a Parcelable will return objects of the surrounding
class. The first we can't do something about without
breaking the public API (due to implementations like
TextUtils.CHAR_SEQUENCE_CREATOR). The second is
currently typically implicitly enforced with an implicit
cast in the (app's) calling code (e.g. callers to
readParcelable() that causes a language-introduced cast
to the type expected). A larger refactoring of Parcel
would be required to ensure that the class that is
produced by Creator is of a type compatible with the
class that declared CREATOR, and is not a goal for this
change.
A fix is included for a class that doesn't implement
Parcelable like it should and would probably fail
check (1).
Bug: 1171613
Change-Id: I31d07516efee29a320e80f4bc4f96aaac628f81c
diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index 9d8a1ba..ff91cc8 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -34,6 +34,7 @@
import java.io.ObjectStreamClass;
import java.io.Serializable;
import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -1373,8 +1374,7 @@
writeString(null);
return;
}
- String name = p.getClass().getName();
- writeString(name);
+ writeParcelableCreator(p);
p.writeToParcel(this, parcelableFlags);
}
@@ -2275,78 +2275,94 @@
* @throws BadParcelableException Throws BadParcelableException if there
* was an error trying to instantiate the Parcelable.
*/
+ @SuppressWarnings("unchecked")
public final <T extends Parcelable> T readParcelable(ClassLoader loader) {
- Parcelable.Creator<T> creator = readParcelableCreator(loader);
+ Parcelable.Creator<?> creator = readParcelableCreator(loader);
if (creator == null) {
return null;
}
if (creator instanceof Parcelable.ClassLoaderCreator<?>) {
- return ((Parcelable.ClassLoaderCreator<T>)creator).createFromParcel(this, loader);
+ Parcelable.ClassLoaderCreator<?> classLoaderCreator =
+ (Parcelable.ClassLoaderCreator<?>) creator;
+ return (T) classLoaderCreator.createFromParcel(this, loader);
}
- return creator.createFromParcel(this);
+ return (T) creator.createFromParcel(this);
}
/** @hide */
- public final <T extends Parcelable> T readCreator(Parcelable.Creator<T> creator,
+ @SuppressWarnings("unchecked")
+ public final <T extends Parcelable> T readCreator(Parcelable.Creator<?> creator,
ClassLoader loader) {
if (creator instanceof Parcelable.ClassLoaderCreator<?>) {
- return ((Parcelable.ClassLoaderCreator<T>)creator).createFromParcel(this, loader);
+ Parcelable.ClassLoaderCreator<?> classLoaderCreator =
+ (Parcelable.ClassLoaderCreator<?>) creator;
+ return (T) classLoaderCreator.createFromParcel(this, loader);
}
- return creator.createFromParcel(this);
+ return (T) creator.createFromParcel(this);
}
/** @hide */
- public final <T extends Parcelable> Parcelable.Creator<T> readParcelableCreator(
- ClassLoader loader) {
+ public final Parcelable.Creator<?> readParcelableCreator(ClassLoader loader) {
String name = readString();
if (name == null) {
return null;
}
- Parcelable.Creator<T> creator;
+ Parcelable.Creator<?> creator;
synchronized (mCreators) {
- HashMap<String,Parcelable.Creator> map = mCreators.get(loader);
+ HashMap<String,Parcelable.Creator<?>> map = mCreators.get(loader);
if (map == null) {
- map = new HashMap<String,Parcelable.Creator>();
+ map = new HashMap<>();
mCreators.put(loader, map);
}
creator = map.get(name);
if (creator == null) {
try {
- Class c = loader == null ?
- Class.forName(name) : Class.forName(name, true, loader);
- Field f = c.getField("CREATOR");
- creator = (Parcelable.Creator)f.get(null);
+ // If loader == null, explicitly emulate Class.forName(String) "caller
+ // classloader" behavior.
+ ClassLoader parcelableClassLoader =
+ (loader == null ? getClass().getClassLoader() : loader);
+ // Avoid initializing the Parcelable class until we know it implements
+ // Parcelable and has the necessary CREATOR field. http://b/1171613.
+ Class<?> parcelableClass = Class.forName(name, false /* initialize */,
+ parcelableClassLoader);
+ if (!Parcelable.class.isAssignableFrom(parcelableClass)) {
+ throw new BadParcelableException("Parcelable protocol requires that the "
+ + "class implements Parcelable");
+ }
+ Field f = parcelableClass.getField("CREATOR");
+ if ((f.getModifiers() & Modifier.STATIC) == 0) {
+ throw new BadParcelableException("Parcelable protocol requires "
+ + "the CREATOR object to be static on class " + name);
+ }
+ Class<?> creatorType = f.getType();
+ if (!Parcelable.Creator.class.isAssignableFrom(creatorType)) {
+ // Fail before calling Field.get(), not after, to avoid initializing
+ // parcelableClass unnecessarily.
+ throw new BadParcelableException("Parcelable protocol requires a "
+ + "Parcelable.Creator object called "
+ + "CREATOR on class " + name);
+ }
+ creator = (Parcelable.Creator<?>) f.get(null);
}
catch (IllegalAccessException e) {
- Log.e(TAG, "Illegal access when unmarshalling: "
- + name, e);
+ Log.e(TAG, "Illegal access when unmarshalling: " + name, e);
throw new BadParcelableException(
"IllegalAccessException when unmarshalling: " + name);
}
catch (ClassNotFoundException e) {
- Log.e(TAG, "Class not found when unmarshalling: "
- + name, e);
+ Log.e(TAG, "Class not found when unmarshalling: " + name, e);
throw new BadParcelableException(
"ClassNotFoundException when unmarshalling: " + name);
}
- catch (ClassCastException e) {
- throw new BadParcelableException("Parcelable protocol requires a "
- + "Parcelable.Creator object called "
- + " CREATOR on class " + name);
- }
catch (NoSuchFieldException e) {
throw new BadParcelableException("Parcelable protocol requires a "
- + "Parcelable.Creator object called "
- + " CREATOR on class " + name);
- }
- catch (NullPointerException e) {
- throw new BadParcelableException("Parcelable protocol requires "
- + "the CREATOR object to be static on class " + name);
+ + "Parcelable.Creator object called "
+ + "CREATOR on class " + name);
}
if (creator == null) {
throw new BadParcelableException("Parcelable protocol requires a "
- + "Parcelable.Creator object called "
- + " CREATOR on class " + name);
+ + "non-null Parcelable.Creator object called "
+ + "CREATOR on class " + name);
}
map.put(name, creator);
@@ -2369,7 +2385,7 @@
}
Parcelable[] p = new Parcelable[N];
for (int i = 0; i < N; i++) {
- p[i] = (Parcelable) readParcelable(loader);
+ p[i] = readParcelable(loader);
}
return p;
}
@@ -2424,8 +2440,8 @@
// Cache of previously looked up CREATOR.createFromParcel() methods for
// particular classes. Keys are the names of the classes, values are
// Method objects.
- private static final HashMap<ClassLoader,HashMap<String,Parcelable.Creator>>
- mCreators = new HashMap<ClassLoader,HashMap<String,Parcelable.Creator>>();
+ private static final HashMap<ClassLoader,HashMap<String,Parcelable.Creator<?>>>
+ mCreators = new HashMap<>();
/** @hide for internal use only. */
static protected final Parcel obtain(int obj) {
diff --git a/core/java/android/os/Parcelable.java b/core/java/android/os/Parcelable.java
index 594fbb2..448b591 100644
--- a/core/java/android/os/Parcelable.java
+++ b/core/java/android/os/Parcelable.java
@@ -19,9 +19,8 @@
/**
* Interface for classes whose instances can be written to
* and restored from a {@link Parcel}. Classes implementing the Parcelable
- * interface must also have a static field called <code>CREATOR</code>, which
- * is an object implementing the {@link Parcelable.Creator Parcelable.Creator}
- * interface.
+ * interface must also have a non-null static field called <code>CREATOR</code>
+ * of a type that implements the {@link Parcelable.Creator} interface.
*
* <p>A typical implementation of Parcelable is:</p>
*