From 6af3f2ce428901bdb3bb18a52f6dbe14cd16c0da Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 21 Jul 2025 11:43:15 -0400 Subject: [PATCH] Do not upload long text attachments to the archive. --- .../securesms/database/AttachmentTableTest.kt | 42 +++++++++---------- .../securesms/backup/v2/BackupRepository.kt | 6 ++- .../securesms/database/AttachmentTable.kt | 32 +++++++------- .../securesms/jobs/AttachmentUploadJob.kt | 4 +- .../securesms/jobs/BackupMessagesJob.kt | 2 +- .../jobs/CopyAttachmentToArchiveJob.kt | 7 ++++ .../jobs/UploadAttachmentToArchiveJob.kt | 7 ++++ 7 files changed, 57 insertions(+), 43 deletions(-) diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentTableTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentTableTest.kt index f0d847ed09..ff63c75b5e 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentTableTest.kt +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentTableTest.kt @@ -44,7 +44,6 @@ import java.util.UUID import kotlin.random.Random import kotlin.time.Duration import kotlin.time.Duration.Companion.days -import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds @@ -286,25 +285,6 @@ class AttachmentTableTest { assertThat(attachments).isNotEmpty() } - @Test - fun givenAnAttachmentWithAMessageWithExpirationStartedThatExpiresIn5Minutes_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoNotExpectThatAttachment() { - // GIVEN - val uncompressData = byteArrayOf(1, 2, 3, 4, 5) - val blobUncompressed = BlobProvider.getInstance().forData(uncompressData).createForSingleSessionInMemory() - val attachment = createAttachment(1, blobUncompressed, AttachmentTable.TransformProperties.empty()) - val message = createIncomingMessage(serverTime = 0.days, attachment = attachment, expiresIn = 5.days) - val messageId = SignalDatabase.messages.insertMessageInbox(message).map { it.messageId }.get() - SignalDatabase.messages.markExpireStarted(messageId, startedTimestamp = System.currentTimeMillis() - (4.days + 12.hours).inWholeMilliseconds) - SignalDatabase.attachments.setArchiveTransferState(AttachmentId(1L), AttachmentTable.ArchiveTransferState.NONE) - SignalDatabase.attachments.setTransferState(messageId, AttachmentId(1L), AttachmentTable.TRANSFER_PROGRESS_DONE) - - // WHEN - val attachments = SignalDatabase.attachments.getAttachmentsThatNeedArchiveUpload() - - // THEN - assertThat(attachments).isEmpty() - } - @Test fun givenAnAttachmentWithAMessageWithExpirationStartedThatExpiresIn5Days_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoExpectThatAttachment() { // GIVEN @@ -324,6 +304,24 @@ class AttachmentTableTest { assertThat(attachments).isNotEmpty() } + @Test + fun givenAnAttachmentWithALongTextAttachment_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoNotExpectThatAttachment() { + // GIVEN + val uncompressData = byteArrayOf(1, 2, 3, 4, 5) + val blobUncompressed = BlobProvider.getInstance().forData(uncompressData).createForSingleSessionInMemory() + val attachment = createAttachment(1, blobUncompressed, AttachmentTable.TransformProperties.empty(), contentType = MediaUtil.LONG_TEXT) + val message = createIncomingMessage(serverTime = 0.days, attachment = attachment) + val messageId = SignalDatabase.messages.insertMessageInbox(message).map { it.messageId }.get() + SignalDatabase.attachments.setArchiveTransferState(AttachmentId(1L), AttachmentTable.ArchiveTransferState.NONE) + SignalDatabase.attachments.setTransferState(messageId, AttachmentId(1L), AttachmentTable.TRANSFER_PROGRESS_DONE) + + // WHEN + val attachments = SignalDatabase.attachments.getAttachmentsThatNeedArchiveUpload() + + // THEN + assertThat(attachments).isEmpty() + } + private fun createIncomingMessage( serverTime: Duration, attachment: Attachment, @@ -395,11 +393,11 @@ class AttachmentTableTest { ) } - private fun createAttachment(id: Long, uri: Uri, transformProperties: AttachmentTable.TransformProperties): UriAttachment { + private fun createAttachment(id: Long, uri: Uri, transformProperties: AttachmentTable.TransformProperties, contentType: String = MediaUtil.IMAGE_JPEG): UriAttachment { return UriAttachmentBuilder.build( id, uri = uri, - contentType = MediaUtil.IMAGE_JPEG, + contentType = contentType, transformProperties = transformProperties ) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt index 7115473906..0acc6216ce 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/BackupRepository.kt @@ -836,16 +836,20 @@ object BackupRepository { var frameCount = 0L writer.use { + val debugInfo = buildDebugInfo() + eventTimer.emit("debug-info") + writer.write( BackupInfo( version = VERSION, backupTimeMs = exportState.backupTime, mediaRootBackupKey = SignalStore.backup.mediaRootBackupKey.value.toByteString(), firstAppVersion = SignalStore.backup.firstAppVersion, - debugInfo = buildDebugInfo() + debugInfo = debugInfo ) ) frameCount++ + eventTimer.emit("header") // We're using a snapshot, so the transaction is more for perf than correctness dbSnapshot.rawWritableDatabase.withinTransaction { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt index cccf1b6bc7..d6546b4f7b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt @@ -630,12 +630,6 @@ class AttachmentTable( .readToSingleLong() } - private fun getMessageDoesNotExpireWithinTimeoutClause(): String { - val messageHasExpiration = "${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} > 0" - val messageExpiresInOneDayAfterViewing = "$messageHasExpiration AND ${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} < ${1.days.inWholeMilliseconds}" - return "NOT $messageExpiresInOneDayAfterViewing" - } - /** * Finds all of the attachmentIds of attachments that need to be uploaded to the archive cdn. */ @@ -643,11 +637,7 @@ class AttachmentTable( return readableDatabase .select("$TABLE_NAME.$ID") .from("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}") - .where( - "($ARCHIVE_TRANSFER_STATE = ? or $ARCHIVE_TRANSFER_STATE = ?) AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND ${getMessageDoesNotExpireWithinTimeoutClause()}", - ArchiveTransferState.NONE.value, - ArchiveTransferState.TEMPORARY_FAILURE.value - ) + .where(buildAttachmentsThatNeedUploadQuery()) .orderBy("$TABLE_NAME.$ID DESC") .run() .readToList { AttachmentId(it.requireLong(ID)) } @@ -691,14 +681,10 @@ class AttachmentTable( /** * Similar to [getAttachmentsThatNeedArchiveUpload], but returns if the list would be non-null in a more efficient way. */ - fun doAnyAttachmentsNeedArchiveUpload(): Boolean { + fun doAnyAttachmentsNeedArchiveUpload(currentTime: Long): Boolean { return readableDatabase .exists("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}") - .where( - "($ARCHIVE_TRANSFER_STATE = ? OR $ARCHIVE_TRANSFER_STATE = ?) AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND ${getMessageDoesNotExpireWithinTimeoutClause()}", - ArchiveTransferState.NONE.value, - ArchiveTransferState.TEMPORARY_FAILURE.value - ) + .where(buildAttachmentsThatNeedUploadQuery()) .run() } @@ -856,7 +842,7 @@ class AttachmentTable( $REMOTE_KEY NOT NULL AND $ARCHIVE_TRANSFER_STATE NOT IN (${ArchiveTransferState.FINISHED.value}, ${ArchiveTransferState.PERMANENT_FAILURE.value}) AND (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND - ${getMessageDoesNotExpireWithinTimeoutClause()} + (${MessageTable.EXPIRES_IN} = 0 OR ${MessageTable.EXPIRES_IN} > ${ChatItemArchiveExporter.EXPIRATION_CUTOFF.inWholeMilliseconds}) ) """.trimIndent() ) @@ -2547,6 +2533,16 @@ class AttachmentTable( } } + private fun buildAttachmentsThatNeedUploadQuery(): String { + return """ + $ARCHIVE_TRANSFER_STATE IN (${ArchiveTransferState.NONE.value}, ${ArchiveTransferState.TEMPORARY_FAILURE.value}) AND + $DATA_FILE NOT NULL AND + $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND + (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND + (${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} <= 0 OR ${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} > ${ChatItemArchiveExporter.EXPIRATION_CUTOFF.inWholeMilliseconds}) AND + $CONTENT_TYPE != '${MediaUtil.LONG_TEXT}' + """ + } private fun getAttachment(cursor: Cursor): DatabaseAttachment { val contentType = cursor.requireString(CONTENT_TYPE) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.kt index 496f6917fe..2e7fcfbde9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.kt @@ -198,10 +198,12 @@ class AttachmentUploadJob private constructor( messageId == AttachmentTable.PREUPLOAD_MESSAGE_ID -> { Log.i(TAG, "[$attachmentId] Avoid uploading preuploaded attachments to archive. Skipping.") } - SignalDatabase.messages.willMessageExpireBeforeCutoff(messageId) -> { Log.i(TAG, "[$attachmentId] Message will expire within 24hrs. Skipping.") } + databaseAttachment.contentType == MediaUtil.LONG_TEXT -> { + Log.i(TAG, "[$attachmentId] Long text attachment. Skipping.") + } else -> { Log.i(TAG, "[$attachmentId] Enqueuing job to copy to archive.") AppDependencies.jobManager.add(CopyAttachmentToArchiveJob(attachmentId)) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt index 75b1ec1376..ec82efbe76 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/BackupMessagesJob.kt @@ -240,7 +240,7 @@ class BackupMessagesJob private constructor( return Result.failure() } - if (SignalStore.backup.backsUpMedia && SignalDatabase.attachments.doAnyAttachmentsNeedArchiveUpload()) { + if (SignalStore.backup.backsUpMedia && SignalDatabase.attachments.doAnyAttachmentsNeedArchiveUpload(System.currentTimeMillis())) { Log.i(TAG, "Enqueuing attachment backfill job.") AppDependencies.jobManager.add(ArchiveAttachmentBackfillJob()) } else { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt index 075992a421..acaa7549d8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/CopyAttachmentToArchiveJob.kt @@ -19,6 +19,7 @@ import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint import org.thoughtcrime.securesms.jobs.protos.CopyAttachmentToArchiveJobData import org.thoughtcrime.securesms.keyvalue.SignalStore +import org.thoughtcrime.securesms.util.MediaUtil import org.whispersystems.signalservice.api.NetworkResult import java.util.concurrent.TimeUnit @@ -102,6 +103,12 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A return Result.success() } + if (attachment.contentType == MediaUtil.LONG_TEXT) { + Log.i(TAG, "[$attachmentId] Attachment is long text. Resetting transfer state to none and skipping.") + SignalDatabase.attachments.setArchiveTransferState(attachmentId, AttachmentTable.ArchiveTransferState.NONE) + return Result.success() + } + if (isCanceled) { Log.w(TAG, "[$attachmentId] Canceled. Refusing to proceed.") return Result.failure() diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt index 3b0b1b4bb0..f059c2bd1b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/UploadAttachmentToArchiveJob.kt @@ -26,6 +26,7 @@ import org.thoughtcrime.securesms.jobs.protos.UploadAttachmentToArchiveJobData import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.net.SignalNetwork import org.thoughtcrime.securesms.service.AttachmentProgressService +import org.thoughtcrime.securesms.util.MediaUtil import org.thoughtcrime.securesms.util.Util import org.whispersystems.signalservice.api.NetworkResult import org.whispersystems.signalservice.api.archive.ArchiveMediaUploadFormStatusCodes @@ -137,6 +138,12 @@ class UploadAttachmentToArchiveJob private constructor( return Result.success() } + if (attachment.contentType == MediaUtil.LONG_TEXT) { + Log.i(TAG, "[$attachmentId] Attachment is long text. Resetting transfer state to none and skipping.") + SignalDatabase.attachments.setArchiveTransferState(attachmentId, AttachmentTable.ArchiveTransferState.NONE) + return Result.success() + } + if (attachment.remoteKey == null) { Log.w(TAG, "[$attachmentId] Attachment is missing remote key! Cannot upload.") return Result.failure()