From 31d70842e0033660e73c1c553deade4f213bd411 Mon Sep 17 00:00:00 2001 From: Bernardo Rufino Date: Tue, 2 Nov 2021 10:38:13 +0000 Subject: Improve @hide Parcel.hasFileDescriptors(Object) Addressing Jeff's comment on aosp/1787847 to support List<> in Parcel.hasFileDescriptor(), also added support for Map (+ArrayMap) and Object[] that were missing. Made the checks recursive since it's possible to put nested containers in bundle. Since that's @hide and only used by bundle, included Parcel as a supported type too and clarified in the javadoc. That allowed to clean up Bundle.hasFileDescriptors(). Bug: 195622897 Test: atest -d android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest Change-Id: I6acf358763d8f544fc6ff1a5b0c8bdff567d50be --- core/java/android/os/Bundle.java | 26 ++----------- core/java/android/os/Parcel.java | 79 +++++++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 55 deletions(-) (limited to 'core/java') diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java index 92eb7a5d047e..feca81e1aea1 100644 --- a/core/java/android/os/Bundle.java +++ b/core/java/android/os/Bundle.java @@ -324,28 +324,10 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable { */ public boolean hasFileDescriptors() { if ((mFlags & FLAG_HAS_FDS_KNOWN) == 0) { - boolean fdFound = false; // keep going until we find one or run out of data - - if (mParcelledData != null) { - if (mParcelledData.hasFileDescriptors()) { - fdFound = true; - } - } else { - // 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 (Parcel.hasFileDescriptors(obj)) { - fdFound = true; - break; - } - } - } - - if (fdFound) { - mFlags |= FLAG_HAS_FDS; - } else { - mFlags &= ~FLAG_HAS_FDS; - } + Parcel p = mParcelledData; + mFlags = (Parcel.hasFileDescriptors((p != null) ? p : mMap)) + ? mFlags | FLAG_HAS_FDS + : mFlags & ~FLAG_HAS_FDS; mFlags |= FLAG_HAS_FDS_KNOWN; } return (mFlags & FLAG_HAS_FDS) != 0; diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 61882ffcee85..623c03dc80dd 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -747,57 +747,70 @@ public final class Parcel { } /** - * Check if the object used in {@link #readValue(ClassLoader)} / {@link #writeValue(Object)} - * has file descriptors. + * Check if the object has file descriptors. + * + *

Objects supported are {@link Parcel} and objects that can be passed to {@link + * #writeValue(Object)}} * *

For most cases, it will use the self-reported {@link Parcelable#describeContents()} method * for that. * - * @throws IllegalArgumentException if you provide any object not supported by above methods. - * Most notably, if you pass {@link Parcel}, this method will throw, for that check - * {@link Parcel#hasFileDescriptors()} + * @throws IllegalArgumentException if you provide any object not supported by above methods + * (including if the unsupported object is inside a nested container). * * @hide */ public static boolean hasFileDescriptors(Object value) { - if (value instanceof LazyValue) { - return ((LazyValue) value).hasFileDescriptors(); + if (value instanceof Parcel) { + Parcel parcel = (Parcel) value; + if (parcel.hasFileDescriptors()) { + return true; + } + } else if (value instanceof LazyValue) { + LazyValue lazy = (LazyValue) value; + if (lazy.hasFileDescriptors()) { + return true; + } } else if (value instanceof Parcelable) { - if ((((Parcelable) value).describeContents() - & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0) { + Parcelable parcelable = (Parcelable) value; + if ((parcelable.describeContents() & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0) { return true; } - } else if (value instanceof Parcelable[]) { - Parcelable[] array = (Parcelable[]) value; - for (int n = array.length - 1; n >= 0; n--) { - Parcelable p = array[n]; - if (p != null && ((p.describeContents() - & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0)) { + } else if (value instanceof ArrayMap) { + ArrayMap map = (ArrayMap) value; + for (int i = 0, n = map.size(); i < n; i++) { + if (hasFileDescriptors(map.keyAt(i)) + || hasFileDescriptors(map.valueAt(i))) { + return true; + } + } + } else if (value instanceof Map) { + Map map = (Map) value; + for (Map.Entry entry : map.entrySet()) { + if (hasFileDescriptors(entry.getKey()) + || hasFileDescriptors(entry.getValue())) { + return true; + } + } + } else if (value instanceof List) { + List list = (List) value; + for (int i = 0, n = list.size(); i < n; i++) { + if (hasFileDescriptors(list.get(i))) { return true; } } } else if (value instanceof SparseArray) { SparseArray array = (SparseArray) value; - for (int n = array.size() - 1; n >= 0; n--) { - Object object = array.valueAt(n); - if (object instanceof Parcelable) { - Parcelable p = (Parcelable) object; - if (p != null && (p.describeContents() - & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0) { - return true; - } + for (int i = 0, n = array.size(); i < n; i++) { + if (hasFileDescriptors(array.valueAt(i))) { + return true; } } - } else if (value instanceof ArrayList) { - ArrayList array = (ArrayList) value; - for (int n = array.size() - 1; n >= 0; n--) { - Object object = array.get(n); - if (object instanceof Parcelable) { - Parcelable p = (Parcelable) object; - if (p != null && ((p.describeContents() - & Parcelable.CONTENTS_FILE_DESCRIPTOR) != 0)) { - return true; - } + } else if (value instanceof Object[]) { + Object[] array = (Object[]) value; + for (int i = 0, n = array.length; i < n; i++) { + if (hasFileDescriptors(array[i])) { + return true; } } } else { -- cgit v1.2.3