Skip to content

fix(chat): guard localLastReadMessage against implausible message ids - #6486

Merged
mahibi merged 2 commits into
masterfrom
guardReadMessage
Aug 13, 2026
Merged

fix(chat): guard localLastReadMessage against implausible message ids#6486
mahibi merged 2 commits into
masterfrom
guardReadMessage

Conversation

@mahibi

@mahibi mahibi commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

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

This should ensure unrealistic high messageIds for lastReadMessage never make it to the server

馃弫 Checklist

  • 鉀戯笍 Tests (unit and/or integration) are included or not needed
  • 馃敄 Capability is checked or not needed
  • 馃敊 Backport requests are created or not needed: /backport to stable-xx.x
  • 馃搮 Milestone is set
  • 馃尭 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

馃 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@mahibi mahibi added this to the 25.0.0 milestone Aug 12, 2026
@mahibi
mahibi requested a review from rapterjet2004 August 12, 2026 14:05
@mahibi mahibi self-assigned this Aug 12, 2026
@mahibi
mahibi force-pushed the guardReadMessage branch 2 times, most recently from 532280b to 7e184f6 Compare August 12, 2026 14:09

@rapterjet2004 rapterjet2004 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It remains important to make sure that the conversation model is synchronized with the server. We don't want to end up missing notification messages because the conversation?.lastMessage is out of date.

@mahibi

mahibi commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

It remains important to make sure that the conversation model is synchronized with the server. We don't want to end up missing notification messages because the conversation?.lastMessage is out of date.

Sure. As far as i see an outdated lastMessage would not be a problem regarding the notifications. It's just about gating unrealistic high messageIds (that could be caused by bugs) from becoming the lastReadMessage on server

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 <dev@mhibbe.de>
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 <dev@mhibbe.de>
@github-actions

Copy link
Copy Markdown
Contributor

APK file: https://github.com/nextcloud/talk-android/actions/runs/31689094233/artifacts/9176906697
To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.
qrcode (please click on link to get QR code displayed)

@mahibi
mahibi merged commit 4abf39b into master Aug 13, 2026
16 of 19 checks passed
@mahibi
mahibi deleted the guardReadMessage branch August 13, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants