Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid full unparcelling where possible in Bundle

Browse Source 
and put a warning where it's not possible. This is to make sure we know
when to touch bundles provided by apps.

In this CL:
* deepCopy() now doesn't deserialize lazy objects before copying. It
  doesn't copy them either, it merely passes them into the new map as
  the same reference. This works because we implemented fine-grained
  locking into each lazy value, so concurrent access won't be a problem.
  Furthermore, LazyValue caches the deserialized object, so we'd still
  honor the contract of deepCopy() that "Other types of objects (such
  as Parcelable or Serializable) are referenced as-is and not copied in
  any way". I had to perform one extra check in the synchronized block
  in LazyValue to guarantee this (double-checked locking), I explain
  that in the comments.
* Removed filterValues() and codepaths that used it. This was created
  with the purpose of removing items whose classes weren't available to
  the system to prevent crashes coming from full deserialization. This
  is not a concern anymore with lazy bundle, hence we can remove the
  codepaths altogether (see email for more details).
* Put warnings in javadoc of getMap() and PeristableBundle().

Test: Boots
Test: atest -d android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest
Bug: 195622897
Change-Id: I14bb6a7874814f42cbcc6b5fd372c42752aa74c8
This commit is contained in:
Bernardo Rufino
2021-09-27 17:28:38 +01:00
parent d3199a6084
commit 9e26212dac
6 changed files with 28 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
core/java/android/app/ActivityThread.java
View File

@@ -3724,7 +3724,6 @@ public final class ActivityThread extends ClientTransactionHandler {
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 {

10
core/java/android/content/Intent.java
View File

@@ -8567,16 +8567,6 @@ public class Intent implements Parcelable, Cloneable {
: null;
}
/**
* 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.

16
core/java/android/os/BaseBundle.java
View File

@@ -376,8 +376,16 @@ public class BaseBundle {
}
}
/** @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 +508,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(from.getValueAt(i)));
mMap.append(fromMap.keyAt(i), deepCopyValue(fromMap.valueAt(i)));
}
}
} else {
@@ -1772,7 +1780,7 @@ public class BaseBundle {
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));
}

50
core/java/android/os/Bundle.java
View File

@@ -347,56 +347,6 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable {
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) {

17
core/java/android/os/Parcel.java
View File

@@ -3470,14 +3470,17 @@ public final class Parcel {
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;

7
core/java/android/os/PersistableBundle.java
View File

@@ -34,6 +34,7 @@ import org.xmlpull.v1.XmlSerializer;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.Serializable;
import java.util.ArrayList;
/**
@@ -100,6 +101,10 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
/**
* 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 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
* @hide
*/
public PersistableBundle(Bundle b) {
this(b.getMap());
this(b.getItemwiseMap());
}
/**