From d7772d3fc6022c3a28dd2b1ad350b310981993f2 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Fri, 15 Jan 2021 18:47:30 +0100 Subject: [PATCH] Skip wrapping emojis in private group chats This commit fixes a couple of issues: 1) Emojis were sent to any member of the group chat, regardless of whether they joined 2) We don't want to wrap emojis, as there's no need to do so, only messages are to be wrapped --- appdatabase/migrations/bindata.go | 2 +- protocol/common/chat_entity.go | 1 + protocol/common/message.go | 5 ++ protocol/common/raw_message.go | 1 + protocol/emoji_reaction.go | 5 ++ protocol/messenger.go | 48 ++++++++++++------- protocol/migrations/migrations.go | 30 ++++++++++-- ...59908_add_dont_wrap_to_raw_messages.up.sql | 1 + protocol/persistence.go | 16 +++++-- 9 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 protocol/migrations/sqlite/1610959908_add_dont_wrap_to_raw_messages.up.sql diff --git a/appdatabase/migrations/bindata.go b/appdatabase/migrations/bindata.go index 8f2ce97ad..03270a522 100644 --- a/appdatabase/migrations/bindata.go +++ b/appdatabase/migrations/bindata.go @@ -736,7 +736,7 @@ func _0018_profile_pictures_visibilityUpSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "0018_profile_pictures_visibility.up.sql", size: 84, mode: os.FileMode(0644), modTime: time.Unix(1610715642, 0)} + info := bindataFileInfo{name: "0018_profile_pictures_visibility.up.sql", size: 84, mode: os.FileMode(0644), modTime: time.Unix(1610959528, 0)} a := &asset{bytes: bytes, info: info, digest: [32]uint8{0xc9, 0xe3, 0xc5, 0xec, 0x83, 0x55, 0x45, 0x57, 0x7a, 0xaa, 0xd2, 0xa7, 0x59, 0xa7, 0x87, 0xef, 0x63, 0x19, 0x9c, 0x46, 0x9c, 0xc5, 0x32, 0x89, 0xa4, 0x68, 0x70, 0xd8, 0x83, 0x43, 0xa4, 0x72}} return a, nil } diff --git a/protocol/common/chat_entity.go b/protocol/common/chat_entity.go index 7d96e6ad9..992c9b77a 100644 --- a/protocol/common/chat_entity.go +++ b/protocol/common/chat_entity.go @@ -18,6 +18,7 @@ type ChatEntity interface { GetSigPubKey() *ecdsa.PublicKey GetProtobuf() proto.Message GetGrant() []byte + WrapGroupMessage() bool SetMessageType(messageType protobuf.MessageType) } diff --git a/protocol/common/message.go b/protocol/common/message.go index 730d8ae3f..bfb49b400 100644 --- a/protocol/common/message.go +++ b/protocol/common/message.go @@ -392,3 +392,8 @@ func (m *Message) GetProtobuf() proto.Message { func (m *Message) SetMessageType(messageType protobuf.MessageType) { m.MessageType = messageType } + +// WrapGroupMessage indicates whether we should wrap this in membership information +func (m *Message) WrapGroupMessage() bool { + return true +} diff --git a/protocol/common/raw_message.go b/protocol/common/raw_message.go index 9ac87cebe..02137bed4 100644 --- a/protocol/common/raw_message.go +++ b/protocol/common/raw_message.go @@ -21,4 +21,5 @@ type RawMessage struct { Payload []byte Sender *ecdsa.PrivateKey Recipients []*ecdsa.PublicKey + SkipGroupMessageWrap bool } diff --git a/protocol/emoji_reaction.go b/protocol/emoji_reaction.go index 9687c5c69..88c9dc9f1 100644 --- a/protocol/emoji_reaction.go +++ b/protocol/emoji_reaction.go @@ -76,3 +76,8 @@ func (e EmojiReaction) MarshalJSON() ([]byte, error) { return json.Marshal(item) } + +// WrapGroupMessage indicates whether we should wrap this in membership information +func (e EmojiReaction) WrapGroupMessage() bool { + return false +} diff --git a/protocol/messenger.go b/protocol/messenger.go index 1a6a9964f..581273108 100644 --- a/protocol/messenger.go +++ b/protocol/messenger.go @@ -2266,8 +2266,10 @@ func (m *Messenger) dispatchMessage(ctx context.Context, spec common.RawMessage) case ChatTypePrivateGroupChat: logger.Debug("sending group message", zap.String("chatName", chat.Name)) if spec.Recipients == nil { - // Chat messages are only dispatched to users who joined - if spec.MessageType == protobuf.ApplicationMetadataMessage_CHAT_MESSAGE { + // Anything that is not a membership update message is only dispatched to joined users + // NOTE: I think here it might make sense to always invite to joined users apart from the + // initial message + if spec.MessageType != protobuf.ApplicationMetadataMessage_MEMBERSHIP_UPDATE_MESSAGE { spec.Recipients, err = chat.JoinedMembersAsPublicKeys() if err != nil { return nil, err @@ -2280,6 +2282,7 @@ func (m *Messenger) dispatchMessage(ctx context.Context, spec common.RawMessage) } } } + hasPairedDevices := m.hasPairedDevices() if !hasPairedDevices { @@ -2294,8 +2297,10 @@ func (m *Messenger) dispatchMessage(ctx context.Context, spec common.RawMessage) spec.Recipients = spec.Recipients[:n] } - spec.MessageType = protobuf.ApplicationMetadataMessage_MEMBERSHIP_UPDATE_MESSAGE - // We always wrap in group information + // We skip wrapping in some cases (emoji reactions for example) + if !spec.SkipGroupMessageWrap { + spec.MessageType = protobuf.ApplicationMetadataMessage_MEMBERSHIP_UPDATE_MESSAGE + } id, err = m.processor.SendGroup(ctx, spec.Recipients, spec) if err != nil { return nil, err @@ -4385,9 +4390,10 @@ func (m *Messenger) SendEmojiReaction(ctx context.Context, chatID, messageID str } _, err = m.dispatchMessage(ctx, common.RawMessage{ - LocalChatID: chatID, - Payload: encodedMessage, - MessageType: protobuf.ApplicationMetadataMessage_EMOJI_REACTION, + LocalChatID: chatID, + Payload: encodedMessage, + SkipGroupMessageWrap: true, + MessageType: protobuf.ApplicationMetadataMessage_EMOJI_REACTION, // Don't resend using datasync, that would create quite a lot // of traffic if clicking too eagelry ResendAutomatically: false, @@ -4462,9 +4468,10 @@ func (m *Messenger) SendEmojiReactionRetraction(ctx context.Context, emojiReacti // Send the marshalled EmojiReactionRetraction protobuf _, err = m.dispatchMessage(ctx, common.RawMessage{ - LocalChatID: emojiR.GetChatId(), - Payload: encodedMessage, - MessageType: protobuf.ApplicationMetadataMessage_EMOJI_REACTION, + LocalChatID: emojiR.GetChatId(), + Payload: encodedMessage, + SkipGroupMessageWrap: true, + MessageType: protobuf.ApplicationMetadataMessage_EMOJI_REACTION, // Don't resend using datasync, that would create quite a lot // of traffic if clicking too eagelry ResendAutomatically: false, @@ -4519,15 +4526,22 @@ func (m *Messenger) encodeChatEntity(chat *Chat, message common.ChatEntity) ([]b case ChatTypePrivateGroupChat: message.SetMessageType(protobuf.MessageType_PRIVATE_GROUP) l.Debug("sending group message", zap.String("chatName", chat.Name)) + if !message.WrapGroupMessage() { + encodedMessage, err = proto.Marshal(message.GetProtobuf()) + if err != nil { + return nil, err + } + } else { - group, err := newProtocolGroupFromChat(chat) - if err != nil { - return nil, err - } + group, err := newProtocolGroupFromChat(chat) + if err != nil { + return nil, err + } - encodedMessage, err = m.processor.EncodeMembershipUpdate(group, message) - if err != nil { - return nil, err + encodedMessage, err = m.processor.EncodeMembershipUpdate(group, message) + if err != nil { + return nil, err + } } default: diff --git a/protocol/migrations/migrations.go b/protocol/migrations/migrations.go index 213d340f2..8ec5f51d4 100644 --- a/protocol/migrations/migrations.go +++ b/protocol/migrations/migrations.go @@ -21,6 +21,7 @@ // 1603888149_create_chat_identity_last_published_table.up.sql (407B) // 1605075346_add_communities.up.sql (6.971kB) // 1610117927_add_message_cache.up.sql (142B) +// 1610959908_add_dont_wrap_to_raw_messages.up.sql (83B) // README.md (554B) // doc.go (850B) @@ -506,11 +507,31 @@ func _1610117927_add_message_cacheUpSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "1610117927_add_message_cache.up.sql", size: 142, mode: os.FileMode(0644), modTime: time.Unix(1610715642, 0)} + info := bindataFileInfo{name: "1610117927_add_message_cache.up.sql", size: 142, mode: os.FileMode(0644), modTime: time.Unix(1610959528, 0)} a := &asset{bytes: bytes, info: info, digest: [32]uint8{0x34, 0xf1, 0xf0, 0x82, 0x79, 0x28, 0x19, 0xc2, 0x39, 0x6a, 0xa5, 0x96, 0x59, 0x23, 0xa0, 0xed, 0x60, 0x58, 0x86, 0x9, 0xb9, 0xad, 0xfb, 0xa, 0xe3, 0x47, 0x6e, 0xa1, 0x18, 0xe8, 0x39, 0x2c}} return a, nil } +var __1610959908_add_dont_wrap_to_raw_messagesUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x28\x4a\x2c\x8f\xcf\x4d\x2d\x2e\x4e\x4c\x4f\x2d\x56\x70\x74\x71\x51\x70\xf6\xf7\x09\xf5\xf5\x53\x28\xce\xce\x2c\x88\x4f\x2f\xca\x2f\x2d\x80\x49\xc7\x97\x17\x25\x16\x28\x38\xf9\xfb\xfb\xb8\x3a\xfa\x29\xb8\xb8\xba\x39\x86\xfa\x84\x28\xb8\x39\xfa\x04\xbb\x5a\x73\x01\x02\x00\x00\xff\xff\x3c\x1f\xd3\xe4\x53\x00\x00\x00") + +func _1610959908_add_dont_wrap_to_raw_messagesUpSqlBytes() ([]byte, error) { + return bindataRead( + __1610959908_add_dont_wrap_to_raw_messagesUpSql, + "1610959908_add_dont_wrap_to_raw_messages.up.sql", + ) +} + +func _1610959908_add_dont_wrap_to_raw_messagesUpSql() (*asset, error) { + bytes, err := _1610959908_add_dont_wrap_to_raw_messagesUpSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "1610959908_add_dont_wrap_to_raw_messages.up.sql", size: 83, mode: os.FileMode(0644), modTime: time.Unix(1610959977, 0)} + a := &asset{bytes: bytes, info: info, digest: [32]uint8{0x71, 0x2, 0x9a, 0xca, 0xd4, 0x38, 0x44, 0x30, 0x2b, 0xa8, 0x27, 0x32, 0x63, 0x53, 0x22, 0x60, 0x59, 0x84, 0x23, 0x96, 0x77, 0xf0, 0x56, 0xd7, 0x94, 0xe0, 0x95, 0x28, 0x6, 0x1d, 0x4e, 0xb1}} + return a, nil +} + var _readmeMd = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x54\x91\xc1\xce\xd3\x30\x10\x84\xef\x7e\x8a\x91\x7a\x01\xa9\x2a\x8f\xc0\x0d\x71\x82\x03\x48\x1c\xc9\x36\x9e\x36\x96\x1c\x6f\xf0\xae\x93\xe6\xed\x91\xa3\xc2\xdf\xff\x66\xed\xd8\x33\xdf\x78\x4f\xa7\x13\xbe\xea\x06\x57\x6c\x35\x39\x31\xa7\x7b\x15\x4f\x5a\xec\x73\x08\xbf\x08\x2d\x79\x7f\x4a\x43\x5b\x86\x17\xfd\x8c\x21\xea\x56\x5e\x47\x90\x4a\x14\x75\x48\xde\x64\x37\x2c\x6a\x96\xae\x99\x48\x05\xf6\x27\x77\x13\xad\x08\xae\x8a\x51\xe7\x25\xf3\xf1\xa9\x9f\xf9\x58\x58\x2c\xad\xbc\xe0\x8b\x56\xf0\x21\x5d\xeb\x4c\x95\xb3\xae\x84\x60\xd4\xdc\xe6\x82\x5d\x1b\x36\x6d\x39\x62\x92\xf5\xb8\x11\xdb\x92\xd3\x28\xce\xe0\x13\xe1\x72\xcd\x3c\x63\xd4\x65\x87\xae\xac\xe8\xc3\x28\x2e\x67\x44\x66\x3a\x21\x25\xa2\x72\xac\x14\x67\xbc\x84\x9f\x53\x32\x8c\x52\x70\x25\x56\xd6\xfd\x8d\x05\x37\xad\x30\x9d\x9f\xa6\x86\x0f\xcd\x58\x7f\xcf\x34\x93\x3b\xed\x90\x9f\xa4\x1f\xcf\x30\x85\x4d\x07\x58\xaf\x7f\x25\xc4\x9d\xf3\x72\x64\x84\xd0\x7f\xf9\x9b\x3a\x2d\x84\xef\x85\x48\x66\x8d\xd8\x88\x9b\x8c\x8c\x98\x5b\xf6\x74\x14\x4e\x33\x0d\xc9\xe0\x93\x38\xda\x12\xc5\x69\xbd\xe4\xf0\x2e\x7a\x78\x07\x1c\xfe\x13\x9f\x91\x29\x31\x95\x7b\x7f\x62\x59\x37\xb4\xe5\x5e\x25\xfe\x33\xee\xd5\x53\x71\xd6\xda\x3a\xd8\xcb\xde\x2e\xf8\xa1\x90\x55\x53\x0c\xc7\xaa\x0d\xe9\x76\x14\x29\x1c\x7b\x68\xdd\x2f\xe1\x6f\x00\x00\x00\xff\xff\x3c\x0a\xc2\xfe\x2a\x02\x00\x00") func readmeMdBytes() ([]byte, error) { @@ -684,6 +705,8 @@ var _bindata = map[string]func() (*asset, error){ "1610117927_add_message_cache.up.sql": _1610117927_add_message_cacheUpSql, + "1610959908_add_dont_wrap_to_raw_messages.up.sql": _1610959908_add_dont_wrap_to_raw_messagesUpSql, + "README.md": readmeMd, "doc.go": docGo, @@ -751,8 +774,9 @@ var _bintree = &bintree{nil, map[string]*bintree{ "1603888149_create_chat_identity_last_published_table.up.sql": &bintree{_1603888149_create_chat_identity_last_published_tableUpSql, map[string]*bintree{}}, "1605075346_add_communities.up.sql": &bintree{_1605075346_add_communitiesUpSql, map[string]*bintree{}}, "1610117927_add_message_cache.up.sql": &bintree{_1610117927_add_message_cacheUpSql, map[string]*bintree{}}, - "README.md": &bintree{readmeMd, map[string]*bintree{}}, - "doc.go": &bintree{docGo, map[string]*bintree{}}, + "1610959908_add_dont_wrap_to_raw_messages.up.sql": &bintree{_1610959908_add_dont_wrap_to_raw_messagesUpSql, map[string]*bintree{}}, + "README.md": &bintree{readmeMd, map[string]*bintree{}}, + "doc.go": &bintree{docGo, map[string]*bintree{}}, }} // RestoreAsset restores an asset under the given directory. diff --git a/protocol/migrations/sqlite/1610959908_add_dont_wrap_to_raw_messages.up.sql b/protocol/migrations/sqlite/1610959908_add_dont_wrap_to_raw_messages.up.sql new file mode 100644 index 000000000..15a6b5821 --- /dev/null +++ b/protocol/migrations/sqlite/1610959908_add_dont_wrap_to_raw_messages.up.sql @@ -0,0 +1 @@ +ALTER TABLE raw_messages ADD COLUMN skip_group_message_wrap BOOLEAN DEFAULT FALSE; diff --git a/protocol/persistence.go b/protocol/persistence.go index f0cb763f1..ca0f83a13 100644 --- a/protocol/persistence.go +++ b/protocol/persistence.go @@ -590,10 +590,11 @@ func (db sqlitePersistence) SaveRawMessage(message *common.RawMessage) error { resend_automatically, recipients, skip_encryption, - send_push_notification, + send_push_notification, + skip_group_message_wrap, payload ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,?)`, message.ID, message.LocalChatID, message.LastSent, @@ -604,6 +605,7 @@ func (db sqlitePersistence) SaveRawMessage(message *common.RawMessage) error { encodedRecipients.Bytes(), message.SkipEncryption, message.SendPushNotification, + message.SkipGroupMessageWrap, message.Payload) return err } @@ -611,6 +613,7 @@ func (db sqlitePersistence) SaveRawMessage(message *common.RawMessage) error { func (db sqlitePersistence) RawMessageByID(id string) (*common.RawMessage, error) { var rawPubKeys [][]byte var encodedRecipients []byte + var skipGroupMessageWrap sql.NullBool message := &common.RawMessage{} err := db.db.QueryRow(` @@ -624,7 +627,8 @@ func (db sqlitePersistence) RawMessageByID(id string) (*common.RawMessage, error resend_automatically, recipients, skip_encryption, - send_push_notification, + send_push_notification, + skip_group_message_wrap, payload FROM raw_messages @@ -642,6 +646,7 @@ func (db sqlitePersistence) RawMessageByID(id string) (*common.RawMessage, error &encodedRecipients, &message.SkipEncryption, &message.SendPushNotification, + &skipGroupMessageWrap, &message.Payload, ) if err != nil { @@ -662,6 +667,11 @@ func (db sqlitePersistence) RawMessageByID(id string) (*common.RawMessage, error message.Recipients = append(message.Recipients, pubkey) } + if skipGroupMessageWrap.Valid { + message.SkipGroupMessageWrap = skipGroupMessageWrap.Bool + + } + return message, nil }