Inconsistent messages order (in group chats at least) as Messages inserted into chat based on receiver's clock and not on server's clock or sender's message order

beat commented 2 years ago

Bug description

Received messages are inserted into the list by receiver's android clock, and not at end or by the server's timeclock, respectively by the message timestamp.

This leads to group chats having different chat histories depending on the clocks of each device of group members.

It also leads to messages sent while being offline being missed in group conversations. Maybe in individual conversations too.

Steps to reproduce

Create a group chat with users and devices A, B and C, clocks synchronized fine by NTP (supposing that time is xx:00 at start in description below).
Send messages with enough lines to fill half devices chat screen: message "1" from A at :00, "2" from B at :02 and "3" from C at :03, then you can send any more number of messages so that 1,2,3,4 are way up in the chat.
Change clock of device A back in time to :00
Send message "4" from B into chat when clock of A reaches :01
Check result according to Actual/Expected result below
Move clock of device A back to real time (NTP synced), check it's real again.
Check results (they are unchanged)

Actual result: On device A, as messages 2 and 3 take full screen, message 4 is invisible and gets unnoticed. Scrolling back shows message 4 inserted quietly and unnoticeably between 1 and 2! The order of messages is incorrectly 1,4,2,3 (and 4 is not marked "unread" nor scrolled back to display new message. On device B and C, order is correctly 1,2,3,4. Clicking "i" on message 4 on device A shows correct sending and receiving times (as timestamped by local device's clock). Message Expected result: Order of messages should be same on all devices: 1,2,3,4. And message 4 should in all caes be marked unread.

Device info

Device: Samsung Galaxy S7 Android version: 8.0.0 Signal version: 5.25.8

This could be related with issue https://github.com/signalapp/Signal-Android/issues/11494 or explain it too. Not sure, so reporting separately.

P.S. I'm reporting it here, notifying @cody-signal as this bug allows to manipulate sequences within a (group) chat, it could potententially be a legal/security issue if chats screendumps are used in legal cases.

LeadManPL commented 2 years ago

I can confirm. And here is visual proof: Signal_error_screen

stale[bot] commented 2 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

beat commented 2 years ago

This is still very relevant and annoying, please keep open to be addressed!

greyson-signal commented 2 years ago

This is something we eventually want to address. We'd like to make it so messages are always rendered in received order (which is different from sorting by received time, as you note). Unfortunately, our current message DB schema makes this prohibitively difficult.

beat commented 2 years ago

Thank you for your reply @greyson-signal !

Maybe a different solution would work too: How about making them ordered by server-received time ?

I have no idea on what your database structure is, but it would basically just be a timestamp that should anyway exist as a database column ?

And that timestamp could be transmitted in the message transmitted to the clients too, including to the sender. That way all clients have same order, and additionally it would allow to scroll back to last seen message consistently ?

I sure must be missing something as I'm sure you had such a solution in mind, but other things are preventing it from being implemented ;-)

greyson-signal commented 2 years ago

Server timestamp is a possibility, but there's two kinds of server timestamp -- the time on the server when a message was sent, and the time it was received/downloaded. Using either is funny when you consider that you have to account for "outgoing messages that have not reached to server yet" into consideration. There's ways to handle it, it's just not as straightforward as you would think.

The "proper" way to do this would be to to sort by database primary key, but the reason our schema makes that hard is that our messages are split between two tables :/ (sms and mms, from the old textsecure days). Anyway, we'll think about it some more and figure something out. But I don't anticipate it will be in the imminent future. More like in the new few months.

beat commented 2 years ago

imho, messages that "have not reached to server yet" should stay ordered in sending at the bottom of the chat of the sending client.

It's indeed a special sorting case for the client, but nothing special for the server. In all cases, what's important is to disregard the time of the clients in the message sorting.

It sure is not a vital issue (in most cases), just annoying, so fully understand that it's not a top priority. Just hope that you find a simple way to fix it.

ZwergNaseXXL commented 2 years ago

Didn't read the whole thread, but I get the impression you're thinking to complicated here. I'm pretty sure that in our group chat we all have the same clock as we live close to each other, still conversations with multiple minutes or even hours between the messages arrive completely garbled. Can't even be sure on which day they were sent.

be a member of a group
have 2 or 3 other members have a conversation while your offline
go online, start signal
messages arrive in random order, conversation is completely garbled
closing and restarting signal doesn't help, messages stay in random order

This is particular fun when you missed a couple of days. There's no way of making sense of the conversation. Displaying messages in the order they were sent seems such a basic functionality...

JanLukasGithub commented 2 years ago

There's another issue potentially caused by this: When A writes a message to the group and B replies to it, it can happen that C receives the reply before it receives the replied-to message, resulting in a "Original Message not found" when clicking on the reply part of the message.

I don't know how Signal handles replies, so this might be part of this issue or a partially different one. In that case I can open another issue (no issue regarding this exists).

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

LeadManPL commented 2 years ago

Leaving comment to avoid closing - this issue is still not resolved

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

beat commented 2 years ago

still not resolved, stale bot. Leave open.

signalapp / Signal-Android