Merge "Mark Soong maintainers as a last resort suggestion"
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 749e8f5..7c140d6 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -3724,7 +3724,6 @@
Intent intent = new Intent(activityIntent);
intent.setFlags(intent.getFlags() & ~(Intent.FLAG_GRANT_WRITE_URI_PERMISSION
| Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION));
- intent.removeUnsafeExtras();
content.setDefaultIntent(intent);
}
} else {
diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
index 0fad63f..9746066 100644
--- a/core/java/android/content/Intent.java
+++ b/core/java/android/content/Intent.java
@@ -8568,16 +8568,6 @@
}
/**
- * Filter extras to only basic types.
- * @hide
- */
- public void removeUnsafeExtras() {
- if (mExtras != null) {
- mExtras = mExtras.filterValues();
- }
- }
-
- /**
* @return Whether {@link #maybeStripForHistory} will return an lightened intent or
* return itself as-is.
* @hide
diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java
index 7ce8d72..2a344f6 100644
--- a/core/java/android/os/BaseBundle.java
+++ b/core/java/android/os/BaseBundle.java
@@ -43,18 +43,22 @@
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 @@
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 @@
}
}
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 @@
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 @@
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 "
@@ -376,8 +395,16 @@
}
}
- /** @hide */
- ArrayMap<String, Object> getMap() {
+ /**
+ * Returns the backing map of this bundle after deserializing every item.
+ *
+ * <p><b>Warning:</b> This method will deserialize every item on the bundle, including custom
+ * types such as {@link Parcelable} and {@link Serializable}, so only use this when you trust
+ * the source. Specifically don't use this method on app-provided bundles.
+ *
+ * @hide
+ */
+ ArrayMap<String, Object> getItemwiseMap() {
unparcel(/* itemwise */ true);
return mMap;
}
@@ -500,7 +527,7 @@
final int N = fromMap.size();
mMap = new ArrayMap<>(N);
for (int i = 0; i < N; i++) {
- mMap.append(fromMap.keyAt(i), deepCopyValue(from.getValueAt(i)));
+ mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
}
}
} else {
@@ -1772,7 +1799,7 @@
pw.println("[null]");
return;
}
- final ArrayMap<String, Object> map = bundle.getMap();
+ final ArrayMap<String, Object> map = bundle.getItemwiseMap();
for (int i = 0; i < map.size(); i++) {
dumpStats(pw, map.keyAt(i), map.valueAt(i));
}
diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java
index 5626bde..b3827b3 100644
--- a/core/java/android/os/Bundle.java
+++ b/core/java/android/os/Bundle.java
@@ -347,56 +347,6 @@
return (mFlags & FLAG_HAS_FDS) != 0;
}
- /**
- * Filter values in Bundle to only basic types.
- * @hide
- */
- @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
- public Bundle filterValues() {
- unparcel(/* itemwise */ true);
- Bundle bundle = this;
- if (mMap != null) {
- ArrayMap<String, Object> map = mMap;
- for (int i = map.size() - 1; i >= 0; i--) {
- Object value = map.valueAt(i);
- if (PersistableBundle.isValidType(value)) {
- continue;
- }
- if (value instanceof Bundle) {
- Bundle newBundle = ((Bundle)value).filterValues();
- if (newBundle != value) {
- if (map == mMap) {
- // The filter had to generate a new bundle, but we have not yet
- // created a new one here. Do that now.
- bundle = new Bundle(this);
- // Note the ArrayMap<> constructor is guaranteed to generate
- // a new object with items in the same order as the original.
- map = bundle.mMap;
- }
- // Replace this current entry with the new child bundle.
- map.setValueAt(i, newBundle);
- }
- continue;
- }
- if (value.getClass().getName().startsWith("android.")) {
- continue;
- }
- if (map == mMap) {
- // This is the first time we have had to remove something, that means we
- // need to switch to a new Bundle.
- bundle = new Bundle(this);
- // Note the ArrayMap<> constructor is guaranteed to generate
- // a new object with items in the same order as the original.
- map = bundle.mMap;
- }
- map.removeAt(i);
- }
- }
- mFlags |= FLAG_HAS_FDS_KNOWN;
- mFlags &= ~FLAG_HAS_FDS;
- return bundle;
- }
-
/** {@hide} */
@Override
public void putObject(@Nullable String key, @Nullable Object value) {
diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java
index 358aa29..e6925cb 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -3532,14 +3532,17 @@
Parcel source = mSource;
if (source != null) {
synchronized (source) {
- int restore = source.dataPosition();
- try {
- source.setDataPosition(mPosition);
- mObject = source.readValue(mLoader);
- } finally {
- source.setDataPosition(restore);
+ // Check mSource != null guarantees callers won't ever see different objects.
+ if (mSource != null) {
+ int restore = source.dataPosition();
+ try {
+ source.setDataPosition(mPosition);
+ mObject = source.readValue(mLoader);
+ } finally {
+ source.setDataPosition(restore);
+ }
+ mSource = null;
}
- mSource = null;
}
}
return mObject;
@@ -3776,7 +3779,7 @@
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/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
index 339371b..c7edbec 100644
--- a/core/java/android/os/PersistableBundle.java
+++ b/core/java/android/os/PersistableBundle.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.io.Serializable;
import java.util.ArrayList;
/**
@@ -100,6 +101,10 @@
/**
* Constructs a PersistableBundle from a Bundle. Does only a shallow copy of the Bundle.
*
+ * <p><b>Warning:</b> This method will deserialize every item on the bundle, including custom
+ * types such as {@link Parcelable} and {@link Serializable}, so only use this when you trust
+ * the source. Specifically don't use this method on app-provided bundles.
+ *
* @param b a Bundle to be copied.
*
* @throws IllegalArgumentException if any element of {@code b} cannot be persisted.
@@ -107,7 +112,7 @@
* @hide
*/
public PersistableBundle(Bundle b) {
- this(b.getMap());
+ this(b.getItemwiseMap());
}
/**
diff --git a/core/tests/coretests/src/android/os/BundleTest.java b/core/tests/coretests/src/android/os/BundleTest.java
index 9d2cab3..ddd0070 100644
--- a/core/tests/coretests/src/android/os/BundleTest.java
+++ b/core/tests/coretests/src/android/os/BundleTest.java
@@ -47,6 +47,7 @@
@After
public void tearDown() throws Exception {
+ BaseBundle.setShouldDefuse(false);
if (mWtfHandler != null) {
Log.setWtfHandler(mWtfHandler);
}
@@ -355,6 +356,81 @@
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;