Fix a major bug in Bundle when unparcelling from AIDL.
There was a serious problem in the Bundle(Parcel) and readFromParcel() methods,
where it wasn't doing the copying of the Parcel that Parcel.readBundle() does
and is a basic requirement for it to work correctly.
This re-arranges the code to make all of these functions (hopefully) correct.
Also fix a problem in Parcel where we were not duping fds when copying data from
one Parcel to another.
diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java
index b669fa2..a91655f 100644
--- a/core/java/android/os/Bundle.java
+++ b/core/java/android/os/Bundle.java
@@ -78,6 +78,10 @@
readFromParcel(parcelledData);
}
+ /* package */ Bundle(Parcel parcelledData, int length) {
+ readFromParcelInner(parcelledData, length);
+ }
+
/**
* Constructs a new, empty Bundle that uses a specific ClassLoader for
* instantiating Parcelable and Serializable objects.
@@ -155,13 +159,14 @@
return;
}
- mParcelledData.setDataPosition(0);
- Bundle b = mParcelledData.readBundleUnpacked(mClassLoader);
- mMap = b.mMap;
-
- mHasFds = mParcelledData.hasFileDescriptors();
- mFdsKnown = true;
-
+ int N = mParcelledData.readInt();
+ if (N < 0) {
+ return;
+ }
+ if (mMap == null) {
+ mMap = new HashMap<String, Object>();
+ }
+ mParcelledData.readMapInternal(mMap, N, mClassLoader);
mParcelledData.recycle();
mParcelledData = null;
}
@@ -1427,7 +1432,25 @@
* @param parcel The parcel to copy this bundle to.
*/
public void writeToParcel(Parcel parcel, int flags) {
- parcel.writeBundle(this);
+ if (mParcelledData != null) {
+ int length = mParcelledData.dataSize();
+ parcel.writeInt(length);
+ parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'
+ parcel.appendFrom(mParcelledData, 0, length);
+ } else {
+ parcel.writeInt(-1); // dummy, will hold length
+ parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'
+
+ int oldPos = parcel.dataPosition();
+ parcel.writeMapInternal(mMap);
+ int newPos = parcel.dataPosition();
+
+ // Backpatch length
+ parcel.setDataPosition(oldPos - 8);
+ int length = newPos - oldPos;
+ parcel.writeInt(length);
+ parcel.setDataPosition(newPos);
+ }
}
/**
@@ -1436,8 +1459,33 @@
* @param parcel The parcel to overwrite this bundle from.
*/
public void readFromParcel(Parcel parcel) {
- mParcelledData = parcel;
- mHasFds = mParcelledData.hasFileDescriptors();
+ int length = parcel.readInt();
+ if (length < 0) {
+ throw new RuntimeException("Bad length in parcel: " + length);
+ }
+ readFromParcelInner(parcel, length);
+ }
+
+ void readFromParcelInner(Parcel parcel, int length) {
+ int magic = parcel.readInt();
+ if (magic != 0x4C444E42) {
+ //noinspection ThrowableInstanceNeverThrown
+ String st = Log.getStackTraceString(new RuntimeException());
+ Log.e("Bundle", "readBundle: bad magic number");
+ Log.e("Bundle", "readBundle: trace = " + st);
+ }
+
+ // Advance within this Parcel
+ int offset = parcel.dataPosition();
+ parcel.setDataPosition(offset + length);
+
+ Parcel p = Parcel.obtain();
+ p.setDataPosition(0);
+ p.appendFrom(parcel, offset, length);
+ p.setDataPosition(0);
+
+ mParcelledData = p;
+ mHasFds = p.hasFileDescriptors();
mFdsKnown = true;
}
diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index 9a71f6e..6cfccee 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -457,7 +457,7 @@
* Flatten a Map into the parcel at the current dataPosition(),
* growing dataCapacity() if needed. The Map keys must be String objects.
*/
- private void writeMapInternal(Map<String,Object> val) {
+ /* package */ void writeMapInternal(Map<String,Object> val) {
if (val == null) {
writeInt(-1);
return;
@@ -480,23 +480,7 @@
return;
}
- if (val.mParcelledData != null) {
- int length = val.mParcelledData.dataSize();
- appendFrom(val.mParcelledData, 0, length);
- } else {
- writeInt(-1); // dummy, will hold length
- int oldPos = dataPosition();
- writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'
-
- writeMapInternal(val.mMap);
- int newPos = dataPosition();
-
- // Backpatch length
- setDataPosition(oldPos - 4);
- int length = newPos - oldPos;
- writeInt(length);
- setDataPosition(newPos);
- }
+ val.writeToParcel(this, 0);
}
/**
@@ -1352,27 +1336,12 @@
* Returns null if the previously written Bundle object was null.
*/
public final Bundle readBundle(ClassLoader loader) {
- int offset = dataPosition();
int length = readInt();
if (length < 0) {
return null;
}
- int magic = readInt();
- if (magic != 0x4C444E42) {
- //noinspection ThrowableInstanceNeverThrown
- String st = Log.getStackTraceString(new RuntimeException());
- Log.e("Bundle", "readBundle: bad magic number");
- Log.e("Bundle", "readBundle: trace = " + st);
- }
-
- // Advance within this Parcel
- setDataPosition(offset + length + 4);
-
- Parcel p = new Parcel(0);
- p.setDataPosition(0);
- p.appendFrom(this, offset, length + 4);
- p.setDataPosition(0);
- final Bundle bundle = new Bundle(p);
+
+ final Bundle bundle = new Bundle(this, length);
if (loader != null) {
bundle.setClassLoader(loader);
}
@@ -1380,33 +1349,6 @@
}
/**
- * Read and return a new Bundle object from the parcel at the current
- * dataPosition(). Returns null if the previously written Bundle object was
- * null. The returned bundle will have its contents fully unpacked using
- * the given ClassLoader.
- */
- /* package */ Bundle readBundleUnpacked(ClassLoader loader) {
- int length = readInt();
- if (length == -1) {
- return null;
- }
- int magic = readInt();
- if (magic != 0x4C444E42) {
- //noinspection ThrowableInstanceNeverThrown
- String st = Log.getStackTraceString(new RuntimeException());
- Log.e("Bundle", "readBundleUnpacked: bad magic number");
- Log.e("Bundle", "readBundleUnpacked: trace = " + st);
- }
- Bundle m = new Bundle(loader);
- int N = readInt();
- if (N < 0) {
- return null;
- }
- readMapInternal(m.mMap, N, loader);
- return m;
- }
-
- /**
* Read and return a byte[] object from the parcel.
*/
public final native byte[] createByteArray();
@@ -1998,7 +1940,7 @@
private native void init(int obj);
private native void destroy();
- private void readMapInternal(Map outVal, int N,
+ /* package */ void readMapInternal(Map outVal, int N,
ClassLoader loader) {
while (N > 0) {
Object key = readValue(loader);
diff --git a/libs/utils/Parcel.cpp b/libs/utils/Parcel.cpp
index e74ad4a..b0e3750 100644
--- a/libs/utils/Parcel.cpp
+++ b/libs/utils/Parcel.cpp
@@ -409,12 +409,16 @@
mObjects[idx++] = off;
mObjectsSize++;
- const flat_binder_object* flat
+ flat_binder_object* flat
= reinterpret_cast<flat_binder_object*>(mData + off);
acquire_object(proc, *flat, this);
- // take note if the object is a file descriptor
if (flat->type == BINDER_TYPE_FD) {
+ // If this is a file descriptor, we need to dup it so the
+ // new Parcel now owns its own fd, and can declare that we
+ // officially know we have fds.
+ flat->handle = dup(flat->handle);
+ flat->cookie = (void*)1;
mHasFds = mFdsKnown = true;
}
}
