From d22a2c0a50ad47f58b6e73723ca647b393bf4132 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Mon, 15 Jun 2026 12:28:34 -0400 Subject: [PATCH] Fix transfer control progress reporting bugs. --- .../transfercontrols/TransferControlView.kt | 32 ++++++++----------- .../transfercontrols/TransferControls.kt | 4 ++- .../transfercontrols/TransferControlsTest.kt | 20 +++++++----- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlView.kt b/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlView.kt index b43c60b08f..5144c7932c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlView.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlView.kt @@ -120,25 +120,13 @@ class TransferControlView @JvmOverloads constructor(context: Context, attrs: Att } if (event.type == PartProgressEvent.Type.COMPRESSION) { - val mutableMap = it.compressionProgress.toMutableMap() - val updateEvent = Progress.fromEvent(event) - val existingEvent = mutableMap[attachment] - if (existingEvent == null || updateEvent.completed > existingEvent.completed) { - mutableMap[attachment] = updateEvent - } else if (updateEvent.completed < 0.bytes) { - mutableMap.remove(attachment) - } - return@updateState it.copy(compressionProgress = mutableMap.toMap()) + val progress = it.compressionProgress.toMutableMap() + progress.applyProgress(attachment, Progress.fromEvent(event)) + return@updateState it.copy(compressionProgress = progress.toMap()) } else { - val mutableMap = it.networkProgress.toMutableMap() - val updateEvent = Progress.fromEvent(event) - val existingEvent = mutableMap[attachment] - if (existingEvent == null || updateEvent.completed > existingEvent.completed) { - mutableMap[attachment] = updateEvent - } else if (updateEvent.completed < 0.bytes) { - mutableMap.remove(attachment) - } - return@updateState it.copy(networkProgress = mutableMap.toMap()) + val progress = it.networkProgress.toMutableMap() + progress.applyProgress(attachment, Progress.fromEvent(event)) + return@updateState it.copy(networkProgress = progress.toMap()) } } } @@ -227,6 +215,14 @@ class TransferControlView @JvmOverloads constructor(context: Context, attrs: Att updateState { it.copy(isClickable = clickable) } } + private fun MutableMap.applyProgress(attachment: Attachment, update: Progress) { + if (update.completed < 0.bytes) { + remove(attachment) + } else { + put(attachment, update) + } + } + private inline fun verboseLog(message: () -> String) { if (VERBOSE_DEVELOPMENT_LOGGING) { Log.d(TAG, "[$viewId] ${message()}") diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControls.kt b/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControls.kt index 4a9bf46126..9d3e5818f2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControls.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControls.kt @@ -11,6 +11,8 @@ import org.thoughtcrime.securesms.attachments.Attachment import org.thoughtcrime.securesms.database.AttachmentTable import org.thoughtcrime.securesms.mms.Slide import org.thoughtcrime.securesms.util.MediaUtil +import org.whispersystems.signalservice.api.crypto.AttachmentCipherStreamUtil +import org.whispersystems.signalservice.internal.crypto.PaddingInputStream /** * Pure, Android-View-free logic for the transfer controls UI. @@ -152,7 +154,7 @@ object TransferControls { } else if (state.isUpload) { ProgressLabel.Bytes(state.networkProgress.sumCompleted(), state.networkProgress.sumTotal()) } else { - val total = state.slides.sumOf { it.fileSize }.bytes + val total = state.slides.sumOf { AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(it.fileSize)) }.bytes val completed = state.networkProgress.sumCompleted().let { if (it > total) total else it } ProgressLabel.Bytes(completed, total) } diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlsTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlsTest.kt index bdbdd28ccc..33b4d8b32c 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlsTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlsTest.kt @@ -21,6 +21,8 @@ import org.thoughtcrime.securesms.attachments.Attachment import org.thoughtcrime.securesms.database.AttachmentTable import org.thoughtcrime.securesms.mms.Slide import org.thoughtcrime.securesms.util.MediaUtil +import org.whispersystems.signalservice.api.crypto.AttachmentCipherStreamUtil +import org.whispersystems.signalservice.internal.crypto.PaddingInputStream class TransferControlsTest { @@ -187,22 +189,24 @@ class TransferControlsTest { } @Test - fun `download label uses fixed slide size as denominator, not network total`() { + fun `download label denominator is ciphertext size derived from slide, not network total`() { val slides = listOf(slide(AttachmentTable.TRANSFER_PROGRESS_STARTED, size = 1000)) - // Network total (2000) is intentionally larger than the slide's fixed file size (1000) to prove the denominator - // comes from the slide size, which does not ramp up mid-transfer. + // Completed is reported in ciphertext bytes, so the denominator must be the matching ciphertext length derived from the + // slide's plaintext size. The network event's total (2000) is intentionally different to prove it is not the source. val state = stateOf(slides, networkProgress = progressOf(slides, completed = 500, total = 2000)) val render = TransferControls.deriveRenderState(state) as TransferControlsRenderState.InProgress - assertEquals(TransferControls.ProgressLabel.Bytes(500L.bytes, 1000L.bytes), render.label) + val expectedTotal = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(1000)) + assertEquals(TransferControls.ProgressLabel.Bytes(500L.bytes, expectedTotal.bytes), render.label) } @Test - fun `download label clamps completed to total`() { + fun `download label clamps completed to ciphertext total`() { val slides = listOf(slide(AttachmentTable.TRANSFER_PROGRESS_STARTED, size = 1000)) - // Network bytes include encryption overhead, so completed can edge past the file size; it should clamp to total. - val state = stateOf(slides, networkProgress = progressOf(slides, completed = 1100, total = 1100)) + val expectedTotal = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(1000)) + // Incremental-MAC overhead means transmitted bytes can edge just past the computed ciphertext length; clamp to total. + val state = stateOf(slides, networkProgress = progressOf(slides, completed = expectedTotal + 100, total = expectedTotal + 100)) val render = TransferControls.deriveRenderState(state) as TransferControlsRenderState.InProgress - assertEquals(TransferControls.ProgressLabel.Bytes(1000L.bytes, 1000L.bytes), render.label) + assertEquals(TransferControls.ProgressLabel.Bytes(expectedTotal.bytes, expectedTotal.bytes), render.label) } @Test