Change defusing for lazy bundles

With lazy bundle, exceptions thrown by deserialization of custom items
moved from being thrown in initializeFromParcelLocked() (when the bundle
is first touched) to being thrown in getValueAt() (whenever the item is
retrieved), which means they were escaping the defuse logic that caught
those exceptions. So, now also catching these exceptions in
getValueAt(), however, here we can be much less drastic, instead of
erasing the bundle, we can simply remove the bad element.

This also means we don't need to log wtf in initializeFromParcelLocked()
if sShouldDefuse = true but the bundle is not marked as defusable, since
touching the bundle doesn't carry the same consequences as before
go/lazy-bundle. So, I moved that log to unparcel(itemwise = true), ie.
whenever the entire bundle is deserialized on purpose.

Now, 2 types of defusing can happen:
1. If the (custom) item we're retrieving caused the exception, we'll
   remove the item and return null.
2. If the exception was raised during partial deserialization, that is,
   during the read of the map and its basic types (while skipping custom
   types), the map will be left empty. I believe only manually written
   parcels would cause this type of exception and if an exception is
   raised here it will also be raised in the final destination too,
   afaict. Because of this, we can now touch app-provided bundles
   without fear of clobbering the data on its way to the final
   destination.

Following conversation on previous CL, narrowed exception raised in case
of unknown type to be BadParcelableException.

Test: atest -d android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest
Bug: 195622897
Change-Id: I4746140f63482a9ea475aac25897a447737393e4

3 files changed, 114 insertions, 19 deletions

diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java
index 7ce8d7267de2..8dde64d8af43 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.
      * <p>
-     * This should <em>only</em> 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 <em>only</em> 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:
+     * <ul>
+     *   <li>If it was the deserialization of a custom item (eg. {@link Parcelable}) that caused the
+     *   exception, {@code null} will be returned but the item will be held in the map in its
+     *   serialized form (lazy value).
+     *   <li>If the exception happened during partial deserialization, that is, during the read of
+     *   the map and its basic types (while skipping custom types), the map will be left empty.
+     * </ul>
      *
      * @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 243dfb3e04bc..fc2f444c4e72 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -3778,7 +3778,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 9d2cab3f7026..ddd0070588ed 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.<Parcelable>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.<Parcelable>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.<Parcelable>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;