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
This commit is contained in:
Evgeny Poberezkin 2023-04-16 19:30:25 +02:00 committed by GitHub
parent 393238f47c
commit 5b4c183466
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 31 additions and 29 deletions

View File

@ -1466,10 +1466,10 @@ data class ChatItem (
file = null file = null
) )
fun invalidJSON(json: String): ChatItem = fun invalidJSON(chatDir: CIDirection?, meta: CIMeta?, json: String): ChatItem =
ChatItem( ChatItem(
chatDir = CIDirection.DirectSnd(), chatDir = chatDir ?: CIDirection.DirectSnd(),
meta = CIMeta.invalidJSON(), meta = meta ?: CIMeta.invalidJSON(),
content = CIContent.InvalidJSON(json), content = CIContent.InvalidJSON(json),
quotedItem = null, quotedItem = null,
file = null file = null
@ -1681,13 +1681,11 @@ sealed class CIContent: ItemContent {
@Serializable @Serializable
enum class MsgDecryptError { enum class MsgDecryptError {
@SerialName("ratchetHeader") RatchetHeader, @SerialName("ratchetHeader") RatchetHeader,
@SerialName("earlier") Earlier,
@SerialName("tooManySkipped") TooManySkipped; @SerialName("tooManySkipped") TooManySkipped;
val text: String get() = when (this) { val text: String get() = when (this) {
RatchetHeader -> generalGetString(R.string.decryption_error_permanent) RatchetHeader -> generalGetString(R.string.decryption_error)
Earlier -> generalGetString(R.string.decryption_error) TooManySkipped -> generalGetString(R.string.decryption_error)
TooManySkipped -> generalGetString(R.string.decryption_error_permanent)
} }
} }

View File

@ -3011,9 +3011,15 @@ private fun parseChatData(chat: JsonElement): Chat {
?: ChatInfo.InvalidJSON(json.encodeToString(chat.jsonObject["chatInfo"])) ?: ChatInfo.InvalidJSON(json.encodeToString(chat.jsonObject["chatInfo"]))
val chatStats = decodeObject(Chat.ChatStats.serializer(), chat.jsonObject["chatStats"])!! val chatStats = decodeObject(Chat.ChatStats.serializer(), chat.jsonObject["chatStats"])!!
val chatItems: List<ChatItem> = chat.jsonObject["chatItems"]!!.jsonArray.map { val chatItems: List<ChatItem> = 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 <T> decodeObject(deserializer: DeserializationStrategy<T>, obj: JsonElement?): T? = private fun <T> decodeObject(deserializer: DeserializationStrategy<T>, obj: JsonElement?): T? =

View File

@ -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" + 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_encryption_out_of_sync_old_database) + "\n" +
generalGetString(R.string.alert_text_fragment_permanent_error_reconnect) 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" + 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_encryption_out_of_sync_old_database) + "\n" +
generalGetString(R.string.alert_text_fragment_permanent_error_reconnect) generalGetString(R.string.alert_text_fragment_permanent_error_reconnect)

View File

@ -32,7 +32,6 @@
<string name="moderated_description">moderated</string> <string name="moderated_description">moderated</string>
<string name="invalid_chat">invalid chat</string> <string name="invalid_chat">invalid chat</string>
<string name="invalid_data">invalid data</string> <string name="invalid_data">invalid data</string>
<string name="decryption_error_permanent">Permanent decryption error</string>
<string name="decryption_error">Decryption error</string> <string name="decryption_error">Decryption error</string>
<!-- PendingContactConnection - ChatModel.kt --> <!-- PendingContactConnection - ChatModel.kt -->
@ -749,7 +748,6 @@
<string name="alert_title_msg_bad_id">Bad message ID</string> <string name="alert_title_msg_bad_id">Bad message ID</string>
<string name="alert_text_msg_bad_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.</string> <string name="alert_text_msg_bad_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.</string>
<string name="alert_text_decryption_error_header"><xliff:g id="message count" example="1">%1$d</xliff:g> messages failed to decrypt.</string> <string name="alert_text_decryption_error_header"><xliff:g id="message count" example="1">%1$d</xliff:g> messages failed to decrypt.</string>
<string name="alert_text_decryption_error_earlier"><xliff:g id="message count" example="1">%1$d</xliff:g> messages failed to decrypt and won\'t be shown.</string>
<string name="alert_text_decryption_error_too_many_skipped"><xliff:g id="message count" example="1">%1$d</xliff:g> messages skipped.</string> <string name="alert_text_decryption_error_too_many_skipped"><xliff:g id="message count" example="1">%1$d</xliff:g> messages skipped.</string>
<string name="alert_text_fragment_encryption_out_of_sync_old_database">It can happen when you or your connection used the old database backup.</string> <string name="alert_text_fragment_encryption_out_of_sync_old_database">It can happen when you or your connection used the old database backup.</string>
<string name="alert_text_fragment_permanent_error_reconnect">This error is permanent for this connection, please re-connect.</string> <string name="alert_text_fragment_permanent_error_reconnect">This error is permanent for this connection, please re-connect.</string>

View File

@ -25,8 +25,6 @@ struct CIRcvDecryptionError: View {
switch msgDecryptError { switch msgDecryptError {
case .ratchetHeader: case .ratchetHeader:
message = Text("\(msgCount) messages failed to decrypt.") + Text("\n") + why + Text("\n") + permanent 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: case .tooManySkipped:
message = Text("\(msgCount) messages skipped.") + Text("\n") + why + Text("\n") + permanent message = Text("\(msgCount) messages skipped.") + Text("\n") + why + Text("\n") + permanent
} }

View File

@ -174,7 +174,11 @@ func parseChatData(_ jChat: Any) throws -> ChatData {
if let ci: ChatItem = try? decodeObject(jCI) { if let ci: ChatItem = try? decodeObject(jCI) {
return ci 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) return ChatData(chatInfo: chatInfo, chatItems: chatItems, chatStats: chatStats)
} }
@ -183,6 +187,13 @@ func decodeObject<T: Decodable>(_ obj: Any) throws -> T {
try jsonDecoder.decode(T.self, from: JSONSerialization.data(withJSONObject: obj)) try jsonDecoder.decode(T.self, from: JSONSerialization.data(withJSONObject: obj))
} }
func decodeProperty<T: Decodable>(_ obj: Any, _ prop: NSString) -> T? {
if let jProp = (obj as? NSDictionary)?[prop] {
return try? decodeObject(jProp)
}
return nil
}
func prettyJSON(_ obj: Any) -> String? { func prettyJSON(_ obj: Any) -> String? {
if let d = try? JSONSerialization.data(withJSONObject: obj, options: .prettyPrinted) { if let d = try? JSONSerialization.data(withJSONObject: obj, options: .prettyPrinted) {
return String(decoding: d, as: UTF8.self) return String(decoding: d, as: UTF8.self)

View File

@ -1943,10 +1943,10 @@ public struct ChatItem: Identifiable, Decodable {
return item return item
} }
public static func invalidJSON(_ json: String) -> ChatItem { public static func invalidJSON(chatDir: CIDirection?, meta: CIMeta?, json: String) -> ChatItem {
ChatItem( ChatItem(
chatDir: CIDirection.directSnd, chatDir: chatDir ?? .directSnd,
meta: CIMeta.invalidJSON, meta: meta ?? .invalidJSON,
content: .invalidJSON(json: json), content: .invalidJSON(json: json),
quotedItem: nil, quotedItem: nil,
file: nil file: nil
@ -2178,13 +2178,11 @@ public enum CIContent: Decodable, ItemContent {
public enum MsgDecryptError: String, Decodable { public enum MsgDecryptError: String, Decodable {
case ratchetHeader case ratchetHeader
case earlier
case tooManySkipped case tooManySkipped
var text: String { var text: String {
switch self { switch self {
case .ratchetHeader: return NSLocalizedString("Permanent decryption error", comment: "message decrypt error item") 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") case .tooManySkipped: return NSLocalizedString("Permanent decryption error", comment: "message decrypt error item")
} }
} }

View File

@ -2820,19 +2820,19 @@ processAgentMessageConn user@User {userId} corrId agentConnId agentMessage = do
agentMsgDecryptError :: AgentErrorType -> Maybe (MsgDecryptError, Word32) agentMsgDecryptError :: AgentErrorType -> Maybe (MsgDecryptError, Word32)
agentMsgDecryptError = \case agentMsgDecryptError = \case
AGENT (A_CRYPTO RATCHET_HEADER) -> Just (MDERatchetHeader, 1) 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) 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, -- 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. -- 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 -- we could store the hashes of past messages in the agent, or delaying message deletion after ACK
-- A_DUPLICATE -> Nothing -- 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 _ -> Nothing
mdeUpdatedCI :: (MsgDecryptError, Word32) -> CChatItem c -> Maybe (ChatItem c 'MDRcv, CIContent 'MDRcv) mdeUpdatedCI :: (MsgDecryptError, Word32) -> CChatItem c -> Maybe (ChatItem c 'MDRcv, CIContent 'MDRcv)
mdeUpdatedCI (mde', n') (CChatItem _ ci@ChatItem {content = CIRcvDecryptionError mde n}) mdeUpdatedCI (mde', n') (CChatItem _ ci@ChatItem {content = CIRcvDecryptionError mde n})
| mde == mde' = case mde of | mde == mde' = case mde of
MDERatchetHeader -> r (n + n') 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 MDETooManySkipped -> r n' -- the numbers are not added as sequential MDETooManySkipped will have it incremented by 1
| otherwise = Nothing | otherwise = Nothing
where where

View File

@ -735,10 +735,7 @@ data CIContent (d :: MsgDirection) where
deriving instance Show (CIContent d) deriving instance Show (CIContent d)
data MsgDecryptError data MsgDecryptError = MDERatchetHeader | MDETooManySkipped
= MDERatchetHeader
| MDEEarlier
| MDETooManySkipped
deriving (Eq, Show, Generic) deriving (Eq, Show, Generic)
instance ToJSON MsgDecryptError where instance ToJSON MsgDecryptError where
@ -954,7 +951,6 @@ msgDecryptErrorText err n =
where where
errName = case err of errName = case err of
MDERatchetHeader -> "header" MDERatchetHeader -> "header"
MDEEarlier -> "earlier message"
MDETooManySkipped -> "too many skipped messages" MDETooManySkipped -> "too many skipped messages"
msgDirToModeratedContent_ :: SMsgDirection d -> CIContent d msgDirToModeratedContent_ :: SMsgDirection d -> CIContent d

View File

@ -2055,7 +2055,6 @@ testGroupMsgDecryptError tmp =
bob #> "#team 4" bob #> "#team 4"
alice <# "#team bob> 4" alice <# "#team bob> 4"
bob #> "#team 5" bob #> "#team 5"
cath <# "#team bob> decryption error, possibly due to the device change (earlier message, 2 messages)"
cath <# "#team bob> incorrect message hash" cath <# "#team bob> incorrect message hash"
[alice, cath] *<# "#team bob> 5" [alice, cath] *<# "#team bob> 5"
bob #> "#team 6" bob #> "#team 6"