Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9ca6a5e2 authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

Lazy bundle

Implement lazy deserialization for custom types in bundle:
* Parcelable (VAL_PARCELABLE)
* Serializable (VAL_SERIALIZABLE)
* Parcelable array (VAL_PARCELABLEARRAY)
* Lists (VAL_LIST)
* Sparse array (VAL_SPARSEARRAY)
* Bundle (VAL_BUNDLE)

This enhances security, makes bundles more robust to deserialization
errors and avoid deserializing unneeded objects in some cases*, for more
details check go/lazy-bundle.

To do that, we prefix those types with their length when writing them on
the wire. Map serialization and deserialization now happens inside
Bundle (instead of calling Parcel's readArrayMapInternal() /
writeArrayMapInternal()) and we use an intermediary object - LazyValue -
that holds information about the position and length of the value we
will deserialize when queried.

So, there are basically 3 states:

1. We received the bundle but haven't queried anything about it (not
   even isEmpty()): in this case the original parcel is held inside and
   we haven't attempted any deserialization (except for the metadata at
   the beginning such as the magic, etc)

2. We queried something on it (eg. isEmpty()): Now we deserialize the
   bundle skipping the custom values above (we're able to do this now
   with the length written on the wire) and instead placing LazyValue
   objects for them in the map.

3. We query one of the lazy values: Now, we deserialize the object
   represented by LazyValue and replace it on the map.

Since after (2) LazyValue objects are the only ones holding references
to the original Parcel, when all LazyValues are deserialized, the
original Parcel is available for GC.

Inside bundle now we differentiate between unparcel(itemwise = true) and
unparcel(itemwise = false) where the first also deserializes each item
(such that there are no LazyValues in the map). This is because some
operations such as kindofEquals() need all items deserialized.

I had to break a few methods in parcel into multiple methods in parcel
to be able to control the format in bundle. They are all @hide.

* In quick local experiments, counting the bytes that didn't need to be
deserialized after the change. Roughly 10% of bytes from custom-type
items in Bundle are not deserialized in the testing scenario (if I
haven't messed up the stats :). That's on a sdk_gphone_x86_64_arm64 a
few minutes after boot. Check
https://screenshot.googleplex.com/53uXrrqDMYahzg3, stats collection is
on ag/15403076.

Test: atest -d android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest
Test: Boot device
Bug: 195622897
Change-Id: Icfe8880cad00c3cd2afcbe4b92400ad4579e680e
parent 6f1a606d
Loading
Loading
Loading
Loading
+75 −28
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import com.android.internal.util.IndentingPrintWriter;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Set;
import java.util.function.Supplier;

/**
 * A mapping from String keys to values of various types. In most cases, you
@@ -38,7 +39,8 @@ import java.util.Set;
 * {@link PersistableBundle} subclass.
 */
public class BaseBundle {
    private static final String TAG = "Bundle";
    /** @hide */
    protected static final String TAG = "Bundle";
    static final boolean DEBUG = false;

    // Keep them in sync with frameworks/native/libs/binder/PersistableBundle.cpp.
@@ -95,7 +97,7 @@ public class BaseBundle {
    Parcel mParcelledData = null;

    /**
     * Whether {@link #mParcelledData} was generated by native coed or not.
     * Whether {@link #mParcelledData} was generated by native code or not.
     */
    private boolean mParcelledByNative;

@@ -198,7 +200,7 @@ public class BaseBundle {
        if (size == 0) {
            return null;
        }
        Object o = mMap.valueAt(0);
        Object o = getValueAt(0);
        try {
            return (String) o;
        } catch (ClassCastException e) {
@@ -229,7 +231,12 @@ public class BaseBundle {
     * using the currently assigned class loader.
     */
    @UnsupportedAppUsage
    /* package */ void unparcel() {
    final void unparcel() {
        unparcel(/* itemwise */ false);
    }

    /** Deserializes the underlying data and each item if {@code itemwise} is true. */
    final void unparcel(boolean itemwise) {
        synchronized (this) {
            final Parcel source = mParcelledData;
            if (source != null) {
@@ -241,9 +248,42 @@ public class BaseBundle {
                            + ": no parcelled data");
                }
            }
            if (itemwise) {
                for (int i = 0, n = mMap.size(); i < n; i++) {
                    // Triggers deserialization of i-th item, if needed
                    getValueAt(i);
                }
            }
        }
    }

    /**
     * Returns the value for key {@code key}.
     *
     * @hide
     */
    final Object getValue(String key) {
        int i = mMap.indexOfKey(key);
        return (i >= 0) ? getValueAt(i) : null;
    }

    /**
     * Returns the value for a certain position in the array map.
     *
     * @hide
     */
    final Object getValueAt(int i) {
        Object object = mMap.valueAt(i);
        if (object instanceof Supplier<?>) {
            Supplier<?> supplier = (Supplier<?>) object;
            synchronized (this) {
                object = supplier.get();
            }
            mMap.setValueAt(i, object);
        }
        return object;
    }

    private void initializeFromParcelLocked(@NonNull Parcel parcelledData, boolean recycleParcel,
            boolean parcelledByNative) {
        if (LOG_DEFUSABLE && sShouldDefuse && (mFlags & FLAG_DEFUSABLE) == 0) {
@@ -282,15 +322,8 @@ public class BaseBundle {
            map.ensureCapacity(count);
        }
        try {
            if (parcelledByNative) {
                // If it was parcelled by native code, then the array map keys aren't sorted
                // by their hash codes, so use the safe (slow) one.
                parcelledData.readArrayMapSafelyInternal(map, count, mClassLoader);
            } else {
                // If parcelled by Java, we know the contents are sorted properly,
                // so we can use ArrayMap.append().
                parcelledData.readArrayMapInternal(map, count, mClassLoader);
            }
            recycleParcel &= parcelledData.readArrayMap(map, count, !parcelledByNative,
                    /* lazy */ true, mClassLoader);
        } catch (BadParcelableException e) {
            if (sShouldDefuse) {
                Log.w(TAG, "Failed to parse Bundle, but defusing quietly", e);
@@ -342,7 +375,7 @@ public class BaseBundle {

    /** @hide */
    ArrayMap<String, Object> getMap() {
        unparcel();
        unparcel(/* itemwise */ true);
        return mMap;
    }

@@ -400,7 +433,12 @@ public class BaseBundle {
    }

    /**
     * @hide This kind-of does an equality comparison.  Kind-of.
     * Performs a loose equality check, which means there can be false negatives but if the method
     * returns true than both objects are guaranteed to be equal.
     *
     * The point is that this method is a light-weight check in performance terms.
     *
     * @hide
     */
    public boolean kindofEquals(BaseBundle other) {
        if (other == null) {
@@ -415,6 +453,12 @@ public class BaseBundle {
        } else if (isParcelled()) {
            return mParcelledData.compareData(other.mParcelledData) == 0;
        } else {
            // Following semantic above of failing in case we get a serialized value vs a
            // deserialized one, we'll compare the map. If a certain element hasn't been
            // deserialized yet, it's a Supplier (or more specifically a LazyValue, but let's
            // pretend we don't know that here :P), we'll use that element's equality comparison as
            // map naturally does. That will takes care of comparing the payload if needed (see
            // Parcel.readLazyValue() for details).
            return mMap.equals(other.mMap);
        }
    }
@@ -453,7 +497,7 @@ public class BaseBundle {
                    final int N = fromMap.size();
                    mMap = new ArrayMap<>(N);
                    for (int i = 0; i < N; i++) {
                        mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
                        mMap.append(fromMap.keyAt(i), deepCopyValue(from.getValueAt(i)));
                    }
                }
            } else {
@@ -526,7 +570,7 @@ public class BaseBundle {
    @Nullable
    public Object get(String key) {
        unparcel();
        return mMap.get(key);
        return getValue(key);
    }

    /**
@@ -1001,7 +1045,7 @@ public class BaseBundle {
     */
    char getChar(String key, char defaultValue) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return defaultValue;
        }
@@ -1266,7 +1310,7 @@ public class BaseBundle {
    @Nullable
    Serializable getSerializable(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1289,7 +1333,7 @@ public class BaseBundle {
    @Nullable
    ArrayList<Integer> getIntegerArrayList(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1312,7 +1356,7 @@ public class BaseBundle {
    @Nullable
    ArrayList<String> getStringArrayList(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1335,7 +1379,7 @@ public class BaseBundle {
    @Nullable
    ArrayList<CharSequence> getCharSequenceArrayList(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1404,7 +1448,7 @@ public class BaseBundle {
    @Nullable
    short[] getShortArray(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1427,7 +1471,7 @@ public class BaseBundle {
    @Nullable
    char[] getCharArray(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1496,7 +1540,7 @@ public class BaseBundle {
    @Nullable
    float[] getFloatArray(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1585,7 +1629,7 @@ public class BaseBundle {
    void writeToParcelInner(Parcel parcel, int flags) {
        // If the parcel has a read-write helper, we can't just copy the blob, so unparcel it first.
        if (parcel.hasReadWriteHelper()) {
            unparcel();
            unparcel(/* itemwise */ true);
        }
        // Keep implementation in sync with writeToParcel() in
        // frameworks/native/libs/binder/PersistableBundle.cpp.
@@ -1660,10 +1704,13 @@ public class BaseBundle {
        }

        if (parcel.hasReadWriteHelper()) {
            // If the parcel has a read-write helper, then we can't lazily-unparcel it, so just
            // unparcel right away.
            // If the parcel has a read-write helper, it's better to deserialize immediately
            // otherwise the helper would have to either maintain valid state long after the bundle
            // had been constructed with parcel or to make sure they trigger deserialization of the
            // bundle immediately; neither of which is obvious.
            synchronized (this) {
                initializeFromParcelLocked(parcel, /*recycleParcel=*/ false, isNativeBundle);
                unparcel(/* itemwise */ true);
            }
            return;
        }
+11 −49
Original line number Diff line number Diff line
@@ -330,48 +330,10 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
                // It's been unparcelled, so we need to walk the map
                for (int i=mMap.size()-1; i>=0; i--) {
                    Object obj = mMap.valueAt(i);
                    if (obj instanceof Parcelable) {
                        if ((((Parcelable)obj).describeContents()
                                & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0) {
                    if (Parcel.hasFileDescriptors(obj)) {
                        fdFound = true;
                        break;
                    }
                    } else if (obj instanceof Parcelable[]) {
                        Parcelable[] array = (Parcelable[]) obj;
                        for (int n = array.length - 1; n >= 0; n--) {
                            Parcelable p = array[n];
                            if (p != null && ((p.describeContents()
                                    & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0)) {
                                fdFound = true;
                                break;
                            }
                        }
                    } else if (obj instanceof SparseArray) {
                        SparseArray<? extends Parcelable> array =
                                (SparseArray<? extends Parcelable>) obj;
                        for (int n = array.size() - 1; n >= 0; n--) {
                            Parcelable p = array.valueAt(n);
                            if (p != null && (p.describeContents()
                                    & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0) {
                                fdFound = true;
                                break;
                            }
                        }
                    } else if (obj instanceof ArrayList) {
                        ArrayList array = (ArrayList) obj;
                        // an ArrayList here might contain either Strings or
                        // Parcelables; only look inside for Parcelables
                        if (!array.isEmpty() && (array.get(0) instanceof Parcelable)) {
                            for (int n = array.size() - 1; n >= 0; n--) {
                                Parcelable p = (Parcelable) array.get(n);
                                if (p != null && ((p.describeContents()
                                        & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0)) {
                                    fdFound = true;
                                    break;
                                }
                            }
                        }
                    }
                }
            }

@@ -391,7 +353,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
     */
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
    public Bundle filterValues() {
        unparcel();
        unparcel(/* itemwise */ true);
        Bundle bundle = this;
        if (mMap != null) {
            ArrayMap<String, Object> map = mMap;
@@ -972,7 +934,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    @Nullable
    public Bundle getBundle(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -999,7 +961,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    @Nullable
    public <T extends Parcelable> T getParcelable(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1026,7 +988,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    @Nullable
    public Parcelable[] getParcelableArray(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1053,7 +1015,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    @Nullable
    public <T extends Parcelable> ArrayList<T> getParcelableArrayList(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1077,7 +1039,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    @Nullable
    public <T extends Parcelable> SparseArray<T> getSparseParcelableArray(@Nullable String key) {
        unparcel();
        Object o = mMap.get(key);
        Object o = getValue(key);
        if (o == null) {
            return null;
        }
@@ -1300,7 +1262,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
    public void writeToParcel(Parcel parcel, int flags) {
        final boolean oldAllowFds = parcel.pushAllowFds((mFlags & FLAG_ALLOW_FDS) != 0);
        try {
            super.writeToParcelInner(parcel, flags);
            writeToParcelInner(parcel, flags);
        } finally {
            parcel.restoreAllowFds(oldAllowFds);
        }
@@ -1312,7 +1274,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
     * @param parcel The parcel to overwrite this bundle from.
     */
    public void readFromParcel(Parcel parcel) {
        super.readFromParcelInner(parcel);
        readFromParcelInner(parcel);
        mFlags = FLAG_ALLOW_FDS;
        maybePrefillHasFds();
    }
+474 −93

File changed.

Preview size limit exceeded, changes collapsed.