Do not allow adding custom Parcelable in Bundles
This CL prevents the API users from passing any Bundles with
custom Parcelable to following APIs:
- MediaSession2.Builder#setExtras()
- MediaController2.Builder#setConnectionHints()
- MediaSession constructor
Bug: 135572812
Test: Passed followings tests
atest CtsMediaTestCases:android.media.cts.MediaSessionTest;
atest CtsMediaTestCases:android.media.cts.MediaControllerTest;
atest CtsMediaTestCases:android.media.cts.MediaBrowserTest;
atest CtsMediaTestCases:android.media.cts.MediaSessionManagerTest;
atest CtsMediaTestCases:android.media.cts.MediaSession2Test;
atest CtsMediaTestCases:android.media.cts.MediaController2Test;
atest CtsMediaTestCases:android.media.cts.MediaSession2ServiceTest;
Change-Id: I703c9fc0b180fb7bb3bf9bbec677f01a2a128c7a
diff --git a/media/apex/java/android/media/Media2Utils.java b/media/apex/java/android/media/Media2Utils.java
index 5fd6191..a87e967 100644
--- a/media/apex/java/android/media/Media2Utils.java
+++ b/media/apex/java/android/media/Media2Utils.java
@@ -75,5 +75,4 @@
Log.v(TAG, "storeCookies: cookieHandler: " + cookieHandler + " Cookies: " + cookies);
}
-
}
diff --git a/media/apex/java/android/media/MediaController2.java b/media/apex/java/android/media/MediaController2.java
index 9848f1a..63a4510 100644
--- a/media/apex/java/android/media/MediaController2.java
+++ b/media/apex/java/android/media/MediaController2.java
@@ -100,7 +100,7 @@
* @param callback controller callback to receive changes in.
*/
MediaController2(@NonNull Context context, @NonNull Session2Token token,
- @Nullable Bundle connectionHints, @NonNull Executor executor,
+ @NonNull Bundle connectionHints, @NonNull Executor executor,
@NonNull ControllerCallback callback) {
if (context == null) {
throw new IllegalArgumentException("context shouldn't be null");
@@ -259,7 +259,16 @@
Session2CommandGroup allowedCommands =
connectionResult.getParcelable(KEY_ALLOWED_COMMANDS);
boolean playbackActive = connectionResult.getBoolean(KEY_PLAYBACK_ACTIVE);
+
Bundle tokenExtras = connectionResult.getBundle(KEY_TOKEN_EXTRAS);
+ if (tokenExtras == null) {
+ Log.w(TAG, "extras shouldn't be null.");
+ tokenExtras = Bundle.EMPTY;
+ } else if (MediaSession2.hasCustomParcelable(tokenExtras)) {
+ Log.w(TAG, "extras contain custom parcelable. Ignoring.");
+ tokenExtras = Bundle.EMPTY;
+ }
+
if (DEBUG) {
Log.d(TAG, "notifyConnected sessionBinder=" + sessionBinder
+ ", allowedCommands=" + allowedCommands);
@@ -343,7 +352,7 @@
}
}
- private Bundle createConnectionRequest(@Nullable Bundle connectionHints) {
+ private Bundle createConnectionRequest(@NonNull Bundle connectionHints) {
Bundle connectionRequest = new Bundle();
connectionRequest.putString(KEY_PACKAGE_NAME, mContext.getPackageName());
connectionRequest.putInt(KEY_PID, Process.myPid());
@@ -351,7 +360,7 @@
return connectionRequest;
}
- private boolean requestConnectToSession(@Nullable Bundle connectionHints) {
+ private boolean requestConnectToSession(@NonNull Bundle connectionHints) {
Session2Link sessionBinder = mSessionToken.getSessionLink();
Bundle connectionRequest = createConnectionRequest(connectionHints);
try {
@@ -430,6 +439,9 @@
* <p>
* {@code connectionHints} is a session-specific argument to send to the session when
* connecting. The contents of this bundle may affect the connection result.
+ * <p>
+ * An {@link IllegalArgumentException} will be thrown if the bundle contains any
+ * non-framework Parcelable objects.
*
* @param connectionHints a bundle which contains the connection hints
* @return The Builder to allow chaining
@@ -439,6 +451,10 @@
if (connectionHints == null) {
throw new IllegalArgumentException("connectionHints shouldn't be null");
}
+ if (MediaSession2.hasCustomParcelable(connectionHints)) {
+ throw new IllegalArgumentException("connectionHints shouldn't contain any custom "
+ + "parcelables");
+ }
mConnectionHints = new Bundle(connectionHints);
return this;
}
@@ -477,6 +493,9 @@
if (mCallback == null) {
mCallback = new ControllerCallback() {};
}
+ if (mConnectionHints == null) {
+ mConnectionHints = Bundle.EMPTY;
+ }
return new MediaController2(
mContext, mToken, mConnectionHints, mCallbackExecutor, mCallback);
}
diff --git a/media/apex/java/android/media/MediaSession2.java b/media/apex/java/android/media/MediaSession2.java
index 0819118..b3edf3f 100644
--- a/media/apex/java/android/media/MediaSession2.java
+++ b/media/apex/java/android/media/MediaSession2.java
@@ -34,8 +34,10 @@
import android.content.Intent;
import android.media.session.MediaSessionManager;
import android.media.session.MediaSessionManager.RemoteUserInfo;
+import android.os.BadParcelableException;
import android.os.Bundle;
import android.os.Handler;
+import android.os.Parcel;
import android.os.Process;
import android.os.ResultReceiver;
import android.util.ArrayMap;
@@ -97,7 +99,7 @@
MediaSession2(@NonNull Context context, @NonNull String id, PendingIntent sessionActivity,
@NonNull Executor callbackExecutor, @NonNull SessionCallback callback,
- Bundle tokenExtras) {
+ @NonNull Bundle tokenExtras) {
synchronized (MediaSession2.class) {
if (SESSION_ID_LIST.contains(id)) {
throw new IllegalStateException("Session ID must be unique. ID=" + id);
@@ -276,6 +278,35 @@
return controllers;
}
+ /**
+ * Returns whether the given bundle includes non-framework Parcelables.
+ */
+ static boolean hasCustomParcelable(@Nullable Bundle bundle) {
+ if (bundle == null) {
+ return false;
+ }
+
+ // Try writing the bundle to parcel, and read it with framework classloader.
+ Parcel parcel = null;
+ try {
+ parcel = Parcel.obtain();
+ parcel.writeBundle(bundle);
+ parcel.setDataPosition(0);
+ Bundle out = parcel.readBundle(null);
+
+ // Calling Bundle#size() will trigger Bundle#unparcel().
+ out.size();
+ } catch (BadParcelableException e) {
+ Log.d(TAG, "Custom parcelable in bundle.", e);
+ return true;
+ } finally {
+ if (parcel != null) {
+ parcel.recycle();
+ }
+ }
+ return false;
+ }
+
boolean isClosed() {
synchronized (mLock) {
return mClosed;
@@ -309,11 +340,21 @@
String callingPkg = connectionRequest.getString(KEY_PACKAGE_NAME);
RemoteUserInfo remoteUserInfo = new RemoteUserInfo(callingPkg, callingPid, callingUid);
+
+ Bundle connectionHints = connectionRequest.getBundle(KEY_CONNECTION_HINTS);
+ if (connectionHints == null) {
+ Log.w(TAG, "connectionHints shouldn't be null.");
+ connectionHints = Bundle.EMPTY;
+ } else if (hasCustomParcelable(connectionHints)) {
+ Log.w(TAG, "connectionHints contain custom parcelable. Ignoring.");
+ connectionHints = Bundle.EMPTY;
+ }
+
final ControllerInfo controllerInfo = new ControllerInfo(
remoteUserInfo,
mSessionManager.isTrustedForMediaControl(remoteUserInfo),
controller,
- connectionRequest.getBundle(KEY_CONNECTION_HINTS));
+ connectionHints);
mCallbackExecutor.execute(() -> {
boolean connected = false;
try {
@@ -516,7 +557,8 @@
/**
* Set extras for the session token. If null or not set, {@link Session2Token#getExtras()}
- * will return {@link Bundle#EMPTY}.
+ * will return an empty {@link Bundle}. An {@link IllegalArgumentException} will be thrown
+ * if the bundle contains any non-framework Parcelable objects.
*
* @return The Builder to allow chaining
* @see Session2Token#getExtras()
@@ -526,7 +568,11 @@
if (extras == null) {
throw new NullPointerException("extras shouldn't be null");
}
- mExtras = extras;
+ if (hasCustomParcelable(extras)) {
+ throw new IllegalArgumentException(
+ "extras shouldn't contain any custom parcelables");
+ }
+ mExtras = new Bundle(extras);
return this;
}
@@ -548,6 +594,9 @@
if (mId == null) {
mId = "";
}
+ if (mExtras == null) {
+ mExtras = Bundle.EMPTY;
+ }
MediaSession2 session2 = new MediaSession2(mContext, mId, mSessionActivity,
mCallbackExecutor, mCallback, mExtras);
@@ -596,7 +645,7 @@
* connection result.
*/
ControllerInfo(@NonNull RemoteUserInfo remoteUserInfo, boolean trusted,
- @Nullable Controller2Link controllerBinder, @Nullable Bundle connectionHints) {
+ @Nullable Controller2Link controllerBinder, @NonNull Bundle connectionHints) {
mRemoteUserInfo = remoteUserInfo;
mIsTrusted = trusted;
mControllerBinder = controllerBinder;
@@ -629,11 +678,11 @@
}
/**
- * @return connection hints sent from controller, or {@link Bundle#EMPTY} if none.
+ * @return connection hints sent from controller.
*/
@NonNull
public Bundle getConnectionHints() {
- return mConnectionHints == null ? Bundle.EMPTY : new Bundle(mConnectionHints);
+ return new Bundle(mConnectionHints);
}
/**
diff --git a/media/apex/java/android/media/MediaSession2Service.java b/media/apex/java/android/media/MediaSession2Service.java
index b8bf384..ee584e5 100644
--- a/media/apex/java/android/media/MediaSession2Service.java
+++ b/media/apex/java/android/media/MediaSession2Service.java
@@ -378,12 +378,22 @@
callingPkg,
pid == 0 ? connectionRequest.getInt(KEY_PID) : pid,
uid);
+
+ Bundle connectionHints = connectionRequest.getBundle(KEY_CONNECTION_HINTS);
+ if (connectionHints == null) {
+ Log.w(TAG, "connectionHints shouldn't be null.");
+ connectionHints = Bundle.EMPTY;
+ } else if (MediaSession2.hasCustomParcelable(connectionHints)) {
+ Log.w(TAG, "connectionHints contain custom parcelable. Ignoring.");
+ connectionHints = Bundle.EMPTY;
+ }
+
final ControllerInfo controllerInfo = new ControllerInfo(
remoteUserInfo,
service.getMediaSessionManager()
.isTrustedForMediaControl(remoteUserInfo),
caller,
- connectionRequest.getBundle(KEY_CONNECTION_HINTS));
+ connectionHints);
if (DEBUG) {
Log.d(TAG, "Handling incoming connection request from the"
diff --git a/media/apex/java/android/media/Session2Token.java b/media/apex/java/android/media/Session2Token.java
index d7cb978..6d499fa 100644
--- a/media/apex/java/android/media/Session2Token.java
+++ b/media/apex/java/android/media/Session2Token.java
@@ -118,11 +118,11 @@
mUid = uid;
mType = TYPE_SESSION_SERVICE;
mSessionLink = null;
- mExtras = null;
+ mExtras = Bundle.EMPTY;
}
Session2Token(int uid, int type, String packageName, Session2Link sessionLink,
- Bundle tokenExtras) {
+ @NonNull Bundle tokenExtras) {
mUid = uid;
mType = type;
mPackageName = packageName;
@@ -139,7 +139,16 @@
mServiceName = in.readString();
mSessionLink = in.readParcelable(null);
mComponentName = ComponentName.unflattenFromString(in.readString());
- mExtras = in.readBundle();
+
+ Bundle extras = in.readBundle();
+ if (extras == null) {
+ Log.w(TAG, "extras shouldn't be null.");
+ extras = Bundle.EMPTY;
+ } else if (MediaSession2.hasCustomParcelable(extras)) {
+ Log.w(TAG, "extras contain custom parcelable. Ignoring.");
+ extras = Bundle.EMPTY;
+ }
+ mExtras = extras;
}
@Override
@@ -220,7 +229,7 @@
*/
@NonNull
public Bundle getExtras() {
- return mExtras == null ? Bundle.EMPTY : mExtras;
+ return new Bundle(mExtras);
}
Session2Link getSessionLink() {
diff --git a/media/java/android/media/session/MediaController.java b/media/java/android/media/session/MediaController.java
index c1c7fca..1fc4f7d 100644
--- a/media/java/android/media/session/MediaController.java
+++ b/media/java/android/media/session/MediaController.java
@@ -414,7 +414,7 @@
/**
* Gets the additional session information which was set when the session was created.
*
- * @return The additional session information, or {@link Bundle#EMPTY} if not set.
+ * @return The additional session information, or an empty {@link Bundle} if not set.
*/
@NonNull
public Bundle getSessionInfo() {
@@ -430,6 +430,10 @@
}
if (mSessionInfo == null) {
+ Log.w(TAG, "sessionInfo shouldn't be null.");
+ mSessionInfo = Bundle.EMPTY;
+ } else if (MediaSession.hasCustomParcelable(mSessionInfo)) {
+ Log.w(TAG, "sessionInfo contains custom parcelable. Ignoring.");
mSessionInfo = Bundle.EMPTY;
}
return new Bundle(mSessionInfo);
diff --git a/media/java/android/media/session/MediaSession.java b/media/java/android/media/session/MediaSession.java
index c4085f8..e11715f 100644
--- a/media/java/android/media/session/MediaSession.java
+++ b/media/java/android/media/session/MediaSession.java
@@ -32,6 +32,7 @@
import android.media.VolumeProvider;
import android.media.session.MediaSessionManager.RemoteUserInfo;
import android.net.Uri;
+import android.os.BadParcelableException;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
@@ -168,6 +169,8 @@
* @param sessionInfo A bundle for additional information about this session.
* Controllers can get this information by calling
* {@link MediaController#getSessionInfo()}.
+ * An {@link IllegalArgumentException} will be thrown if this contains
+ * any non-framework Parcelable objects.
*/
public MediaSession(@NonNull Context context, @NonNull String tag,
@Nullable Bundle sessionInfo) {
@@ -177,6 +180,11 @@
if (TextUtils.isEmpty(tag)) {
throw new IllegalArgumentException("tag cannot be null or empty");
}
+ if (hasCustomParcelable(sessionInfo)) {
+ throw new IllegalArgumentException("sessionInfo shouldn't contain any custom "
+ + "parcelables");
+ }
+
mMaxBitmapSize = context.getResources().getDimensionPixelSize(
com.android.internal.R.dimen.config_mediaMetadataBitmapMaxSize);
mCbStub = new CallbackStub(this);
@@ -600,6 +608,35 @@
return false;
}
+ /**
+ * Returns whether the given bundle includes non-framework Parcelables.
+ */
+ static boolean hasCustomParcelable(@Nullable Bundle bundle) {
+ if (bundle == null) {
+ return false;
+ }
+
+ // Try writing the bundle to parcel, and read it with framework classloader.
+ Parcel parcel = null;
+ try {
+ parcel = Parcel.obtain();
+ parcel.writeBundle(bundle);
+ parcel.setDataPosition(0);
+ Bundle out = parcel.readBundle(null);
+
+ // Calling Bundle#size() will trigger Bundle#unparcel().
+ out.size();
+ } catch (BadParcelableException e) {
+ Log.d(TAG, "Custom parcelable in bundle.", e);
+ return true;
+ } finally {
+ if (parcel != null) {
+ parcel.recycle();
+ }
+ }
+ return false;
+ }
+
void dispatchPrepare(RemoteUserInfo caller) {
postToCallback(caller, CallbackMessageHandler.MSG_PREPARE, null, null);
}