mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-08-05 21:07:49 +01:00
Persist the share data watermark so a restored conversation can't replay a share.
This commit is contained in:
@@ -134,7 +134,6 @@ import org.thoughtcrime.securesms.components.voice.VoiceNoteMediaControllerOwner
|
||||
import org.thoughtcrime.securesms.conversation.ConversationIntents
|
||||
import org.thoughtcrime.securesms.conversation.NewConversationActivity
|
||||
import org.thoughtcrime.securesms.conversation.v2.MotionEventRelay
|
||||
import org.thoughtcrime.securesms.conversation.v2.ShareDataTimestampViewModel
|
||||
import org.thoughtcrime.securesms.conversationlist.ConversationListArchiveFragment
|
||||
import org.thoughtcrime.securesms.conversationlist.ConversationListFragment
|
||||
import org.thoughtcrime.securesms.conversationlist.RelinkDevicesReminderBottomSheetFragment
|
||||
@@ -257,7 +256,6 @@ class MainActivity :
|
||||
|
||||
private val toolbarViewModel: MainToolbarViewModel by viewModels()
|
||||
private val toolbarCallback = ToolbarCallback()
|
||||
private val shareDataTimestampViewModel: ShareDataTimestampViewModel by viewModels()
|
||||
|
||||
private val motionEventRelay: MotionEventRelay by viewModels()
|
||||
|
||||
@@ -382,8 +380,6 @@ class MainActivity :
|
||||
}
|
||||
}
|
||||
|
||||
shareDataTimestampViewModel.setTimestampFromActivityCreation(savedInstanceState, intent)
|
||||
|
||||
setContent {
|
||||
val mainToolbarState by toolbarViewModel.state.collectAsStateWithLifecycle()
|
||||
val megaphone by mainNavigationViewModel.megaphone.collectAsStateWithLifecycle()
|
||||
|
||||
+47
-29
@@ -33,6 +33,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.BodyRangeList
|
||||
import org.thoughtcrime.securesms.database.withAttachments
|
||||
import org.thoughtcrime.securesms.dependencies.AppDependencies
|
||||
import org.thoughtcrime.securesms.keyboard.KeyboardUtil
|
||||
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
||||
import org.thoughtcrime.securesms.mms.GifSlide
|
||||
import org.thoughtcrime.securesms.mms.ImageSlide
|
||||
import org.thoughtcrime.securesms.mms.PartAuthority
|
||||
@@ -60,8 +61,8 @@ class DraftRepository(
|
||||
val TAG = Log.tag(DraftRepository::class.java)
|
||||
}
|
||||
|
||||
fun getShareOrDraftData(lastShareDataTimestamp: Long): Maybe<Pair<ShareOrDraftData?, Drafts?>> {
|
||||
return MaybeCompat.fromCallable { getShareOrDraftDataInternal(lastShareDataTimestamp) }
|
||||
fun getShareOrDraftData(): Maybe<Pair<ShareOrDraftData?, Drafts?>> {
|
||||
return MaybeCompat.fromCallable { getShareOrDraftDataInternal() }
|
||||
.observeOn(Schedulers.io())
|
||||
}
|
||||
|
||||
@@ -72,16 +73,21 @@ class DraftRepository(
|
||||
* Note: Voice note drafts are handled differently and via the [DraftViewModel.state]
|
||||
*/
|
||||
@Suppress("ConvertTwoComparisonsToRangeCheck")
|
||||
private fun getShareOrDraftDataInternal(lastShareDataTimestamp: Long): Pair<ShareOrDraftData?, Drafts?>? {
|
||||
private fun getShareOrDraftDataInternal(): Pair<ShareOrDraftData?, Drafts?>? {
|
||||
val sharedDataTimestamp: Long = conversationArguments?.shareDataTimestamp ?: -1
|
||||
val lastShareDataTimestamp: Long = SignalStore.misc.lastProcessedShareDataTimestamp
|
||||
Log.d(TAG, "Shared this data at $sharedDataTimestamp and last processed share data at $lastShareDataTimestamp")
|
||||
if (sharedDataTimestamp > 0 && sharedDataTimestamp <= lastShareDataTimestamp) {
|
||||
Log.d(TAG, "Already processed this share data. Skipping.")
|
||||
return null
|
||||
Log.d(TAG, "Already processed this share data. Falling back to database drafts.")
|
||||
return loadDatabaseDraftData()
|
||||
} else {
|
||||
Log.d(TAG, "Have not processed this share data. Proceeding.")
|
||||
}
|
||||
|
||||
if (sharedDataTimestamp > 0) {
|
||||
SignalStore.misc.lastProcessedShareDataTimestamp = sharedDataTimestamp
|
||||
}
|
||||
|
||||
val shareText = conversationArguments?.draftText
|
||||
val shareMedia = conversationArguments?.draftMedia
|
||||
val shareContentType = conversationArguments?.draftContentType
|
||||
@@ -127,36 +133,48 @@ class DraftRepository(
|
||||
}
|
||||
|
||||
if (conversationArguments?.canInitializeFromDatabase() == true) {
|
||||
val (drafts, updatedText) = loadDraftsInternal(conversationArguments.threadId)
|
||||
|
||||
val draftText: CharSequence? = drafts.firstOrNull { it.type == DraftTable.Draft.TEXT }?.let { updatedText ?: it.value }
|
||||
|
||||
val messageEdit: ConversationMessage? = drafts.firstOrNull { it.type == DraftTable.Draft.MESSAGE_EDIT }?.let { loadDraftMessageEditInternal(it.value) }
|
||||
if (messageEdit != null) {
|
||||
return ShareOrDraftData.SetEditMessage(messageEdit, draftText, clearQuote = drafts.none { it.type == DraftTable.Draft.QUOTE }) to drafts
|
||||
}
|
||||
|
||||
val location: SignalPlace? = drafts.firstOrNull { it.type == DraftTable.Draft.LOCATION }?.let { SignalPlace.deserialize(it.value) }
|
||||
if (location != null) {
|
||||
return ShareOrDraftData.SetLocation(location, draftText) to drafts
|
||||
}
|
||||
|
||||
val quote: ConversationMessage? = drafts.firstOrNull { it.type == DraftTable.Draft.QUOTE }?.let { loadDraftQuoteInternal(it.value) }
|
||||
if (quote != null) {
|
||||
return ShareOrDraftData.SetQuote(quote, draftText) to drafts
|
||||
}
|
||||
|
||||
if (draftText != null) {
|
||||
return ShareOrDraftData.SetText(draftText) to drafts
|
||||
}
|
||||
|
||||
return null to drafts
|
||||
return loadDatabaseDraftData()
|
||||
}
|
||||
|
||||
// no share or draft
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Loads the drafts stored for this conversation, ignoring any share payload in [conversationArguments].
|
||||
*/
|
||||
private fun loadDatabaseDraftData(): Pair<ShareOrDraftData?, Drafts?>? {
|
||||
val threadId: Long = conversationArguments?.threadId ?: -1
|
||||
if (threadId <= 0) {
|
||||
return null
|
||||
}
|
||||
|
||||
val (drafts, updatedText) = loadDraftsInternal(threadId)
|
||||
|
||||
val draftText: CharSequence? = drafts.firstOrNull { it.type == DraftTable.Draft.TEXT }?.let { updatedText ?: it.value }
|
||||
|
||||
val messageEdit: ConversationMessage? = drafts.firstOrNull { it.type == DraftTable.Draft.MESSAGE_EDIT }?.let { loadDraftMessageEditInternal(it.value) }
|
||||
if (messageEdit != null) {
|
||||
return ShareOrDraftData.SetEditMessage(messageEdit, draftText, clearQuote = drafts.none { it.type == DraftTable.Draft.QUOTE }) to drafts
|
||||
}
|
||||
|
||||
val location: SignalPlace? = drafts.firstOrNull { it.type == DraftTable.Draft.LOCATION }?.let { SignalPlace.deserialize(it.value) }
|
||||
if (location != null) {
|
||||
return ShareOrDraftData.SetLocation(location, draftText) to drafts
|
||||
}
|
||||
|
||||
val quote: ConversationMessage? = drafts.firstOrNull { it.type == DraftTable.Draft.QUOTE }?.let { loadDraftQuoteInternal(it.value) }
|
||||
if (quote != null) {
|
||||
return ShareOrDraftData.SetQuote(quote, draftText) to drafts
|
||||
}
|
||||
|
||||
if (draftText != null) {
|
||||
return ShareOrDraftData.SetText(draftText) to drafts
|
||||
}
|
||||
|
||||
return null to drafts
|
||||
}
|
||||
|
||||
fun deleteVoiceNoteDraftData(draft: DraftTable.Draft?) {
|
||||
if (draft != null) {
|
||||
SignalExecutors.BOUNDED.execute {
|
||||
|
||||
@@ -132,8 +132,8 @@ class DraftViewModel @JvmOverloads constructor(
|
||||
return newState
|
||||
}
|
||||
|
||||
fun loadShareOrDraftData(lastShareDataTimestamp: Long): Maybe<DraftRepository.ShareOrDraftData> {
|
||||
return repository.getShareOrDraftData(lastShareDataTimestamp)
|
||||
fun loadShareOrDraftData(): Maybe<DraftRepository.ShareOrDraftData> {
|
||||
return repository.getShareOrDraftData()
|
||||
.doOnSuccess { (_, drafts) ->
|
||||
if (drafts != null) {
|
||||
store.update { saveDraftsIfChanged(it, it.copyAndSetDrafts(drafts = drafts)) }
|
||||
|
||||
@@ -51,7 +51,6 @@ open class ConversationActivity : PassphraseRequiredActivity(), VoiceNoteMediaCo
|
||||
override val googlePayResultPublisher: Subject<GooglePayComponent.GooglePayResult> = PublishSubject.create()
|
||||
|
||||
private val motionEventRelay: MotionEventRelay by viewModels()
|
||||
private val shareDataTimestampViewModel: ShareDataTimestampViewModel by viewModels()
|
||||
|
||||
override fun onPreCreate() {
|
||||
theme.onCreate(this)
|
||||
@@ -79,7 +78,6 @@ open class ConversationActivity : PassphraseRequiredActivity(), VoiceNoteMediaCo
|
||||
transitionDebouncer.publish { supportStartPostponedEnterTransition() }
|
||||
window.requestFeature(Window.FEATURE_ACTIVITY_TRANSITIONS)
|
||||
|
||||
shareDataTimestampViewModel.setTimestampFromActivityCreation(savedInstanceState, intent)
|
||||
setContentView(R.layout.fragment_container)
|
||||
|
||||
if (savedInstanceState == null) {
|
||||
|
||||
+1
-5
@@ -540,8 +540,6 @@ class ConversationFragment :
|
||||
InlineQueryViewModelV2(conversationRecipientRepository)
|
||||
}
|
||||
|
||||
private val shareDataTimestampViewModel: ShareDataTimestampViewModel by activityViewModels()
|
||||
|
||||
private val mainNavigationViewModel: MainNavigationViewModel by activityViewModels { MainNavigationViewModel.Factory() }
|
||||
|
||||
private val inlineQueryController: InlineQueryResultsControllerV2 by lazy {
|
||||
@@ -1450,7 +1448,7 @@ class ConversationFragment :
|
||||
.inputReadyState
|
||||
.take(1)
|
||||
.flatMapMaybe { inputReadyState ->
|
||||
draftViewModel.loadShareOrDraftData(shareDataTimestampViewModel.timestamp)
|
||||
draftViewModel.loadShareOrDraftData()
|
||||
.map { inputReadyState to it }
|
||||
}
|
||||
.subscribeBy { (inputReadyState, data) -> handleShareOrDraftData(inputReadyState, data) }
|
||||
@@ -2165,8 +2163,6 @@ class ConversationFragment :
|
||||
}
|
||||
|
||||
private fun handleShareOrDraftData(inputReadyState: InputReadyState, data: ShareOrDraftData) {
|
||||
shareDataTimestampViewModel.setTimestampFromConversationArgs(args)
|
||||
|
||||
if (inputReadyState.isAnnouncementGroup == true && inputReadyState.isAdmin == false) {
|
||||
Toast.makeText(requireContext(), R.string.MultiselectForwardFragment__only_admins_can_send_messages_to_this_group, Toast.LENGTH_SHORT).show()
|
||||
draftViewModel.clearDraft()
|
||||
|
||||
-39
@@ -1,39 +0,0 @@
|
||||
/*
|
||||
* Copyright 2023 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
package org.thoughtcrime.securesms.conversation.v2
|
||||
|
||||
import android.content.Intent
|
||||
import android.os.Bundle
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.ViewModel
|
||||
import org.thoughtcrime.securesms.conversation.ConversationArgs
|
||||
import org.thoughtcrime.securesms.util.delegate
|
||||
|
||||
/**
|
||||
* Hold the last share timestamp in an activity scoped view model for sharing between
|
||||
* the activity and fragments.
|
||||
*/
|
||||
class ShareDataTimestampViewModel(
|
||||
savedStateHandle: SavedStateHandle
|
||||
) : ViewModel() {
|
||||
|
||||
companion object {
|
||||
private const val TIMESTAMP = "timestamp"
|
||||
}
|
||||
|
||||
var timestamp: Long by savedStateHandle.delegate(TIMESTAMP, -1L)
|
||||
private set
|
||||
|
||||
fun setTimestampFromActivityCreation(savedInstanceState: Bundle?, intent: Intent) {
|
||||
if (savedInstanceState == null && intent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY != 0) {
|
||||
timestamp = System.currentTimeMillis()
|
||||
}
|
||||
}
|
||||
|
||||
fun setTimestampFromConversationArgs(args: ConversationArgs) {
|
||||
timestamp = args.shareDataTimestamp
|
||||
}
|
||||
}
|
||||
@@ -55,6 +55,7 @@ class MiscellaneousValues internal constructor(store: KeyValueStore) : SignalSto
|
||||
private const val LAST_SYNC_MESSAGE_SEEN_TIME_MS = "misc.last_sync_message_seen_time"
|
||||
private const val LAST_APPLIED_PNI_CHANGE_SERVER_TIMESTAMP = "misc.last_applied_pni_change_server_timestamp"
|
||||
private const val LAST_MISSING_PLAY_SERVICES_FCM_VERIFICATION_TIME = "misc.last_missing_play_services_fcm_verification_time"
|
||||
private const val LAST_PROCESSED_SHARE_DATA_TIMESTAMP = "misc.last_processed_share_data_timestamp"
|
||||
}
|
||||
|
||||
public override fun onFirstEverAppLaunch() {
|
||||
@@ -360,4 +361,14 @@ class MiscellaneousValues internal constructor(store: KeyValueStore) : SignalSto
|
||||
* The last time we tried to get an FCM token for a user reporting missing Play Services.
|
||||
*/
|
||||
var lastMissingPlayServicesFcmVerificationTime: Long by longValue(LAST_MISSING_PLAY_SERVICES_FCM_VERIFICATION_TIME, 0)
|
||||
|
||||
/**
|
||||
* High-water mark of the most recently consumed share payload, as set by
|
||||
* [org.thoughtcrime.securesms.conversation.ConversationArgs.shareDataTimestamp]. Any share whose timestamp is at or
|
||||
* below this has already been handed off to the conversation and must never be replayed.
|
||||
*
|
||||
* This has to outlive the navigation entry carrying the payload, which is persisted and can be restored long after
|
||||
* the activity that consumed it is gone.
|
||||
*/
|
||||
var lastProcessedShareDataTimestamp: Long by longValue(LAST_PROCESSED_SHARE_DATA_TIMESTAMP, -1)
|
||||
}
|
||||
|
||||
+136
@@ -0,0 +1,136 @@
|
||||
/*
|
||||
* Copyright 2026 Signal Messenger, LLC
|
||||
* SPDX-License-Identifier: AGPL-3.0-only
|
||||
*/
|
||||
|
||||
package org.thoughtcrime.securesms.conversation.drafts
|
||||
|
||||
import android.app.Application
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Before
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
import org.signal.core.util.logging.Log
|
||||
import org.thoughtcrime.securesms.conversation.ConversationArgs
|
||||
import org.thoughtcrime.securesms.conversation.ConversationIntents.ConversationScreenType
|
||||
import org.thoughtcrime.securesms.database.DraftTable
|
||||
import org.thoughtcrime.securesms.database.ThreadTable
|
||||
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
||||
import org.thoughtcrime.securesms.recipients.RecipientId
|
||||
import org.thoughtcrime.securesms.testutil.DirectExecutor
|
||||
import org.thoughtcrime.securesms.testutil.SignalStoreRule
|
||||
import org.thoughtcrime.securesms.testutil.SystemOutLogger
|
||||
|
||||
/**
|
||||
* Verifies that a share payload is handed to the conversation exactly once, using the durable marker in
|
||||
* [org.thoughtcrime.securesms.keyvalue.MiscellaneousValues.lastProcessedShareDataTimestamp].
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(manifest = Config.NONE, application = Application::class)
|
||||
class DraftRepositoryTest {
|
||||
|
||||
companion object {
|
||||
private const val THREAD_ID = 1L
|
||||
}
|
||||
|
||||
@get:Rule
|
||||
val signalStore = SignalStoreRule()
|
||||
|
||||
private lateinit var draftTable: DraftTable
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
Log.initialize(SystemOutLogger())
|
||||
|
||||
draftTable = mockk(relaxed = true)
|
||||
every { draftTable.getDrafts(any()) } returns DraftTable.Drafts()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `unprocessed share is delivered and advances the marker`() {
|
||||
val result = repository(shareDataTimestamp = 1000L, draftText = "shared text").load()
|
||||
|
||||
assertEquals(DraftRepository.ShareOrDraftData.SetText("shared text"), result?.first)
|
||||
assertEquals(1000L, SignalStore.misc.lastProcessedShareDataTimestamp)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `share is not delivered a second time`() {
|
||||
SignalStore.misc.lastProcessedShareDataTimestamp = 1000L
|
||||
|
||||
val result = repository(shareDataTimestamp = 1000L, draftText = "shared text").load()
|
||||
|
||||
assertNull(result?.first)
|
||||
assertEquals(1000L, SignalStore.misc.lastProcessedShareDataTimestamp)
|
||||
}
|
||||
|
||||
/** AND-9817: a restored navigation entry can carry a share that is older than the last one we consumed. */
|
||||
@Test
|
||||
fun `share older than the marker is not delivered`() {
|
||||
SignalStore.misc.lastProcessedShareDataTimestamp = 2000L
|
||||
|
||||
val result = repository(shareDataTimestamp = 1000L, draftText = "shared text").load()
|
||||
|
||||
assertNull(result?.first)
|
||||
assertEquals(2000L, SignalStore.misc.lastProcessedShareDataTimestamp)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `skipped share still loads database drafts`() {
|
||||
SignalStore.misc.lastProcessedShareDataTimestamp = 1000L
|
||||
every { draftTable.getDrafts(THREAD_ID) } returns DraftTable.Drafts(listOf(DraftTable.Draft(DraftTable.Draft.TEXT, "saved draft")))
|
||||
|
||||
val result = repository(shareDataTimestamp = 1000L, draftText = "shared text").load()
|
||||
|
||||
assertEquals(DraftRepository.ShareOrDraftData.SetText("saved draft"), result?.first)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `opening a conversation without share data does not reset the marker`() {
|
||||
SignalStore.misc.lastProcessedShareDataTimestamp = 2000L
|
||||
|
||||
repository(shareDataTimestamp = -1L, draftText = null).load()
|
||||
|
||||
assertEquals(2000L, SignalStore.misc.lastProcessedShareDataTimestamp)
|
||||
}
|
||||
|
||||
private fun DraftRepository.load(): Pair<DraftRepository.ShareOrDraftData?, DraftTable.Drafts?>? {
|
||||
return getShareOrDraftData().blockingGet()
|
||||
}
|
||||
|
||||
private fun repository(shareDataTimestamp: Long, draftText: String?): DraftRepository {
|
||||
return DraftRepository(
|
||||
context = mockk<Application>(relaxed = true),
|
||||
threadTable = mockk<ThreadTable>(relaxed = true),
|
||||
draftTable = draftTable,
|
||||
saveDraftsExecutor = DirectExecutor(),
|
||||
conversationArguments = conversationArgs(shareDataTimestamp, draftText)
|
||||
)
|
||||
}
|
||||
|
||||
private fun conversationArgs(shareDataTimestamp: Long, draftText: String?): ConversationArgs {
|
||||
return ConversationArgs(
|
||||
recipientId = RecipientId.from(1L),
|
||||
threadId = THREAD_ID,
|
||||
draftText = draftText,
|
||||
draftMedia = null,
|
||||
draftContentType = null,
|
||||
media = null,
|
||||
stickerLocator = null,
|
||||
isBorderless = false,
|
||||
distributionType = ThreadTable.DistributionTypes.DEFAULT,
|
||||
startingPosition = -1,
|
||||
isFirstTimeInSelfCreatedGroup = false,
|
||||
isWithSearchOpen = false,
|
||||
giftBadge = null,
|
||||
shareDataTimestamp = shareDataTimestamp,
|
||||
conversationScreenType = ConversationScreenType.NORMAL
|
||||
)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user