From e6f4b0976fd2f687bbd2e8e684c784e5c0cc8bdf Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Thu, 25 Feb 2021 12:33:39 -0400 Subject: [PATCH] Prevent double tap send on camera first flow. Defensive array list copies where used in builders and Intent#putParcelableArrayListExtra. Spelling. --- .../conversation/ConversationIntents.java | 21 +++++------ .../ui/managegroup/ManageGroupRepository.java | 2 +- .../ui/managegroup/ManageGroupViewModel.java | 3 +- .../mediasend/MediaSendActivity.java | 20 ++++------- .../securesms/sharing/MultiShareArgs.java | 35 ++++++++++--------- .../securesms/sharing/MultiShareSender.java | 2 +- .../securesms/sharing/ShareActivity.java | 8 ++--- .../securesms/sharing/ShareIntents.java | 9 ++--- .../securesms/sharing/ShareViewModel.java | 2 +- 9 files changed, 49 insertions(+), 53 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationIntents.java b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationIntents.java index ec0cace41f..1632649585 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationIntents.java +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/ConversationIntents.java @@ -16,6 +16,7 @@ import org.thoughtcrime.securesms.wallpaper.ChatWallpaper; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Objects; public class ConversationIntents { @@ -162,15 +163,15 @@ public class ConversationIntents { private final RecipientId recipientId; private final long threadId; - private String draftText; - private ArrayList media; - private StickerLocator stickerLocator; - private boolean isBorderless; - private int distributionType = ThreadDatabase.DistributionTypes.DEFAULT; - private int startingPosition = -1; - private Uri dataUri; - private String dataType; - private boolean firstTimeInSelfCreatedGroup; + private String draftText; + private List media; + private StickerLocator stickerLocator; + private boolean isBorderless; + private int distributionType = ThreadDatabase.DistributionTypes.DEFAULT; + private int startingPosition = -1; + private Uri dataUri; + private String dataType; + private boolean firstTimeInSelfCreatedGroup; private Builder(@NonNull Context context, @NonNull RecipientId recipientId, @@ -265,7 +266,7 @@ public class ConversationIntents { } if (media != null) { - intent.putParcelableArrayListExtra(EXTRA_MEDIA, media); + intent.putParcelableArrayListExtra(EXTRA_MEDIA, new ArrayList<>(media)); } if (stickerLocator != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupRepository.java b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupRepository.java index 6fd9a72103..d0fd5e03a5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupRepository.java @@ -217,7 +217,7 @@ final class ManageGroupRepository { return selectionLimits.getHardLimit() - members.size(); } - public @NonNull ArrayList getMembersWithoutSelf() { + public @NonNull List getMembersWithoutSelf() { ArrayList recipientIds = new ArrayList<>(members.size()); RecipientId selfId = Recipient.self().getId(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupViewModel.java index 9d581115f4..665ea85262 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/ui/managegroup/ManageGroupViewModel.java @@ -48,6 +48,7 @@ import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.livedata.LiveDataUtil; import org.thoughtcrime.securesms.util.views.SimpleProgressDialog; +import java.util.ArrayList; import java.util.List; public class ManageGroupViewModel extends ViewModel { @@ -332,7 +333,7 @@ public class ManageGroupViewModel extends ViewModel { intent.putExtra(AddMembersActivity.GROUP_ID, getGroupId().toString()); intent.putExtra(ContactSelectionListFragment.DISPLAY_MODE, ContactsCursorLoader.DisplayMode.FLAG_PUSH); intent.putExtra(ContactSelectionListFragment.SELECTION_LIMITS, new SelectionLimits(capacity.getSelectionWarning(), capacity.getSelectionLimit())); - intent.putParcelableArrayListExtra(ContactSelectionListFragment.CURRENT_SELECTION, capacity.getMembersWithoutSelf()); + intent.putParcelableArrayListExtra(ContactSelectionListFragment.CURRENT_SELECTION, new ArrayList<>(capacity.getMembersWithoutSelf())); fragment.startActivityForResult(intent, resultCode); } }); 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 6141dfe0cb..8875226054 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/MediaSendActivity.java @@ -569,22 +569,14 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med @Override public void onCameraContactsSendClicked(@NonNull List recipients) { - MediaSendFragment fragment = getMediaSendFragment(); - - if (fragment != null) { - fragment.pausePlayback(); - - SimpleProgressDialog.DismissibleDialog dialog = SimpleProgressDialog.showDelayed(this, 300, 0); - viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions()).observe(this, result -> { - dialog.dismiss(); - setActivityResultAndFinish(result); - }); - } else { - throw new AssertionError("No editor fragment available!"); - } + onSend(recipients); } private void onSendClicked() { + onSend(Collections.emptyList()); + } + + private void onSend(@NonNull List recipients) { MediaSendFragment fragment = getMediaSendFragment(); if (fragment == null) { @@ -600,7 +592,7 @@ public class MediaSendActivity extends PassphraseRequiredActivity implements Med fragment.pausePlayback(); SimpleProgressDialog.DismissibleDialog dialog = SimpleProgressDialog.showDelayed(this, 300, 0); - viewModel.onSendClicked(buildModelsToTransform(fragment), Collections.emptyList(), composeText.getMentions()) + viewModel.onSendClicked(buildModelsToTransform(fragment), recipients, composeText.getMentions()) .observe(this, result -> { dialog.dismiss(); setActivityResultAndFinish(result); diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareArgs.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareArgs.java index ef36fb1d4e..a98729188c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareArgs.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareArgs.java @@ -18,14 +18,14 @@ import org.thoughtcrime.securesms.util.MediaUtil; import java.io.IOException; import java.util.ArrayList; import java.util.HashSet; +import java.util.List; +import java.util.Objects; import java.util.Set; public final class MultiShareArgs implements Parcelable { - private static final String ARGS = "ShareInterstitialArgs"; - private final Set shareContactAndThreads; - private final ArrayList media; + private final List media; private final String draftText; private final StickerLocator stickerLocator; private final boolean borderless; @@ -36,7 +36,7 @@ public final class MultiShareArgs implements Parcelable { private MultiShareArgs(@NonNull Builder builder) { shareContactAndThreads = builder.shareContactAndThreads; - media = builder.media == null ? new ArrayList<>() : builder.media; + media = builder.media == null ? new ArrayList<>() : new ArrayList<>(builder.media); draftText = builder.draftText; stickerLocator = builder.stickerLocator; borderless = builder.borderless; @@ -47,7 +47,7 @@ public final class MultiShareArgs implements Parcelable { } protected MultiShareArgs(Parcel in) { - shareContactAndThreads = new HashSet<>(in.createTypedArrayList(ShareContactAndThread.CREATOR)); + shareContactAndThreads = new HashSet<>(Objects.requireNonNull(in.createTypedArrayList(ShareContactAndThread.CREATOR))); media = in.createTypedArrayList(Media.CREATOR); draftText = in.readString(); stickerLocator = in.readParcelable(StickerLocator.class.getClassLoader()); @@ -56,9 +56,10 @@ public final class MultiShareArgs implements Parcelable { dataType = in.readString(); viewOnce = in.readByte() != 0; + String linkedPreviewString = in.readString(); LinkPreview preview; try { - preview = LinkPreview.deserialize(in.readString()); + preview = linkedPreviewString != null ? LinkPreview.deserialize(linkedPreviewString) : null; } catch (IOException e) { preview = null; } @@ -70,7 +71,7 @@ public final class MultiShareArgs implements Parcelable { return shareContactAndThreads; } - public ArrayList getMedia() { + public @NonNull List getMedia() { return media; } @@ -176,21 +177,21 @@ public final class MultiShareArgs implements Parcelable { private final Set shareContactAndThreads; - private ArrayList media; - private String draftText; - private StickerLocator stickerLocator; - private boolean borderless; - private Uri dataUri; - private String dataType; - private LinkPreview linkPreview; - private boolean viewOnce; + private List media; + private String draftText; + private StickerLocator stickerLocator; + private boolean borderless; + private Uri dataUri; + private String dataType; + private LinkPreview linkPreview; + private boolean viewOnce; public Builder(@NonNull Set shareContactAndThreads) { this.shareContactAndThreads = shareContactAndThreads; } - public @NonNull Builder withMedia(@Nullable ArrayList media) { - this.media = media; + public @NonNull Builder withMedia(@Nullable List media) { + this.media = media != null ? new ArrayList<>(media) : null; return this; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java index ea4ffd0772..a74dfc13f7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java @@ -97,7 +97,7 @@ public final class MultiShareSender { return new MultiShareSendResultCollection(results); } - public static @NonNull TransportOption getWorseTransportOption(@NonNull Context context, @NonNull Set shareContactAndThreads) { + public static @NonNull TransportOption getWorstTransportOption(@NonNull Context context, @NonNull Set shareContactAndThreads) { for (ShareContactAndThread shareContactAndThread : shareContactAndThreads) { TransportOption option = resolveTransportOption(context, shareContactAndThread.isForceSms()); if (option.isSms()) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareActivity.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareActivity.java index 05e417e62a..b6fe18faa2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareActivity.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareActivity.java @@ -160,7 +160,7 @@ public class ShareActivity extends PassphraseRequiredActivity switch (requestCode) { case RESULT_MEDIA_CONFIRMATION: case RESULT_TEXT_CONFIRMATION: - viewModel.onSuccessulShare(); + viewModel.onSuccessfulShare(); finish(); break; default: @@ -504,7 +504,7 @@ public class ShareActivity extends PassphraseRequiredActivity Log.i(TAG, "Shared data was not external."); } - viewModel.onSuccessulShare(); + viewModel.onSuccessfulShare(); startActivity(builder.build()); } @@ -558,14 +558,14 @@ public class ShareActivity extends PassphraseRequiredActivity media, Stream.of(multiShareArgs.getShareContactAndThreads()).map(ShareContactAndThread::getRecipientId).toList(), multiShareArgs.getDraftText(), - MultiShareSender.getWorseTransportOption(this, multiShareArgs.getShareContactAndThreads())), + MultiShareSender.getWorstTransportOption(this, multiShareArgs.getShareContactAndThreads())), RESULT_MEDIA_CONFIRMATION); break; default: //noinspection CodeBlock2Expr MultiShareSender.send(multiShareArgs, results -> { MultiShareDialogs.displayResultDialog(this, results, () -> { - viewModel.onSuccessulShare(); + viewModel.onSuccessfulShare(); finish(); }); }); diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareIntents.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareIntents.java index d2e69e7e21..fec7aa4272 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareIntents.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareIntents.java @@ -12,6 +12,7 @@ import org.thoughtcrime.securesms.stickers.StickerLocator; import java.util.ArrayList; import java.util.Collection; +import java.util.List; public final class ShareIntents { @@ -68,9 +69,9 @@ public final class ShareIntents { private final Context context; - private String extraText; - private ArrayList extraMedia; - private Slide slide; + private String extraText; + private List extraMedia; + private Slide slide; public Builder(@NonNull Context context) { this.context = context; @@ -101,7 +102,7 @@ public final class ShareIntents { intent.putExtra(Intent.EXTRA_TEXT, extraText); if (extraMedia != null) { - intent.putParcelableArrayListExtra(EXTRA_MEDIA, extraMedia); + intent.putParcelableArrayListExtra(EXTRA_MEDIA, new ArrayList<>(extraMedia)); } else if (slide != null) { intent.putExtra(Intent.EXTRA_STREAM, slide.getUri()); intent.putExtra(EXTRA_BORDERLESS, slide.isBorderless()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareViewModel.java index aa8a814a75..d16ce92322 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/ShareViewModel.java @@ -103,7 +103,7 @@ public class ShareViewModel extends ViewModel { externalShare = false; } - public void onSuccessulShare() { + public void onSuccessfulShare() { mediaUsed = true; }