diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java index 7ce8d7267de23..8dde64d8af430 100644 --- a/core/java/android/os/BaseBundle.java +++ b/core/java/android/os/BaseBundle.java @@ -43,18 +43,22 @@ public class BaseBundle { protected static final String TAG = "Bundle"; static final boolean DEBUG = false; - // Keep them in sync with frameworks/native/libs/binder/PersistableBundle.cpp. - private static final int BUNDLE_MAGIC = 0x4C444E42; // 'B' 'N' 'D' 'L' + /** + * Keep them in sync with frameworks/native/libs/binder/PersistableBundle.cpp. + * + * @hide + */ + @VisibleForTesting + static final int BUNDLE_MAGIC = 0x4C444E42; // 'B' 'N' 'D' 'L' private static final int BUNDLE_MAGIC_NATIVE = 0x4C444E44; // 'B' 'N' 'D' 'N' /** - * Flag indicating that this Bundle is okay to "defuse." That is, it's okay - * for system processes to ignore any {@link BadParcelableException} - * encountered when unparceling it, leaving an empty bundle in its place. + * Flag indicating that this Bundle is okay to "defuse", see {@link #setShouldDefuse(boolean)} + * for more details. *

- * This should only be set when the Bundle reaches its final - * destination, otherwise a system process may clobber contents that were - * destined for an app that could have unparceled them. + * This should only be set when the Bundle reaches its final destination, otherwise a + * system process may clobber contents that were destined for an app that could have unparceled + * them. */ static final int FLAG_DEFUSABLE = 1 << 0; @@ -63,10 +67,15 @@ public class BaseBundle { private static volatile boolean sShouldDefuse = false; /** - * Set global variable indicating that any Bundles parsed in this process - * should be "defused." That is, any {@link BadParcelableException} - * encountered will be suppressed and logged, leaving an empty Bundle - * instead of crashing. + * Set global variable indicating that any Bundles parsed in this process should be "defused". + * That is, any {@link BadParcelableException} encountered will be suppressed and logged. Also: + *

* * @hide */ @@ -249,6 +258,12 @@ public class BaseBundle { } } if (itemwise) { + if (LOG_DEFUSABLE && sShouldDefuse && (mFlags & FLAG_DEFUSABLE) == 0) { + Slog.wtf(TAG, + "Attempting to unparcel all items in a Bundle while in transit; this " + + "may remove elements intended for the final desitination.", + new Throwable()); + } for (int i = 0, n = mMap.size(); i < n; i++) { // Triggers deserialization of i-th item, if needed getValueAt(i); @@ -281,7 +296,16 @@ public class BaseBundle { final Object getValueAt(int i) { Object object = mMap.valueAt(i); if (object instanceof Supplier) { - object = ((Supplier) object).get(); + try { + object = ((Supplier) object).get(); + } catch (BadParcelableException e) { + if (sShouldDefuse) { + Log.w(TAG, "Failed to parse item " + mMap.keyAt(i) + ", returning null.", e); + return null; + } else { + throw e; + } + } mMap.setValueAt(i, object); } return object; @@ -289,11 +313,6 @@ public class BaseBundle { private void initializeFromParcelLocked(@NonNull Parcel parcelledData, boolean recycleParcel, boolean parcelledByNative) { - if (LOG_DEFUSABLE && sShouldDefuse && (mFlags & FLAG_DEFUSABLE) == 0) { - Slog.wtf(TAG, "Attempting to unparcel a Bundle while in transit; this may " - + "clobber all data inside!", new Throwable()); - } - if (isEmptyParcel(parcelledData)) { if (DEBUG) { Log.d(TAG, "unparcel " diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 358aa29eaea6a..a0a0e3e82eae0 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -3776,7 +3776,7 @@ public final class Parcel { default: int off = dataPosition() - 4; - throw new RuntimeException( + throw new BadParcelableException( "Parcel " + this + ": Unmarshalling unknown type code " + type + " at offset " + off); } diff --git a/core/tests/coretests/src/android/os/BundleTest.java b/core/tests/coretests/src/android/os/BundleTest.java index 9d2cab3f7026f..ddd0070588edd 100644 --- a/core/tests/coretests/src/android/os/BundleTest.java +++ b/core/tests/coretests/src/android/os/BundleTest.java @@ -47,6 +47,7 @@ public class BundleTest { @After public void tearDown() throws Exception { + BaseBundle.setShouldDefuse(false); if (mWtfHandler != null) { Log.setWtfHandler(mWtfHandler); } @@ -355,6 +356,81 @@ public class BundleTest { assertThat(e.getCause()).isInstanceOf(Log.TerribleFailure.class); } + @Test + public void getParcelable_whenThrowingAndNotDefusing_throws() throws Exception { + Bundle.setShouldDefuse(false); + Bundle bundle = new Bundle(); + bundle.putParcelable("key", new CustomParcelable(13, "Tiramisu")); + bundle.readFromParcel(getParcelledBundle(bundle)); + + // Default class-loader is the bootpath class-loader, which doesn't contain + // CustomParcelable, so trying to read it will throw BadParcelableException. + assertThrows(BadParcelableException.class, () -> bundle.getParcelable("key")); + } + + @Test + public void getParcelable_whenThrowingAndDefusing_returnsNull() throws Exception { + Bundle.setShouldDefuse(true); + Bundle bundle = new Bundle(); + bundle.putParcelable("key", new CustomParcelable(13, "Tiramisu")); + bundle.putString("string", "value"); + bundle.readFromParcel(getParcelledBundle(bundle)); + + // Default class-loader is the bootpath class-loader, which doesn't contain + // CustomParcelable, so trying to read it will throw BadParcelableException. + assertThat(bundle.getParcelable("key")).isNull(); + // Doesn't affect other items + assertThat(bundle.getString("string")).isEqualTo("value"); + } + + @Test + public void getParcelable_whenThrowingAndDefusing_leavesElement() throws Exception { + Bundle.setShouldDefuse(true); + Bundle bundle = new Bundle(); + Parcelable parcelable = new CustomParcelable(13, "Tiramisu"); + bundle.putParcelable("key", parcelable); + bundle.putString("string", "value"); + bundle.readFromParcel(getParcelledBundle(bundle)); + assertThat(bundle.getParcelable("key")).isNull(); + + // Now, we simulate reserializing and assign the proper class loader to not throw anymore + bundle.readFromParcel(getParcelledBundle(bundle)); + bundle.setClassLoader(getClass().getClassLoader()); + + // We're able to retrieve it even though we failed before + assertThat(bundle.getParcelable("key")).isEqualTo(parcelable); + } + + @Test + public void partialDeserialization_whenNotDefusing_throws() throws Exception { + Bundle.setShouldDefuse(false); + Bundle bundle = getMalformedBundle(); + assertThrows(BadParcelableException.class, bundle::isEmpty); + } + + @Test + public void partialDeserialization_whenDefusing_emptiesMap() throws Exception { + Bundle.setShouldDefuse(true); + Bundle bundle = getMalformedBundle(); + bundle.isEmpty(); + // Nothing thrown + assertThat(bundle.size()).isEqualTo(0); + } + + private Bundle getMalformedBundle() { + Parcel p = Parcel.obtain(); + p.writeInt(BaseBundle.BUNDLE_MAGIC); + int start = p.dataPosition(); + p.writeInt(1); // Number of items + p.writeString("key"); + p.writeInt(131313); // Invalid type + p.writeInt(0); // Anything, really + int end = p.dataPosition(); + p.setDataPosition(0); + return new Bundle(p, end - start); + } + + private static class CustomParcelable implements Parcelable { public final int integer; public final String string;