From 3c3e0c659a8df3ce75d630e1a4c9adbc3bcf5d70 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Sat, 11 Apr 2020 01:27:24 +0800 Subject: [PATCH] Do not unparcel bundle from application in LaunchActivityItem The bundle fields may contain custom Parcelables. And Bundle#size will call unparcel that causes BadParcelableException from LaunchActivityItem#hashCode and LaunchActivityItem#equals. Since the bundle fields of LaunchActivityItem may not be significant for being the accurate identity of the item, the bundle fields can be treated roughly (empty or not) to avoid unparceling. Fixes: 153737846 Test: atest FrameworksCoreTests:TransactionParcelTests#testLaunch Change-Id: I7ec55bbfcffcd47cfb586ede8053ab411891902d --- .../servertransaction/LaunchActivityItem.java | 38 +++++++--------- .../TransactionParcelTests.java | 43 ++++++++++++++++++- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/core/java/android/app/servertransaction/LaunchActivityItem.java b/core/java/android/app/servertransaction/LaunchActivityItem.java index 6d674ae7df143..9ab6e7fc9f58f 100644 --- a/core/java/android/app/servertransaction/LaunchActivityItem.java +++ b/core/java/android/app/servertransaction/LaunchActivityItem.java @@ -186,8 +186,8 @@ public class LaunchActivityItem extends ClientTransactionItem { && Objects.equals(mOverrideConfig, other.mOverrideConfig) && Objects.equals(mCompatInfo, other.mCompatInfo) && Objects.equals(mReferrer, other.mReferrer) - && mProcState == other.mProcState && areBundlesEqual(mState, other.mState) - && areBundlesEqual(mPersistentState, other.mPersistentState) + && mProcState == other.mProcState && areBundlesEqualRoughly(mState, other.mState) + && areBundlesEqualRoughly(mPersistentState, other.mPersistentState) && Objects.equals(mPendingResults, other.mPendingResults) && Objects.equals(mPendingNewIntents, other.mPendingNewIntents) && mIsForward == other.mIsForward @@ -205,8 +205,8 @@ public class LaunchActivityItem extends ClientTransactionItem { result = 31 * result + Objects.hashCode(mCompatInfo); result = 31 * result + Objects.hashCode(mReferrer); result = 31 * result + Objects.hashCode(mProcState); - result = 31 * result + (mState != null ? mState.size() : 0); - result = 31 * result + (mPersistentState != null ? mPersistentState.size() : 0); + result = 31 * result + getRoughBundleHashCode(mState); + result = 31 * result + getRoughBundleHashCode(mPersistentState); result = 31 * result + Objects.hashCode(mPendingResults); result = 31 * result + Objects.hashCode(mPendingNewIntents); result = 31 * result + (mIsForward ? 1 : 0); @@ -225,25 +225,19 @@ public class LaunchActivityItem extends ClientTransactionItem { && Objects.equals(mInfo.getComponentName(), other.getComponentName()); } - private static boolean areBundlesEqual(BaseBundle extras, BaseBundle newExtras) { - if (extras == null || newExtras == null) { - return extras == newExtras; - } + /** + * This method may be used to compare a parceled item with another unparceled item, and the + * parceled bundle may contain customized class that will raise BadParcelableException when + * unparceling if a customized class loader is not set to the bundle. So the hash code is + * simply determined by the bundle is empty or not. + */ + private static int getRoughBundleHashCode(BaseBundle bundle) { + return (bundle == null || bundle.isDefinitelyEmpty()) ? 0 : 1; + } - if (extras.size() != newExtras.size()) { - return false; - } - - for (String key : extras.keySet()) { - if (key != null) { - final Object value = extras.get(key); - final Object newValue = newExtras.get(key); - if (!Objects.equals(value, newValue)) { - return false; - } - } - } - return true; + /** Compares the bundles without unparceling them (avoid BadParcelableException). */ + private static boolean areBundlesEqualRoughly(BaseBundle a, BaseBundle b) { + return getRoughBundleHashCode(a) == getRoughBundleHashCode(b); } @Override diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index f4fbefe9dde46..3766cf72d99ee 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -63,7 +63,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.util.ArrayList; -import java.util.List; import java.util.Map; /** @@ -213,6 +212,7 @@ public class TransactionParcelTests { int procState = 4; Bundle bundle = new Bundle(); bundle.putString("key", "value"); + bundle.putParcelable("data", new ParcelableData(1)); PersistableBundle persistableBundle = new PersistableBundle(); persistableBundle.putInt("k", 4); @@ -374,6 +374,47 @@ public class TransactionParcelTests { mParcel.setDataPosition(0); } + /** + * The parcelable class to make sure that when comparing the {@link LaunchActivityItem} or + * getting its hash code, the bundle is not unparceled. System shouldn't touch the data from + * application, otherwise it will cause exception as: + * android.os.BadParcelableException: ClassNotFoundException when unmarshalling: + * android.app.servertransaction.TransactionParcelTests$ParcelableData". + */ + public static class ParcelableData implements Parcelable { + int mValue; + + ParcelableData() {} + + ParcelableData(int value) { + mValue = value; + } + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeInt(mValue); + } + + public static final Creator CREATOR = new Creator() { + @Override + public ParcelableData createFromParcel(Parcel source) { + final ParcelableData data = new ParcelableData(); + data.mValue = source.readInt(); + return data; + } + + @Override + public ParcelableData[] newArray(int size) { + return new ParcelableData[size]; + } + }; + } + /** Stub implementation of IApplicationThread that can be presented as {@link Binder}. */ class StubAppThread extends android.app.IApplicationThread.Stub {