Merge "Change defusing for lazy bundles" am: fdd1771a4a

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1842142

Change-Id: Ia156038712d6a25acfea8aaa6ac1d257afa95345

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
+     * Flag indicating that this Bundle is okay to "defuse", see {@link #setShouldDefuse(boolean)}
-     * for system processes to ignore any {@link BadParcelableException}
+     * for more details.
-     * encountered when unparceling it, leaving an empty bundle in its place.
      * <p>
-     * This should <em>only</em> be set when the Bundle reaches its final
+     * This should <em>only</em> be set when the Bundle reaches its final destination, otherwise a
-     * destination, otherwise a system process may clobber contents that were
+     * system process may clobber contents that were destined for an app that could have unparceled
-     * destined for an app that could have unparceled them.
+     * 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
+     * Set global variable indicating that any Bundles parsed in this process should be "defused".
-     * should be "defused." That is, any {@link BadParcelableException}
+     * That is, any {@link BadParcelableException} encountered will be suppressed and logged. Also:
-     * encountered will be suppressed and logged, leaving an empty Bundle
+     * <ul>
-     * instead of crashing.
+     *   <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<?>) {
+            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 "

core/java/android/os/Parcel.java

@@ -3817,7 +3817,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);
         }

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;