am 55848681: Add additional field checks for deserialization.
* commit '55848681427721532161c63b43b3f99e912d071c':
Add additional field checks for deserialization.
diff --git a/luni/src/main/java/java/io/ObjectInputStream.java b/luni/src/main/java/java/io/ObjectInputStream.java
index cd7f736..3a89b52 100644
--- a/luni/src/main/java/java/io/ObjectInputStream.java
+++ b/luni/src/main/java/java/io/ObjectInputStream.java
@@ -23,7 +23,6 @@
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.util.ArrayList;
import java.util.Arrays;
@@ -1064,12 +1063,10 @@
}
for (ObjectStreamField fieldDesc : fields) {
- Field field = classDesc.getReflectionField(fieldDesc);
- if (field != null && Modifier.isTransient(field.getModifiers())) {
- field = null; // No setting transient fields! (http://b/4471249)
- }
- // We may not have been able to find the field, or it may be transient, but we still
- // need to read the value and do the other checking...
+ // checkAndGetReflectionField() can return null if it was not able to find the field or
+ // if it is transient or static. We still need to read the data and do the other
+ // checking...
+ Field field = classDesc.checkAndGetReflectionField(fieldDesc);
try {
Class<?> type = fieldDesc.getTypeInternal();
if (type == byte.class) {
diff --git a/luni/src/main/java/java/io/ObjectOutputStream.java b/luni/src/main/java/java/io/ObjectOutputStream.java
index bb6711b..f67919d 100644
--- a/luni/src/main/java/java/io/ObjectOutputStream.java
+++ b/luni/src/main/java/java/io/ObjectOutputStream.java
@@ -928,9 +928,11 @@
for (ObjectStreamField fieldDesc : classDesc.fields()) {
try {
Class<?> type = fieldDesc.getTypeInternal();
- Field field = classDesc.getReflectionField(fieldDesc);
+ Field field = classDesc.checkAndGetReflectionField(fieldDesc);
if (field == null) {
- throw new InvalidClassException(classDesc.getName() + " doesn't have a field " + fieldDesc.getName() + " of type " + type);
+ throw new InvalidClassException(classDesc.getName()
+ + " doesn't have a serializable field " + fieldDesc.getName()
+ + " of type " + type);
}
if (type == byte.class) {
output.writeByte(field.getByte(obj));
@@ -1728,7 +1730,7 @@
// Only write field "name" for enum class, which is the second field of
// enum, that is fields[1]. Ignore all non-fields and fields.length < 2
if (fields != null && fields.length > 1) {
- Field field = classDesc.getSuperclass().getReflectionField(fields[1]);
+ Field field = classDesc.getSuperclass().checkAndGetReflectionField(fields[1]);
if (field == null) {
throw new NoSuchFieldError();
}
diff --git a/luni/src/main/java/java/io/ObjectStreamClass.java b/luni/src/main/java/java/io/ObjectStreamClass.java
index 9b7f2c9..1a27c9d 100644
--- a/luni/src/main/java/java/io/ObjectStreamClass.java
+++ b/luni/src/main/java/java/io/ObjectStreamClass.java
@@ -185,26 +185,46 @@
return constructor;
}
- Field getReflectionField(ObjectStreamField osf) {
+ /**
+ * Returns the {@link Field} referred to by {@link ObjectStreamField} for the class described by
+ * this {@link ObjectStreamClass}. A {@code null} value is returned if the local definition of
+ * the field does not meet the criteria for a serializable / deserializable field, i.e. the
+ * field must be non-static and non-transient. Caching of each field lookup is performed. The
+ * first time a field is returned it is made accessible with a call to
+ * {@link Field#setAccessible(boolean)}.
+ */
+ Field checkAndGetReflectionField(ObjectStreamField osf) {
synchronized (reflectionFields) {
Field field = reflectionFields.get(osf);
- if (field != null) {
+ // null might indicate a cache miss or a hit and a non-serializable field so we
+ // check for a mapping.
+ if (field != null || reflectionFields.containsKey(osf)) {
return field;
}
}
+ Field field;
try {
Class<?> declaringClass = forClass();
- Field field = declaringClass.getDeclaredField(osf.getName());
- field.setAccessible(true);
- synchronized (reflectionFields) {
- reflectionFields.put(osf, field);
+ field = declaringClass.getDeclaredField(osf.getName());
+
+ int modifiers = field.getModifiers();
+ if (Modifier.isStatic(modifiers) || Modifier.isTransient(modifiers)) {
+ // No serialization or deserialization of transient or static fields!
+ // See http://b/4471249 and http://b/17202597.
+ field = null;
+ } else {
+ field.setAccessible(true);
}
- return reflectionFields.get(osf);
} catch (NoSuchFieldException ex) {
// The caller messed up. We'll return null and won't try to resolve this again.
- return null;
+ field = null;
}
+
+ synchronized (reflectionFields) {
+ reflectionFields.put(osf, field);
+ }
+ return field;
}
/*
diff --git a/luni/src/test/java/libcore/java/io/SerializationTest.java b/luni/src/test/java/libcore/java/io/SerializationTest.java
index 1002cf1..32bc402 100644
--- a/luni/src/test/java/libcore/java/io/SerializationTest.java
+++ b/luni/src/test/java/libcore/java/io/SerializationTest.java
@@ -20,6 +20,7 @@
import java.io.InvalidClassException;
import java.io.InvalidObjectException;
+import java.io.NotSerializableException;
import java.io.ObjectStreamClass;
import java.io.ObjectStreamField;
import java.io.Serializable;
@@ -52,6 +53,97 @@
private int nonTransientInt;
}
+ public void testSerializeFieldMadeStatic() throws Exception {
+ // Does ObjectStreamClass have the right idea?
+ ObjectStreamClass osc = ObjectStreamClass.lookup(FieldMadeStatic.class);
+ ObjectStreamField[] fields = osc.getFields();
+ assertEquals(0, fields.length);
+
+ // This was created by serializing a FieldMadeStatic with a non-static staticInt
+ String s = "aced0005737200316c6962636f72652e6a6176612e696f2e53657269616c697a6174696f6e54657"
+ + "374244669656c644d6164655374617469630000000000000000020001490009737461746963496e7"
+ + "47870000022b8";
+ FieldMadeStatic deserialized = (FieldMadeStatic) SerializationTester.deserializeHex(s);
+ // The field data is simply ignored if it is static.
+ assertEquals(9999, deserialized.staticInt);
+ }
+
+ static class FieldMadeStatic implements Serializable {
+ private static final long serialVersionUID = 0L;
+ // private int staticInt = 8888;
+ private static int staticInt = 9999;
+ }
+
+ // We can serialize an object that has an unserializable field providing it is null.
+ public void testDeserializeNullUnserializableField() throws Exception {
+ // This was created by creating a new SerializableContainer and not setting the
+ // unserializable field. A canned serialized form is used so we can tell if the static
+ // initializers were executed during deserialization.
+ // SerializationTester.serializeHex(new SerializableContainer());
+ String s = "aced0005737200376c6962636f72652e6a6176612e696f2e53657269616c697a6174696f6e54657"
+ + "3742453657269616c697a61626c65436f6e7461696e657200000000000000000200014c000e756e7"
+ + "3657269616c697a61626c657400334c6c6962636f72652f6a6176612f696f2f53657269616c697a6"
+ + "174696f6e546573742457617353657269616c697a61626c653b787070";
+
+ serializableContainerInitializedFlag = false;
+ wasSerializableInitializedFlag = false;
+
+ SerializableContainer sc = (SerializableContainer) SerializationTester.deserializeHex(s);
+ assertNull(sc.unserializable);
+
+ // Confirm the container was initialized, but the class for the null field was not.
+ assertTrue(serializableContainerInitializedFlag);
+ assertFalse(wasSerializableInitializedFlag);
+ }
+
+ public static boolean serializableContainerInitializedFlag = false;
+
+ static class SerializableContainer implements Serializable {
+ private static final long serialVersionUID = 0L;
+ private Object unserializable = null;
+
+ static {
+ serializableContainerInitializedFlag = true;
+ }
+ }
+
+ // We must not serialize an object that has a non-null unserializable field.
+ public void testSerializeUnserializableField() throws Exception {
+ SerializableContainer sc = new SerializableContainer();
+ sc.unserializable = new WasSerializable();
+ try {
+ SerializationTester.serializeHex(sc);
+ fail();
+ } catch (NotSerializableException expected) {
+ }
+ }
+
+ // It must not be possible to deserialize an object if a field is no longer serializable.
+ public void testDeserializeUnserializableField() throws Exception {
+ // This was generated by creating a SerializableContainer and setting the unserializable
+ // field to a WasSerializable when it was still Serializable. A canned serialized form is
+ // used so we can tell if the static initializers were executed during deserialization.
+ // SerializableContainer sc = new SerializableContainer();
+ // sc.unserializable = new WasSerializable();
+ // SerializationTester.serializeHex(sc);
+ String s = "aced0005737200376c6962636f72652e6a6176612e696f2e53657269616c697a6174696f6e54657"
+ + "3742453657269616c697a61626c65436f6e7461696e657200000000000000000200014c000e756e7"
+ + "3657269616c697a61626c657400124c6a6176612f6c616e672f4f626a6563743b7870737200316c6"
+ + "962636f72652e6a6176612e696f2e53657269616c697a6174696f6e5465737424576173536572696"
+ + "16c697a61626c65000000000000000002000149000169787000000000";
+
+ serializableContainerInitializedFlag = false;
+ wasSerializableInitializedFlag = false;
+ try {
+ SerializationTester.deserializeHex(s);
+ fail();
+ } catch (InvalidClassException expected) {
+ }
+ // Confirm neither the container nor the contained class was initialized.
+ assertFalse(serializableContainerInitializedFlag);
+ assertFalse(wasSerializableInitializedFlag);
+ }
+
public void testSerialVersionUidChange() throws Exception {
// this was created by serializing a SerialVersionUidChanged with serialVersionUID = 0L
String s = "aced0005737200396c6962636f72652e6a6176612e696f2e53657269616c697a6174696f6e54657"