diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java index 804e808b40..da84c6ccf8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java @@ -179,4 +179,19 @@ public class Media implements Parcelable { public int hashCode() { return uri.hashCode(); } + + public static @NonNull Media withMimeType(@NonNull Media media, @NonNull String newMimeType) { + return new Media(media.getUri(), + newMimeType, + media.getDate(), + media.getWidth(), + media.getHeight(), + media.getSize(), + media.getDuration(), + media.isBorderless(), + media.isVideoGif(), + media.getBucketId(), + media.getCaption(), + media.getTransformProperties()); + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaRepository.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaRepository.java index feccd3b481..bfdb999161 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaRepository.java @@ -14,6 +14,7 @@ import android.util.Pair; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; import com.annimon.stream.Stream; @@ -35,6 +36,7 @@ import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * Handles the retrieval of media present on the user's device. @@ -242,7 +244,7 @@ public class MediaRepository { long size = cursor.getLong(cursor.getColumnIndexOrThrow(Images.Media.SIZE)); long duration = !isImage ? cursor.getInt(cursor.getColumnIndexOrThrow(Video.Media.DURATION)) : 0; - media.add(new Media(uri, mimetype, date, width, height, size, duration, false, false, Optional.of(bucketId), Optional.absent(), Optional.absent())); + media.add(fixMimeType(context, new Media(uri, mimetype, date, width, height, size, duration, false, false, Optional.of(bucketId), Optional.absent(), Optional.absent()))); } } @@ -255,19 +257,22 @@ public class MediaRepository { @WorkerThread private List getPopulatedMedia(@NonNull Context context, @NonNull List media) { - return Stream.of(media).map(m -> { - try { - if (isPopulated(m)) { - return m; - } else if (PartAuthority.isLocalUri(m.getUri())) { - return getLocallyPopulatedMedia(context, m); - } else { - return getContentResolverPopulatedMedia(context, m); - } - } catch (IOException e) { - return m; - } - }).toList(); + return media.stream() + .map(m -> { + try { + if (isPopulated(m)) { + return m; + } else if (PartAuthority.isLocalUri(m.getUri())) { + return getLocallyPopulatedMedia(context, m); + } else { + return getContentResolverPopulatedMedia(context, m); + } + } catch (IOException e) { + return m; + } + }) + .map(m -> fixMimeType(context, m)) + .collect(Collectors.toList()); } @WorkerThread @@ -361,6 +366,25 @@ public class MediaRepository { return new Media(media.getUri(), media.getMimeType(), media.getDate(), width, height, size, 0, media.isBorderless(), media.isVideoGif(), media.getBucketId(), media.getCaption(), Optional.absent()); } + @VisibleForTesting + static @NonNull Media fixMimeType(@NonNull Context context, @NonNull Media media) { + if (MediaUtil.isOctetStream(media.getMimeType())) { + Log.w(TAG, "Media has mimetype octet stream"); + String newMimeType = MediaUtil.getMimeType(context, media.getUri()); + if (newMimeType != null && !newMimeType.equals(media.getMimeType())) { + Log.d(TAG, "Changing mime type to '" + newMimeType + "'"); + return Media.withMimeType(media, newMimeType); + } else if (media.getSize() > 0 && media.getWidth() > 0 && media.getHeight() > 0) { + boolean likelyVideo = media.getDuration() > 0; + Log.d(TAG, "Assuming content is " + (likelyVideo ? "a video" : "an image") + ", setting mimetype"); + return Media.withMimeType(media, likelyVideo ? MediaUtil.VIDEO_UNSPECIFIED : MediaUtil.IMAGE_JPEG); + } else { + Log.d(TAG, "Unable to fix mimetype"); + } + } + return media; + } + private static class FolderResult { private final String cameraBucketId; private final Uri thumbnail; diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java index 51a455518b..3198ce10a7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java @@ -818,10 +818,16 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med case ITEM_TOO_LARGE: Toast.makeText(this, R.string.MediaSendActivity_an_item_was_removed_because_it_exceeded_the_size_limit, Toast.LENGTH_LONG).show(); break; + case ITEM_TOO_LARGE_OR_INVALID_TYPE: + Toast.makeText(this, R.string.MediaSendActivity_an_item_was_removed_because_it_exceeded_the_size_limit_or_had_an_unknown_type, Toast.LENGTH_LONG).show(); + break; case ONLY_ITEM_TOO_LARGE: Toast.makeText(this, R.string.MediaSendActivity_an_item_was_removed_because_it_exceeded_the_size_limit, Toast.LENGTH_LONG).show(); onNoMediaAvailable(); break; + case ONLY_ITEM_IS_INVALID_TYPE: + Toast.makeText(this, R.string.MediaSendActivity_an_item_was_removed_because_it_had_an_unknown_type, Toast.LENGTH_LONG).show(); + onNoMediaAvailable(); case TOO_MANY_ITEMS: int maxSelection = viewModel.getMaxSelection(); Toast.makeText(this, getResources().getQuantityString(R.plurals.MediaSendActivity_cant_share_more_than_n_items, maxSelection, maxSelection), Toast.LENGTH_SHORT).show(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java index 2360e10536..ac85672c93 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendViewModel.java @@ -145,19 +145,23 @@ class MediaSendViewModel extends ViewModel { void onSelectedMediaChanged(@NonNull Context context, @NonNull List newMedia) { List originalMedia = getSelectedMediaOrDefault(); - if (!newMedia.isEmpty()) { - selectedMedia.setValue(newMedia); - } - repository.getPopulatedMedia(context, newMedia, populatedMedia -> { ThreadUtil.runOnMain(() -> { List filteredMedia = getFilteredMedia(context, populatedMedia, mediaConstraints); if (filteredMedia.size() != newMedia.size()) { if (filteredMedia.isEmpty() && newMedia.size() == 1 && page == Page.UNKNOWN) { - error.setValue(Error.ONLY_ITEM_TOO_LARGE); + if (MediaUtil.isImageOrVideoType(newMedia.get(0).getMimeType())) { + error.setValue(Error.ONLY_ITEM_TOO_LARGE); + } else { + error.setValue(Error.ONLY_ITEM_IS_INVALID_TYPE); + } } else { - error.setValue(Error.ITEM_TOO_LARGE); + if (newMedia.stream().allMatch(m -> MediaUtil.isImageOrVideoType(m.getMimeType()))) { + error.setValue(Error.ITEM_TOO_LARGE); + } else { + error.setValue(Error.ITEM_TOO_LARGE_OR_INVALID_TYPE); + } } } @@ -199,8 +203,6 @@ class MediaSendViewModel extends ViewModel { } void onSingleMediaSelected(@NonNull Context context, @NonNull Media media) { - selectedMedia.setValue(Collections.singletonList(media)); - repository.getPopulatedMedia(context, Collections.singletonList(media), populatedMedia -> { ThreadUtil.runOnMain(() -> { List filteredMedia = getFilteredMedia(context, populatedMedia, mediaConstraints); @@ -702,7 +704,7 @@ class MediaSendViewModel extends ViewModel { } enum Error { - ITEM_TOO_LARGE, TOO_MANY_ITEMS, NO_ITEMS, ONLY_ITEM_TOO_LARGE + ITEM_TOO_LARGE, TOO_MANY_ITEMS, NO_ITEMS, ONLY_ITEM_TOO_LARGE, ONLY_ITEM_IS_INVALID_TYPE, ITEM_TOO_LARGE_OR_INVALID_TYPE } enum Event { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/MediaUtil.java b/app/src/main/java/org/thoughtcrime/securesms/util/MediaUtil.java index f95f0b8e67..819bcd507f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/MediaUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/MediaUtil.java @@ -64,6 +64,7 @@ public class MediaUtil { public static final String LONG_TEXT = "text/x-signal-plain"; public static final String VIEW_ONCE = "application/x-signal-view-once"; public static final String UNKNOWN = "*/*"; + public static final String OCTET = "application/octet-stream"; public static SlideType getSlideTypeFromContentType(@NonNull String contentType) { if (isGif(contentType)) { @@ -111,7 +112,7 @@ public class MediaUtil { } String type = context.getContentResolver().getType(uri); - if (type == null) { + if (type == null || isOctetStream(type)) { final String extension = MimeTypeMap.getFileExtensionFromUrl(uri.toString()); type = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension.toLowerCase()); } @@ -325,6 +326,10 @@ public class MediaUtil { return (null != contentType) && contentType.equals(VIEW_ONCE); } + public static boolean isOctetStream(@Nullable String contentType) { + return OCTET.equals(contentType); + } + public static boolean hasVideoThumbnail(@NonNull Context context, @Nullable Uri uri) { if (uri == null) { return false; diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8265a5718a..62a1de1ce7 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1009,6 +1009,8 @@ Add a caption… An item was removed because it exceeded the size limit + An item was removed because it had an unknown type + An item was removed because it exceeded the size limit or had an unknown type Camera unavailable. Message to %s Message diff --git a/app/src/test/java/org/thoughtcrime/securesms/mediasend/MediaRepositoryTest.kt b/app/src/test/java/org/thoughtcrime/securesms/mediasend/MediaRepositoryTest.kt new file mode 100644 index 0000000000..4e6f358bca --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/mediasend/MediaRepositoryTest.kt @@ -0,0 +1,136 @@ +package org.thoughtcrime.securesms.mediasend + +import android.app.Application +import android.content.Context +import android.net.Uri +import androidx.test.core.app.ApplicationProvider +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers +import org.powermock.api.mockito.PowerMockito +import org.powermock.api.mockito.PowerMockito.mockStatic +import org.powermock.core.classloader.annotations.PowerMockIgnore +import org.powermock.core.classloader.annotations.PrepareForTest +import org.powermock.modules.junit4.rule.PowerMockRule +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.database.AttachmentDatabase.TransformProperties +import org.thoughtcrime.securesms.testutil.EmptyLogger +import org.thoughtcrime.securesms.util.MediaUtil +import org.whispersystems.libsignal.util.guava.Optional + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +@PowerMockIgnore("org.mockito.*", "org.robolectric.*", "android.*", "androidx.*") +@PrepareForTest(MediaUtil::class) +class MediaRepositoryTest { + + @Rule + @JvmField + val rule = PowerMockRule() + + private lateinit var context: Context + + @Before + fun setUp() { + Log.initialize(EmptyLogger()) + + context = ApplicationProvider.getApplicationContext() + mockStatic(MediaUtil::class.java) + PowerMockito.`when`(MediaUtil.isOctetStream(MediaUtil.OCTET)).thenReturn(true) + } + + @Test + fun `Given a valid mime type, do not change media`() { + // GIVEN + val media = buildMedia(mimeType = MediaUtil.IMAGE_JPEG) + + // WHEN + val result: Media = MediaRepository.fixMimeType(context, media) + + // THEN + assertEquals(media, result) + } + + @Test + fun `Given an invalid mime type, change media via MediaUtil`() { + // GIVEN + val media = buildMedia(mimeType = MediaUtil.OCTET) + + // WHEN + PowerMockito.`when`(MediaUtil.getMimeType(ArgumentMatchers.any(), ArgumentMatchers.any())).thenReturn(MediaUtil.IMAGE_JPEG) + val result: Media = MediaRepository.fixMimeType(context, media) + + // THEN + assertEquals(MediaUtil.IMAGE_JPEG, result.mimeType) + } + + @Test + fun `Given an invalid mime type with sizing info but no duration, guess image based`() { + // GIVEN + val media = buildMedia( + mimeType = MediaUtil.OCTET, + width = 100, + height = 100, + size = 100 + ) + + // WHEN + val result: Media = MediaRepository.fixMimeType(context, media) + + // THEN + assertEquals(MediaUtil.IMAGE_JPEG, result.mimeType) + } + + @Test + fun `Given an invalid mime type with sizing info and duration, guess video based`() { + // GIVEN + val media = buildMedia( + mimeType = MediaUtil.OCTET, + width = 100, + height = 100, + size = 100, + duration = 100 + ) + + // WHEN + val result: Media = MediaRepository.fixMimeType(context, media) + + // THEN + assertEquals(MediaUtil.VIDEO_UNSPECIFIED, result.mimeType) + } + + private fun buildMedia( + uri: Uri = Uri.EMPTY, + mimeType: String = "", + date: Long = 0, + width: Int = 0, + height: Int = 0, + size: Long = 0, + duration: Long = 0, + borderless: Boolean = false, + videoGif: Boolean = false, + bucketId: Optional = Optional.absent(), + caption: Optional = Optional.absent(), + transformProperties: Optional = Optional.absent() + ): Media { + return Media( + uri, + mimeType, + date, + width, + height, + size, + duration, + borderless, + videoGif, + bucketId, + caption, + transformProperties, + ) + } +}