From f068094c6541133c4a79c92c6fb08f2acc1e2299 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 12 Aug 2026 15:57:19 +0200 Subject: [PATCH 1/2] fix(chat): guard localLastReadMessage against implausible message ids Prevents unplausible high message ids from being sent to the server as the read marker, which was marking conversations read far beyond their real last message and causing new chat notifications to be immediately deleted. Assisted-by: Claude Code 2.1.199:claude-sonnet-5 Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 17 ++++++ .../talk/chat/viewmodels/ChatViewModelTest.kt | 54 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index d0dc70fc1c..2a6c598a87 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -1796,6 +1796,18 @@ class ChatViewModel @AssistedInject constructor( fun advanceLocalLastReadMessageIfNeeded(messageId: Int) { Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, messageId: $messageId") Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, localLastReadMessage: $localLastReadMessage") + + val newestKnownRealMessageId = _uiState.value.conversation?.lastMessage?.id + + if (!isPlausibleLastReadMessageId(messageId, newestKnownRealMessageId)) { + Log.w( + TAG, + "advanceLocalLastReadMessageIfNeeded, messageId ($messageId) is implausibly higher than the " + + "conversation's newest known message id ($newestKnownRealMessageId). We won't advance." + ) + return + } + if (localLastReadMessage < messageId && -1 < messageId) { Log.d(TAG, "advanceLocalLastReadMessageIfNeeded, setting localLastReadMessage to $messageId") localLastReadMessage = messageId @@ -2406,6 +2418,11 @@ class ChatViewModel @AssistedInject constructor( private const val LOAD_MORE_MESSAGES_LIMIT = 100 private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4 private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L + + private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 2_000L + + fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean = + newestKnownRealMessageId == null || messageId <= newestKnownRealMessageId + PLAUSIBLE_MESSAGE_ID_BUFFER } sealed class OutOfOfficeUIState { diff --git a/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt b/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt new file mode 100644 index 0000000000..6ca8d511d7 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt @@ -0,0 +1,54 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.chat.viewmodels + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatViewModelTest { + + @Test + fun `isPlausibleLastReadMessageId returns true when there is no known real message id yet`() { + assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761, newestKnownRealMessageId = null)) + } + + @Test + fun `isPlausibleLastReadMessageId returns true for the newest known real message id itself`() { + assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761, newestKnownRealMessageId = 158761L)) + } + + @Test + fun `isPlausibleLastReadMessageId returns true for an older message id`() { + assertTrue(ChatViewModel.isPlausibleLastReadMessageId(messageId = 100, newestKnownRealMessageId = 158761L)) + } + + @Test + fun `isPlausibleLastReadMessageId returns true within the buffer above the newest known message id`() { + assertTrue( + ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761 + 2000, newestKnownRealMessageId = 158761L) + ) + } + + @Test + fun `isPlausibleLastReadMessageId returns false just beyond the buffer above the newest known message id`() { + assertFalse( + ChatViewModel.isPlausibleLastReadMessageId( + messageId = 158761 + 2000 + 1, + newestKnownRealMessageId = 158761L + ) + ) + } + + @Test + fun `isPlausibleLastReadMessageId rejects a hash-derived placeholder id far beyond the real message id space`() { + assertFalse( + ChatViewModel.isPlausibleLastReadMessageId(messageId = 1_963_726_147, newestKnownRealMessageId = 158761L) + ) + } +} From d747ce395e5fee11260324ca429e2d4b0e36b25d Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 13 Aug 2026 11:59:11 +0200 Subject: [PATCH 2/2] increase PLAUSIBLE_MESSAGE_ID_BUFFER to 10000 + add logger I increased PLAUSIBLE_MESSAGE_ID_BUFFER to 10000 just to be super sure that the gate does not block when lastReadMessage is valid. Imagine a very big instance with a lot of activity: All chats bump the messageId as it's not tied to conversations. I think 10000 must be high enough as buffer and it still guards most weird unrealistic id's. Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index 2a6c598a87..c71dc15d05 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -1800,7 +1800,7 @@ class ChatViewModel @AssistedInject constructor( val newestKnownRealMessageId = _uiState.value.conversation?.lastMessage?.id if (!isPlausibleLastReadMessageId(messageId, newestKnownRealMessageId)) { - Log.w( + logger.w( TAG, "advanceLocalLastReadMessageIfNeeded, messageId ($messageId) is implausibly higher than the " + "conversation's newest known message id ($newestKnownRealMessageId). We won't advance." @@ -2402,7 +2402,7 @@ class ChatViewModel @AssistedInject constructor( } companion object { - private val TAG = ChatViewModel::class.simpleName + private val TAG = ChatViewModel::class.java.simpleName const val JOIN_ROOM_RETRY_COUNT: Long = 3 const val HTTP_CODE_OK: Int = 200 private const val CONVERSATION_AND_USER_FLOW_SHARING_TIMEOUT_MS = 5_000L @@ -2419,7 +2419,7 @@ class ChatViewModel @AssistedInject constructor( private const val POST_UPLOAD_FETCH_MAX_ATTEMPTS = 4 private const val POST_UPLOAD_FETCH_RETRY_DELAY_MS = 1_500L - private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 2_000L + private const val PLAUSIBLE_MESSAGE_ID_BUFFER = 10_000L fun isPlausibleLastReadMessageId(messageId: Int, newestKnownRealMessageId: Long?): Boolean = newestKnownRealMessageId == null || messageId <= newestKnownRealMessageId + PLAUSIBLE_MESSAGE_ID_BUFFER