From ade7efea1aaa07d4821a132efb3891ae91fcbe43 Mon Sep 17 00:00:00 2001 From: wt Date: Fri, 18 Sep 2026 12:46:36 +0700 Subject: [PATCH] fix read state in self-chat and stop leaking read-marker into it (#18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-chat never showed Read: the outgoing message was promoted by sendDisplayedMarkerIfNeeded, but a later XEP-0184 receipt or a LMM correction overwrote .read with .delivered/.sent directly, bypassing Delivery.merged. Mark self-addressed outgoing messages read as soon as the server accepts them and merge every weaker status into the ladder instead of overwriting. The self-addressed read-marker carried a plaintext , so clients that do not know our namespace (Conversations, Monal) rendered it as a "Сообщение прочитано" bubble in the self-chat. Service stanzas now ride on their payload element only. Refs #18 --- Scripts/verify.sh | 8 +++++ Sources/Shared/Models/AppModel.swift | 46 +++++++++++++++++++++++---- Sources/Shared/XMPP/XMPPService.swift | 7 ++-- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/Scripts/verify.sh b/Scripts/verify.sh index 1376dbd..a058fc0 100755 --- a/Scripts/verify.sh +++ b/Scripts/verify.sh @@ -421,6 +421,14 @@ grep -q 'syncReadState' Sources/Shared/Models/AppModel.swift || { echo "Read receipts must be synced to the user's other devices" exit 1 } +grep -q 'delivery = \$0.delivery.merged(with: .delivered)' Sources/Shared/Models/AppModel.swift || { + echo "Delivery receipts must never downgrade an already read message" + exit 1 +} +if grep -q 'message.body = Self.readMarkerBody' Sources/Shared/XMPP/XMPPService.swift; then + echo "Read-marker service stanzas must not carry a plaintext body (it leaks into self-chat on other clients)" + exit 1 +fi grep -q 'markersPublisher' Sources/Shared/XMPP/XMPPService.swift || { echo "XEP-0333 displayed markers must mark messages as read" exit 1 diff --git a/Sources/Shared/Models/AppModel.swift b/Sources/Shared/Models/AppModel.swift index 50b1490..85cf284 100644 --- a/Sources/Shared/Models/AppModel.swift +++ b/Sources/Shared/Models/AppModel.swift @@ -1071,10 +1071,13 @@ final class AppModel: ObservableObject { isGroup: isGroup ) updateMessage(id: id) { - $0.delivery = .sent + $0.delivery = $0.delivery.merged(with: .sent) $0.encryptionFingerprint = fingerprint } let conversationID = peer.lowercased() + if conversationID == account.normalizedJID { + markMessageAsReadIfOutgoing(id: id) + } localTypingPauseTasks[conversationID]?.cancel() localTypingPauseTasks[conversationID] = nil localChatStateByConversation[conversationID] = .active @@ -1117,9 +1120,12 @@ final class AppModel: ObservableObject { isGroup: isGroup ) updateMessage(id: id) { - $0.delivery = .sent + $0.delivery = $0.delivery.merged(with: .sent) $0.encryptionFingerprint = fingerprint } + if peer.lowercased() == account.normalizedJID { + markMessageAsReadIfOutgoing(id: id) + } } catch { updateMessage(id: id) { $0.delivery = .failed } errorMessage = error.localizedDescription @@ -1161,9 +1167,12 @@ final class AppModel: ObservableObject { replacingMessageID: previous.clientID ) updateMessage(id: id) { - $0.delivery = .sent + $0.delivery = $0.delivery.merged(with: .sent) $0.encryptionFingerprint = fingerprint } + if previous.conversationID == account.normalizedJID { + markMessageAsReadIfOutgoing(id: id) + } } catch { correctionReceiptTargets.removeValue(forKey: correctionID) if let currentIndex = messages.firstIndex(where: { $0.clientID == id }), @@ -1511,10 +1520,13 @@ final class AppModel: ObservableObject { isGroup: isGroup ) updateMessage(id: id) { - $0.delivery = .sent + $0.delivery = $0.delivery.merged(with: .sent) $0.remoteAttachmentURL = result.remoteURL $0.encryptionFingerprint = result.fingerprint } + if peer.lowercased() == account.normalizedJID { + markMessageAsReadIfOutgoing(id: id) + } sentID = id } catch { if didInsertPending { @@ -1565,10 +1577,13 @@ final class AppModel: ObservableObject { isGroup: message.isGroupMessage ) updateMessage(id: message.clientID) { - $0.delivery = .sent + $0.delivery = $0.delivery.merged(with: .sent) $0.remoteAttachmentURL = result.remoteURL $0.encryptionFingerprint = result.fingerprint } + if message.conversationID == account?.normalizedJID { + markMessageAsReadIfOutgoing(id: message.clientID) + } } catch { updateMessage(id: message.clientID) { $0.delivery = .failed } errorMessage = error.localizedDescription @@ -2209,7 +2224,13 @@ final class AppModel: ObservableObject { Task { try? await avatarCache.store(data, for: normalized) } case .delivered(let messageID): let targetID = correctionReceiptTargets.removeValue(forKey: messageID) ?? messageID - updateMessage(id: targetID) { $0.delivery = .delivered } + // A `` receipt can arrive after the chat was already + // opened (or after an XEP-0333 marker marked it read). Merge with + // the monotonic ladder instead of overwriting, so `.read` is never + // downgraded back to `.delivered`. + updateMessage(id: targetID) { + $0.delivery = $0.delivery.merged(with: .delivered) + } case .read(let conversationID, let messageID): // Peer-originated read receipt (XEP-0333 displayed marker): apply // it locally and broadcast it to the user's other devices through @@ -2531,6 +2552,17 @@ final class AppModel: ObservableObject { } } + /// Self-chat has no peer that could answer with a `` marker, + /// so an outgoing message counts as read the moment the server accepted + /// it. `markRead` keeps `.failed` messages untouched and never re-syncs + /// the state to other devices (cameFromPeer == false). + private func markMessageAsReadIfOutgoing(id: String) { + guard let account, + let index = messageIndex(id: id, conversationID: account.normalizedJID) + else { return } + markRead(at: index, cameFromPeer: false) + } + /// Sends an XEP-0333 `` marker for the newest incoming 1:1 /// message once the chat is opened, so the peer's client can show it as /// read. MUC rooms never get markers (XEP-0333); deduped per message so @@ -2752,7 +2784,7 @@ final class AppModel: ObservableObject { messages[index].security = envelope.security messages[index].encryptionFingerprint = envelope.fingerprint if envelope.isOutgoing { - messages[index].delivery = .sent + messages[index].delivery = messages[index].delivery.merged(with: .sent) } updateConversationPreview(for: messages[index], incrementUnread: false) } diff --git a/Sources/Shared/XMPP/XMPPService.swift b/Sources/Shared/XMPP/XMPPService.swift index a3d850b..fe53fab 100644 --- a/Sources/Shared/XMPP/XMPPService.swift +++ b/Sources/Shared/XMPP/XMPPService.swift @@ -829,7 +829,10 @@ final class XMPPService { message.to = JID(account.normalizedJID) message.type = .chat message.id = marker.id - message.body = Self.readMarkerBody + // No : this is a service stanza for the user's own devices only. + // A plaintext body would leak into the self-chat and be rendered as a + // normal message by clients that do not know our read-marker namespace + // (Conversations, Monal), so the marker rides solely on its payload. addOriginID(marker.id, to: message) message.addChild(ReadStateSync.payloadElement(marker: marker)) client.context.writer.write(message, writeCompleted: nil) @@ -851,8 +854,6 @@ final class XMPPService { client.context.writer.write(message, writeCompleted: nil) } - private static let readMarkerBody = "Сообщение прочитано" - /// Detects OMEMO 2 (`urn:xmpp:omemo:2`) payloads, which the pinned /// MartinOMEMO version cannot decrypt (it only speaks the legacy /// `eu.siacs.conversations.axolotl` namespace).