From 5b4c18346674dd2aec39a9e011db6ea36c424cc6 Mon Sep 17 00:00:00 2001 From: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com> Date: Sun, 16 Apr 2023 19:30:25 +0200 Subject: [PATCH] core: do not create decryption error chat items for earlier messages (#2189) * core: do not create decryption error chat items for earlier messages * do not report earlier error, mobile items, fix tests --- .../main/java/chat/simplex/app/model/ChatModel.kt | 12 +++++------- .../main/java/chat/simplex/app/model/SimpleXAPI.kt | 10 ++++++++-- .../app/views/chat/item/CIRcvDecryptionError.kt | 2 -- apps/android/app/src/main/res/values/strings.xml | 2 -- .../Views/Chat/ChatItem/CIRcvDecryptionError.swift | 2 -- apps/ios/SimpleXChat/API.swift | 13 ++++++++++++- apps/ios/SimpleXChat/ChatTypes.swift | 8 +++----- src/Simplex/Chat.hs | 4 ++-- src/Simplex/Chat/Messages.hs | 6 +----- tests/ChatTests/Groups.hs | 1 - 10 files changed, 31 insertions(+), 29 deletions(-) diff --git a/apps/android/app/src/main/java/chat/simplex/app/model/ChatModel.kt b/apps/android/app/src/main/java/chat/simplex/app/model/ChatModel.kt index 285be3e47..8080f52f7 100644 --- a/apps/android/app/src/main/java/chat/simplex/app/model/ChatModel.kt +++ b/apps/android/app/src/main/java/chat/simplex/app/model/ChatModel.kt @@ -1466,10 +1466,10 @@ data class ChatItem ( file = null ) - fun invalidJSON(json: String): ChatItem = + fun invalidJSON(chatDir: CIDirection?, meta: CIMeta?, json: String): ChatItem = ChatItem( - chatDir = CIDirection.DirectSnd(), - meta = CIMeta.invalidJSON(), + chatDir = chatDir ?: CIDirection.DirectSnd(), + meta = meta ?: CIMeta.invalidJSON(), content = CIContent.InvalidJSON(json), quotedItem = null, file = null @@ -1681,13 +1681,11 @@ sealed class CIContent: ItemContent { @Serializable enum class MsgDecryptError { @SerialName("ratchetHeader") RatchetHeader, - @SerialName("earlier") Earlier, @SerialName("tooManySkipped") TooManySkipped; val text: String get() = when (this) { - RatchetHeader -> generalGetString(R.string.decryption_error_permanent) - Earlier -> generalGetString(R.string.decryption_error) - TooManySkipped -> generalGetString(R.string.decryption_error_permanent) + RatchetHeader -> generalGetString(R.string.decryption_error) + TooManySkipped -> generalGetString(R.string.decryption_error) } } diff --git a/apps/android/app/src/main/java/chat/simplex/app/model/SimpleXAPI.kt b/apps/android/app/src/main/java/chat/simplex/app/model/SimpleXAPI.kt index 7f295029d..ab65ce96f 100644 --- a/apps/android/app/src/main/java/chat/simplex/app/model/SimpleXAPI.kt +++ b/apps/android/app/src/main/java/chat/simplex/app/model/SimpleXAPI.kt @@ -3011,9 +3011,15 @@ private fun parseChatData(chat: JsonElement): Chat { ?: ChatInfo.InvalidJSON(json.encodeToString(chat.jsonObject["chatInfo"])) val chatStats = decodeObject(Chat.ChatStats.serializer(), chat.jsonObject["chatStats"])!! val chatItems: List = chat.jsonObject["chatItems"]!!.jsonArray.map { - decodeObject(ChatItem.serializer(), it) ?: ChatItem.invalidJSON(json.encodeToString(it)) + decodeObject(ChatItem.serializer(), it) ?: parseChatItem(it) } - return Chat(chatInfo, chatItems, chatStats) + return Chat(chatInfo, chatItems, chatStats) +} + +private fun parseChatItem(j: JsonElement): ChatItem { + val chatDir: CIDirection? = decodeObject(CIDirection.serializer(), j.jsonObject["chatDir"]) + val meta: CIMeta? = decodeObject(CIMeta.serializer(), j.jsonObject["meta"]) + return ChatItem.invalidJSON(chatDir, meta, json.encodeToString(j)) } private fun decodeObject(deserializer: DeserializationStrategy, obj: JsonElement?): T? = diff --git a/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/CIRcvDecryptionError.kt b/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/CIRcvDecryptionError.kt index 22e1e98f9..c33544f2b 100644 --- a/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/CIRcvDecryptionError.kt +++ b/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/CIRcvDecryptionError.kt @@ -15,8 +15,6 @@ fun CIRcvDecryptionError(msgDecryptError: MsgDecryptError, msgCount: UInt, ci: C MsgDecryptError.RatchetHeader -> String.format(generalGetString(R.string.alert_text_decryption_error_header), msgCount.toLong()) + "\n" + generalGetString(R.string.alert_text_fragment_encryption_out_of_sync_old_database) + "\n" + generalGetString(R.string.alert_text_fragment_permanent_error_reconnect) - MsgDecryptError.Earlier -> String.format(generalGetString(R.string.alert_text_decryption_error_earlier), msgCount.toLong()) + "\n" + - generalGetString(R.string.alert_text_fragment_encryption_out_of_sync_old_database) MsgDecryptError.TooManySkipped -> String.format(generalGetString(R.string.alert_text_decryption_error_too_many_skipped), msgCount.toLong()) + "\n" + generalGetString(R.string.alert_text_fragment_encryption_out_of_sync_old_database) + "\n" + generalGetString(R.string.alert_text_fragment_permanent_error_reconnect) diff --git a/apps/android/app/src/main/res/values/strings.xml b/apps/android/app/src/main/res/values/strings.xml index cd955e746..609cb8a83 100644 --- a/apps/android/app/src/main/res/values/strings.xml +++ b/apps/android/app/src/main/res/values/strings.xml @@ -32,7 +32,6 @@ moderated invalid chat invalid data - Permanent decryption error Decryption error @@ -749,7 +748,6 @@ Bad message ID The ID of the next message is incorrect (less or equal to the previous).\nIt can happen because of some bug or when the connection is compromised. %1$d messages failed to decrypt. - %1$d messages failed to decrypt and won\'t be shown. %1$d messages skipped. It can happen when you or your connection used the old database backup. This error is permanent for this connection, please re-connect. diff --git a/apps/ios/Shared/Views/Chat/ChatItem/CIRcvDecryptionError.swift b/apps/ios/Shared/Views/Chat/ChatItem/CIRcvDecryptionError.swift index 849196cd4..928773023 100644 --- a/apps/ios/Shared/Views/Chat/ChatItem/CIRcvDecryptionError.swift +++ b/apps/ios/Shared/Views/Chat/ChatItem/CIRcvDecryptionError.swift @@ -25,8 +25,6 @@ struct CIRcvDecryptionError: View { switch msgDecryptError { case .ratchetHeader: message = Text("\(msgCount) messages failed to decrypt.") + Text("\n") + why + Text("\n") + permanent - case .earlier: - message = Text("\(msgCount) messages failed to decrypt and won't be shown.") + Text("\n") + why case .tooManySkipped: message = Text("\(msgCount) messages skipped.") + Text("\n") + why + Text("\n") + permanent } diff --git a/apps/ios/SimpleXChat/API.swift b/apps/ios/SimpleXChat/API.swift index c0f4e6148..a26d7bb41 100644 --- a/apps/ios/SimpleXChat/API.swift +++ b/apps/ios/SimpleXChat/API.swift @@ -174,7 +174,11 @@ func parseChatData(_ jChat: Any) throws -> ChatData { if let ci: ChatItem = try? decodeObject(jCI) { return ci } - return ChatItem.invalidJSON(prettyJSON(jCI) ?? "") + return ChatItem.invalidJSON( + chatDir: decodeProperty(jCI, "chatDir"), + meta: decodeProperty(jCI, "meta"), + json: prettyJSON(jCI) ?? "" + ) } return ChatData(chatInfo: chatInfo, chatItems: chatItems, chatStats: chatStats) } @@ -183,6 +187,13 @@ func decodeObject(_ obj: Any) throws -> T { try jsonDecoder.decode(T.self, from: JSONSerialization.data(withJSONObject: obj)) } +func decodeProperty(_ obj: Any, _ prop: NSString) -> T? { + if let jProp = (obj as? NSDictionary)?[prop] { + return try? decodeObject(jProp) + } + return nil +} + func prettyJSON(_ obj: Any) -> String? { if let d = try? JSONSerialization.data(withJSONObject: obj, options: .prettyPrinted) { return String(decoding: d, as: UTF8.self) diff --git a/apps/ios/SimpleXChat/ChatTypes.swift b/apps/ios/SimpleXChat/ChatTypes.swift index 1b33d34b2..17d91bc36 100644 --- a/apps/ios/SimpleXChat/ChatTypes.swift +++ b/apps/ios/SimpleXChat/ChatTypes.swift @@ -1943,10 +1943,10 @@ public struct ChatItem: Identifiable, Decodable { return item } - public static func invalidJSON(_ json: String) -> ChatItem { + public static func invalidJSON(chatDir: CIDirection?, meta: CIMeta?, json: String) -> ChatItem { ChatItem( - chatDir: CIDirection.directSnd, - meta: CIMeta.invalidJSON, + chatDir: chatDir ?? .directSnd, + meta: meta ?? .invalidJSON, content: .invalidJSON(json: json), quotedItem: nil, file: nil @@ -2178,13 +2178,11 @@ public enum CIContent: Decodable, ItemContent { public enum MsgDecryptError: String, Decodable { case ratchetHeader - case earlier case tooManySkipped var text: String { switch self { case .ratchetHeader: return NSLocalizedString("Permanent decryption error", comment: "message decrypt error item") - case .earlier: return NSLocalizedString("Decryption error", comment: "message decrypt error item") case .tooManySkipped: return NSLocalizedString("Permanent decryption error", comment: "message decrypt error item") } } diff --git a/src/Simplex/Chat.hs b/src/Simplex/Chat.hs index f6e82e57b..db7e6874c 100644 --- a/src/Simplex/Chat.hs +++ b/src/Simplex/Chat.hs @@ -2820,19 +2820,19 @@ processAgentMessageConn user@User {userId} corrId agentConnId agentMessage = do agentMsgDecryptError :: AgentErrorType -> Maybe (MsgDecryptError, Word32) agentMsgDecryptError = \case AGENT (A_CRYPTO RATCHET_HEADER) -> Just (MDERatchetHeader, 1) - AGENT (A_CRYPTO (RATCHET_EARLIER n)) -> Just (MDEEarlier, n + 1) -- 1 is added to account for the message that has A_DUPLICATE error AGENT (A_CRYPTO (RATCHET_SKIPPED n)) -> Just (MDETooManySkipped, n) -- we are not treating this as decryption error, as in many cases it happens as the result of duplicate or redundant delivery, -- and we don't have a way to differentiate. -- we could store the hashes of past messages in the agent, or delaying message deletion after ACK -- A_DUPLICATE -> Nothing + -- earlier messages may be received in case of redundant delivery, and do not necessarily indicate an error + -- AGENT (A_CRYPTO (RATCHET_EARLIER n)) -> Nothing _ -> Nothing mdeUpdatedCI :: (MsgDecryptError, Word32) -> CChatItem c -> Maybe (ChatItem c 'MDRcv, CIContent 'MDRcv) mdeUpdatedCI (mde', n') (CChatItem _ ci@ChatItem {content = CIRcvDecryptionError mde n}) | mde == mde' = case mde of MDERatchetHeader -> r (n + n') - MDEEarlier -> r n -- the first error in a sequence has the largest number – it's the number of messages to receive to catch up, keeping it MDETooManySkipped -> r n' -- the numbers are not added as sequential MDETooManySkipped will have it incremented by 1 | otherwise = Nothing where diff --git a/src/Simplex/Chat/Messages.hs b/src/Simplex/Chat/Messages.hs index 82dbcb695..18eee1f5d 100644 --- a/src/Simplex/Chat/Messages.hs +++ b/src/Simplex/Chat/Messages.hs @@ -735,10 +735,7 @@ data CIContent (d :: MsgDirection) where deriving instance Show (CIContent d) -data MsgDecryptError - = MDERatchetHeader - | MDEEarlier - | MDETooManySkipped +data MsgDecryptError = MDERatchetHeader | MDETooManySkipped deriving (Eq, Show, Generic) instance ToJSON MsgDecryptError where @@ -954,7 +951,6 @@ msgDecryptErrorText err n = where errName = case err of MDERatchetHeader -> "header" - MDEEarlier -> "earlier message" MDETooManySkipped -> "too many skipped messages" msgDirToModeratedContent_ :: SMsgDirection d -> CIContent d diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index b80835992..f8a7ef00c 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -2055,7 +2055,6 @@ testGroupMsgDecryptError tmp = bob #> "#team 4" alice <# "#team bob> 4" bob #> "#team 5" - cath <# "#team bob> decryption error, possibly due to the device change (earlier message, 2 messages)" cath <# "#team bob> incorrect message hash" [alice, cath] *<# "#team bob> 5" bob #> "#team 6"