Check int and float parameters from external apps for validity

Some player method calls sent from MediaControllers accept int
or float values with a very clear API contract that disallows
some values. Filtering by these values early avoids calling a
Player implementation with invalid values.

PiperOrigin-RevId: 600413993
This commit is contained in:
tonihei 2024-01-22 04:06:27 -08:00 committed by Copybara-Service
parent 999e154b2a
commit c64b271f07
5 changed files with 185 additions and 17 deletions

View File

@ -462,6 +462,9 @@ import org.checkerframework.checker.initialization.qual.Initialized;
@Override @Override
public void onSetPlaybackSpeed(float speed) { public void onSetPlaybackSpeed(float speed) {
if (!(speed > 0f)) {
return;
}
dispatchSessionTaskWithPlayerCommand( dispatchSessionTaskWithPlayerCommand(
COMMAND_SET_SPEED_AND_PITCH, COMMAND_SET_SPEED_AND_PITCH,
controller -> sessionImpl.getPlayerWrapper().setPlaybackSpeed(speed), controller -> sessionImpl.getPlayerWrapper().setPlaybackSpeed(speed),
@ -470,6 +473,9 @@ import org.checkerframework.checker.initialization.qual.Initialized;
@Override @Override
public void onSkipToQueueItem(long queueId) { public void onSkipToQueueItem(long queueId) {
if (queueId < 0) {
return;
}
dispatchSessionTaskWithPlayerCommand( dispatchSessionTaskWithPlayerCommand(
COMMAND_SEEK_TO_MEDIA_ITEM, COMMAND_SEEK_TO_MEDIA_ITEM,
controller -> { controller -> {
@ -851,7 +857,7 @@ import org.checkerframework.checker.initialization.qual.Initialized;
} }
private void handleOnAddQueueItem(@Nullable MediaDescriptionCompat description, int index) { private void handleOnAddQueueItem(@Nullable MediaDescriptionCompat description, int index) {
if (description == null) { if (description == null || (index != C.INDEX_UNSET && index < 0)) {
return; return;
} }
dispatchSessionTaskWithPlayerCommand( dispatchSessionTaskWithPlayerCommand(

View File

@ -771,7 +771,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void seekToDefaultPositionWithMediaItemIndex( public void seekToDefaultPositionWithMediaItemIndex(
@Nullable IMediaController caller, int sequenceNumber, int mediaItemIndex) { @Nullable IMediaController caller, int sequenceNumber, int mediaItemIndex) {
if (caller == null) { if (caller == null || mediaItemIndex < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -799,7 +799,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void seekToWithMediaItemIndex( public void seekToWithMediaItemIndex(
@Nullable IMediaController caller, int sequenceNumber, int mediaItemIndex, long positionMs) { @Nullable IMediaController caller, int sequenceNumber, int mediaItemIndex, long positionMs) {
if (caller == null) { if (caller == null || mediaItemIndex < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -930,7 +930,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void setPlaybackSpeed(@Nullable IMediaController caller, int sequenceNumber, float speed) { public void setPlaybackSpeed(@Nullable IMediaController caller, int sequenceNumber, float speed) {
if (caller == null) { if (caller == null || !(speed > 0f)) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1088,7 +1088,9 @@ import java.util.concurrent.ExecutionException;
@Nullable IBinder mediaItemsRetriever, @Nullable IBinder mediaItemsRetriever,
int startIndex, int startIndex,
long startPositionMs) { long startPositionMs) {
if (caller == null || mediaItemsRetriever == null) { if (caller == null
|| mediaItemsRetriever == null
|| (startIndex != C.INDEX_UNSET && startIndex < 0)) {
return; return;
} }
List<MediaItem> mediaItemList; List<MediaItem> mediaItemList;
@ -1172,7 +1174,7 @@ import java.util.concurrent.ExecutionException;
int sequenceNumber, int sequenceNumber,
int index, int index,
@Nullable Bundle mediaItemBundle) { @Nullable Bundle mediaItemBundle) {
if (caller == null || mediaItemBundle == null) { if (caller == null || mediaItemBundle == null || index < 0) {
return; return;
} }
MediaItem mediaItem; MediaItem mediaItem;
@ -1229,7 +1231,7 @@ import java.util.concurrent.ExecutionException;
int sequenceNumber, int sequenceNumber,
int index, int index,
@Nullable IBinder mediaItemsRetriever) { @Nullable IBinder mediaItemsRetriever) {
if (caller == null || mediaItemsRetriever == null) { if (caller == null || mediaItemsRetriever == null || index < 0) {
return; return;
} }
List<MediaItem> mediaItems; List<MediaItem> mediaItems;
@ -1256,7 +1258,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void removeMediaItem(@Nullable IMediaController caller, int sequenceNumber, int index) { public void removeMediaItem(@Nullable IMediaController caller, int sequenceNumber, int index) {
if (caller == null) { if (caller == null || index < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1271,7 +1273,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void removeMediaItems( public void removeMediaItems(
@Nullable IMediaController caller, int sequenceNumber, int fromIndex, int toIndex) { @Nullable IMediaController caller, int sequenceNumber, int fromIndex, int toIndex) {
if (caller == null) { if (caller == null || fromIndex < 0 || toIndex < fromIndex) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1300,7 +1302,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void moveMediaItem( public void moveMediaItem(
@Nullable IMediaController caller, int sequenceNumber, int currentIndex, int newIndex) { @Nullable IMediaController caller, int sequenceNumber, int currentIndex, int newIndex) {
if (caller == null) { if (caller == null || currentIndex < 0 || newIndex < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1317,7 +1319,7 @@ import java.util.concurrent.ExecutionException;
int fromIndex, int fromIndex,
int toIndex, int toIndex,
int newIndex) { int newIndex) {
if (caller == null) { if (caller == null || fromIndex < 0 || toIndex < fromIndex || newIndex < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1333,7 +1335,7 @@ import java.util.concurrent.ExecutionException;
int sequenceNumber, int sequenceNumber,
int index, int index,
@Nullable Bundle mediaItemBundle) { @Nullable Bundle mediaItemBundle) {
if (caller == null || mediaItemBundle == null) { if (caller == null || mediaItemBundle == null || index < 0) {
return; return;
} }
MediaItem mediaItem; MediaItem mediaItem;
@ -1371,7 +1373,7 @@ import java.util.concurrent.ExecutionException;
int fromIndex, int fromIndex,
int toIndex, int toIndex,
@Nullable IBinder mediaItemsRetriever) { @Nullable IBinder mediaItemsRetriever) {
if (caller == null || mediaItemsRetriever == null) { if (caller == null || mediaItemsRetriever == null || fromIndex < 0 || toIndex < fromIndex) {
return; return;
} }
ImmutableList<MediaItem> mediaItems; ImmutableList<MediaItem> mediaItems;
@ -1468,6 +1470,11 @@ import java.util.concurrent.ExecutionException;
if (caller == null) { if (caller == null) {
return; return;
} }
if (repeatMode != Player.REPEAT_MODE_ALL
&& repeatMode != Player.REPEAT_MODE_OFF
&& repeatMode != Player.REPEAT_MODE_ONE) {
return;
}
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
caller, caller,
sequenceNumber, sequenceNumber,
@ -1503,7 +1510,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void setVolume(@Nullable IMediaController caller, int sequenceNumber, float volume) { public void setVolume(@Nullable IMediaController caller, int sequenceNumber, float volume) {
if (caller == null) { if (caller == null || !(volume >= 0f && volume <= 1f)) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1516,7 +1523,7 @@ import java.util.concurrent.ExecutionException;
@SuppressWarnings("deprecation") // Backwards compatibility a for flag-less method @SuppressWarnings("deprecation") // Backwards compatibility a for flag-less method
@Override @Override
public void setDeviceVolume(@Nullable IMediaController caller, int sequenceNumber, int volume) { public void setDeviceVolume(@Nullable IMediaController caller, int sequenceNumber, int volume) {
if (caller == null) { if (caller == null || volume < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(
@ -1529,7 +1536,7 @@ import java.util.concurrent.ExecutionException;
@Override @Override
public void setDeviceVolumeWithFlags( public void setDeviceVolumeWithFlags(
@Nullable IMediaController caller, int sequenceNumber, int volume, int flags) { @Nullable IMediaController caller, int sequenceNumber, int volume, int flags) {
if (caller == null) { if (caller == null || volume < 0) {
return; return;
} }
queueSessionTaskWithPlayerCommand( queueSessionTaskWithPlayerCommand(

View File

@ -411,6 +411,73 @@ public class MediaSessionStubTest {
/* libraryParams= */ getThrowingBundle()); /* libraryParams= */ getThrowingBundle());
binder.subscribe( binder.subscribe(
caller, /* seq= */ 0, /* parentId= */ "", /* libraryParams= */ getThrowingBundle()); caller, /* seq= */ 0, /* parentId= */ "", /* libraryParams= */ getThrowingBundle());
// Pass invalid int or float values.
binder.setVolume(/* caller= */ null, /* seq= */ 0, /* volume= */ -0.00001f);
binder.setVolume(/* caller= */ null, /* seq= */ 0, /* volume= */ 1.00001f);
binder.setVolume(/* caller= */ null, /* seq= */ 0, /* volume= */ Float.NaN);
binder.setDeviceVolume(/* caller= */ null, /* seq= */ 0, /* volume= */ -1);
binder.setDeviceVolumeWithFlags(
/* caller= */ null, /* seq= */ 0, /* volume= */ -1, /* flags= */ 0);
binder.setMediaItemsWithStartIndex(
/* caller= */ null,
/* seq= */ 0,
/* mediaItems= */ new BundleListRetriever(
ImmutableList.of(new MediaItem.Builder().build().toBundle())),
/* startIndex= */ -1,
/* startPositionMs= */ 0);
binder.setRepeatMode(/* caller= */ null, /* seq= */ 0, /* repeatMode= */ -1);
binder.setRepeatMode(/* caller= */ null, /* seq= */ 0, /* repeatMode= */ 3);
binder.removeMediaItem(/* caller= */ null, /* seq= */ 0, /* index= */ -1);
binder.removeMediaItems(
/* caller= */ null, /* seq= */ 0, /* fromIndex= */ -1, /* toIndex= */ 0);
binder.removeMediaItems(/* caller= */ null, /* seq= */ 0, /* fromIndex= */ 1, /* toIndex= */ 0);
binder.moveMediaItem(
/* caller= */ null, /* seq= */ 0, /* currentIndex= */ -1, /* newIndex= */ 0);
binder.moveMediaItem(
/* caller= */ null, /* seq= */ 0, /* currentIndex= */ 0, /* newIndex= */ -1);
binder.moveMediaItems(
/* caller= */ null, /* seq= */ 0, /* fromIndex= */ -1, /* toIndex= */ 0, /* newIndex= */ 0);
binder.moveMediaItems(
/* caller= */ null, /* seq= */ 0, /* fromIndex= */ 1, /* toIndex= */ 0, /* newIndex= */ 0);
binder.moveMediaItems(
/* caller= */ null, /* seq= */ 0, /* fromIndex= */ 0, /* toIndex= */ 0, /* newIndex= */ -1);
binder.replaceMediaItem(
/* caller= */ null,
/* seq= */ 0,
/* index= */ -1,
/* mediaItemBundle= */ new MediaItem.Builder().build().toBundle());
binder.replaceMediaItems(
/* caller= */ null,
/* seq= */ 0,
/* fromIndex= */ -1,
/* toIndex= */ 0,
/* mediaItems= */ new BundleListRetriever(
ImmutableList.of(new MediaItem.Builder().build().toBundle())));
binder.replaceMediaItems(
/* caller= */ null,
/* seq= */ 0,
/* fromIndex= */ 1,
/* toIndex= */ 0,
/* mediaItems= */ new BundleListRetriever(
ImmutableList.of(new MediaItem.Builder().build().toBundle())));
binder.setPlaybackSpeed(/* caller= */ null, /* seq= */ 0, /* speed= */ -0.0001f);
binder.setPlaybackSpeed(/* caller= */ null, /* seq= */ 0, /* speed= */ Float.NaN);
binder.addMediaItemWithIndex(
/* caller= */ null,
/* seq= */ 0,
/* index= */ -1,
/* mediaItemBundle= */ new MediaItem.Builder().build().toBundle());
binder.addMediaItemsWithIndex(
/* caller= */ null,
/* seq= */ 0,
/* index= */ -1,
/* mediaItems= */ new BundleListRetriever(
ImmutableList.of(new MediaItem.Builder().build().toBundle())));
binder.seekToDefaultPositionWithMediaItemIndex(
/* caller= */ null, /* seq= */ 0, /* mediaItemIndex= */ -1);
binder.seekToWithMediaItemIndex(
/* caller= */ null, /* seq= */ 0, /* mediaItemIndex= */ -1, /* positionMs= */ 0);
} }
@Test @Test
@ -430,7 +497,7 @@ public class MediaSessionStubTest {
caller, caller,
/* seq= */ 0, /* seq= */ 0,
/* connectionRequest= */ new ConnectionRequest( /* connectionRequest= */ new ConnectionRequest(
/* packageName= */ "invalud", /* pid= */ 9999, /* connectionHints= */ new Bundle()) /* packageName= */ "invalid", /* pid= */ 9999, /* connectionHints= */ new Bundle())
.toBundle()); .toBundle());
} }
} }

View File

@ -397,6 +397,25 @@ public class MediaSessionCallbackWithMediaControllerCompatTest {
assertThat(player.playbackParameters.speed).isEqualTo(testSpeed); assertThat(player.playbackParameters.speed).isEqualTo(testSpeed);
} }
@Test
public void setPlaybackSpeed_withInvalidSpeed_doesNotCrashSession() throws Exception {
session =
new MediaSession.Builder(context, player)
.setId("setPlaybackSpeed")
.setCallback(new TestSessionCallback())
.build();
controller =
new RemoteMediaControllerCompat(
context, session.getSessionCompat().getSessionToken(), /* waitForConnection= */ true);
controller.getTransportControls().setPlaybackSpeed(-0.0001f);
controller.getTransportControls().setPlaybackSpeed(Float.NaN);
controller.getTransportControls().setPlaybackSpeed(0.5f); // Add a valid action to wait for.
player.awaitMethodCalled(MockPlayer.METHOD_SET_PLAYBACK_SPEED, TIMEOUT_MS);
assertThat(player.playbackParameters.speed).isEqualTo(0.5f);
}
@Test @Test
public void addQueueItem() throws Exception { public void addQueueItem() throws Exception {
AtomicReference<List<MediaItem>> requestedMediaItems = new AtomicReference<>(); AtomicReference<List<MediaItem>> requestedMediaItems = new AtomicReference<>();
@ -491,6 +510,48 @@ public class MediaSessionCallbackWithMediaControllerCompatTest {
assertThat(player.mediaItems.get(1)).isEqualTo(resolvedMediaItem); assertThat(player.mediaItems.get(1)).isEqualTo(resolvedMediaItem);
} }
@Test
public void addQueueItemWithIndex_withInvalidIndex_doesNotCrashSession() throws Exception {
MediaItem resolvedMediaItem = MediaItem.fromUri(TEST_URI);
MediaSession.Callback callback =
new MediaSession.Callback() {
@Override
public ListenableFuture<List<MediaItem>> onAddMediaItems(
MediaSession mediaSession, ControllerInfo controller, List<MediaItem> mediaItems) {
// Resolve MediaItem asynchronously to test correct threading logic.
return executorService.submit(() -> ImmutableList.of(resolvedMediaItem));
}
};
session =
new MediaSession.Builder(context, player)
.setId("addQueueItemWithIndex_invalidIndex")
.setCallback(callback)
.build();
controller =
new RemoteMediaControllerCompat(
context, session.getSessionCompat().getSessionToken(), /* waitForConnection= */ true);
handler.postAndSync(
() -> {
List<MediaItem> mediaItems = MediaTestUtils.createMediaItems(/* size= */ 10);
player.setMediaItems(mediaItems);
player.timeline = MediaTestUtils.createTimeline(mediaItems);
player.notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
});
// Prepare an item to add.
MediaDescriptionCompat desc =
new MediaDescriptionCompat.Builder()
.setMediaId("media_id")
.setMediaUri(Uri.parse("https://test.test"))
.build();
controller.addQueueItem(desc, /* index= */ -1);
controller.addQueueItem(desc, /* index= */ 1); // Add valid call to wait for.
player.awaitMethodCalled(MockPlayer.METHOD_ADD_MEDIA_ITEMS_WITH_INDEX, TIMEOUT_MS);
assertThat(player.index).isEqualTo(1);
}
@Test @Test
public void removeQueueItem() throws Exception { public void removeQueueItem() throws Exception {
session = session =
@ -617,6 +678,30 @@ public class MediaSessionCallbackWithMediaControllerCompatTest {
assertThat(player.seekMediaItemIndex).isEqualTo(targetIndex); assertThat(player.seekMediaItemIndex).isEqualTo(targetIndex);
} }
@Test
public void skipToQueueItem_withInvalidValue_doesNotCrashSession() throws Exception {
session =
new MediaSession.Builder(context, player)
.setId("skipToQueueItem_invalidValues")
.setCallback(new TestSessionCallback())
.build();
controller =
new RemoteMediaControllerCompat(
context, session.getSessionCompat().getSessionToken(), /* waitForConnection= */ true);
handler.postAndSync(
() -> {
player.timeline = MediaTestUtils.createTimeline(/* windowCount= */ 10);
player.notifyTimelineChanged(Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED);
});
controller.getTransportControls().skipToQueueItem(-1);
controller.getTransportControls().skipToQueueItem(1); // Add valid call to wait for.
player.awaitMethodCalled(
MockPlayer.METHOD_SEEK_TO_DEFAULT_POSITION_WITH_MEDIA_ITEM_INDEX, TIMEOUT_MS);
assertThat(player.seekMediaItemIndex).isEqualTo(1);
}
@Test @Test
public void dispatchMediaButtonEvent_playWithEmptyTimeline_callsPlaybackResumptionPrepareAndPlay() public void dispatchMediaButtonEvent_playWithEmptyTimeline_callsPlaybackResumptionPrepareAndPlay()
throws Exception { throws Exception {

View File

@ -15,6 +15,7 @@
*/ */
package androidx.media3.session; package androidx.media3.session;
import static androidx.media3.common.util.Assertions.checkArgument;
import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkNotNull;
import static java.lang.annotation.ElementType.TYPE_USE; import static java.lang.annotation.ElementType.TYPE_USE;
@ -431,6 +432,7 @@ public class MockPlayer implements Player {
@Override @Override
public void seekToDefaultPosition(int mediaItemIndex) { public void seekToDefaultPosition(int mediaItemIndex) {
checkArgument(mediaItemIndex >= 0);
seekMediaItemIndex = mediaItemIndex; seekMediaItemIndex = mediaItemIndex;
checkNotNull(conditionVariables.get(METHOD_SEEK_TO_DEFAULT_POSITION_WITH_MEDIA_ITEM_INDEX)) checkNotNull(conditionVariables.get(METHOD_SEEK_TO_DEFAULT_POSITION_WITH_MEDIA_ITEM_INDEX))
.open(); .open();
@ -444,6 +446,7 @@ public class MockPlayer implements Player {
@Override @Override
public void seekTo(int mediaItemIndex, long positionMs) { public void seekTo(int mediaItemIndex, long positionMs) {
checkArgument(mediaItemIndex >= 0);
seekMediaItemIndex = mediaItemIndex; seekMediaItemIndex = mediaItemIndex;
seekPositionMs = positionMs; seekPositionMs = positionMs;
checkNotNull(conditionVariables.get(METHOD_SEEK_TO_WITH_MEDIA_ITEM_INDEX)).open(); checkNotNull(conditionVariables.get(METHOD_SEEK_TO_WITH_MEDIA_ITEM_INDEX)).open();