Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-26 09:56:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

feat: Show broadcast channels in their own, proper "Channel" chat (#6901)

Browse Source 
Part of #6884 

----

- [x] Add new chat type `InBroadcastChannel` and `OutBroadcastChannel`
for incoming / outgoing channels, where the former is similar to a
`Mailinglist` and the latter is similar to a `Broadcast` (which is
removed)
- Consideration for naming: `InChannel`/`OutChannel` (without
"broadcast") would be shorter, but less greppable because we already
have a lot of occurences of `channel` in the code. Consistently calling
them `BcChannel`/`bc_channel` in the code would be both short and
greppable, but a bit arcane when reading it at first. Opinions are
welcome; if I hear none, I'll keep with `BroadcastChannel`.
- [x] api: Add create_broadcast_channel(), deprecate
create_broadcast_list() (or `create_channel()` / `create_bc_channel()`
if we decide to switch)
  - Adjust code comments to match the new behavior.
- [x] Ask Desktop developers what they use `is_broadcast` field for, and
whether it should be true for both outgoing & incoming channels (or look
it up myself)
- I added `is_out_broadcast_channel`, and deprecated `is_broadcast`, for
now
- [x] When the user changes the broadcast channel name, immediately show
this change on receiving devices
- [x] Allow to change brodacast channel avatar, and immediately apply it
on the receiving device
- [x] Make it possible to block InBroadcastChannel
- [x] Make it possible to set the avatar of an OutgoingChannel, and
apply it on the receiving side
- [x] DECIDE whether we still want to use the broadcast icon as the
default icon or whether we want to use the letter-in-a-circle
- We decided to use the letter-in-a-circle for now, because it's easier
to implement, and I need to stay in the time plan
- [x] chat.rs: Return an error if the user tries to modify a
`InBroadcastChannel`
- [x] Add automated regression tests
- [x] Grep for `broadcast` and see whether there is any other work I
need to do
- [x] Bug: Don't show `~` in front of the sender's same in broadcast
lists

----

Note that I removed the following guard:

```rust
        if !new_chat_contacts.contains(&ContactId::SELF) {
            warn!(
                context,
                "Received group avatar update for group chat {} we are not a member of.", chat.id
            );
        } else if !new_chat_contacts.contains(&from_id) {
            warn!(
                context,
                "Contact {from_id} attempts to modify group chat {} avatar without being a member.",
                chat.id,
            );
        } else [...]
```

i.e. with this change, non-members will be able to modify the avatar.
Things were slightly easier this way, and I think that this is in line
with non-members being able to modify the group name and memberlist
(they need to know the Group-Chat-Id, anyway), but I can also change it
back.
This commit is contained in:
Hocuri
2025-07-02 22:40:30 +02:00
committed by GitHub
parent 2ee3f58b69
commit 0a73c2b7ab
23 changed files with 744 additions and 319 deletions
Download Patch File Download Diff File Expand all files Collapse all files

390
src/receive_imf.rs
View File

@@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet};
use std::iter;
use std::sync::LazyLock;
use anyhow::{Context as _, Result};
use anyhow::{Context as _, Result, ensure};
use data_encoding::BASE32_NOPAD;
use deltachat_contact_tools::{
ContactAddress, addr_cmp, addr_normalize, may_be_valid_addr, sanitize_single_line,
@@ -93,10 +93,10 @@ enum ChatAssignment {
/// assign to encrypted group.
GroupChat { grpid: String },
/// Mailing list or broadcast list.
/// Mailing list or broadcast channel.
///
/// Mailing lists don't have members.
/// Broadcast lists have members
/// Broadcast channels have members
/// on the sender side,
/// but their addresses don't go into
/// the `To` field.
@@ -107,7 +107,7 @@ enum ChatAssignment {
/// up except the `from_id`
/// which may be an email address contact
/// or a key-contact.
MailingList,
MailingListOrBroadcast,
/// Group chat without a Group ID.
///
@@ -261,7 +261,7 @@ async fn get_to_and_past_contact_ids(
None
}
ChatAssignment::ExistingChat { chat_id, .. } => Some(*chat_id),
ChatAssignment::MailingList => None,
ChatAssignment::MailingListOrBroadcast => None,
ChatAssignment::OneOneChat => {
if is_partial_download.is_none() && !mime_parser.incoming {
parent_message.as_ref().map(|m| m.chat_id)
@@ -326,7 +326,7 @@ async fn get_to_and_past_contact_ids(
.await?;
}
}
ChatAssignment::Trash | ChatAssignment::MailingList => {
ChatAssignment::Trash | ChatAssignment::MailingListOrBroadcast => {
to_ids = Vec::new();
past_ids = Vec::new();
}
@@ -597,8 +597,8 @@ pub(crate) async fn receive_imf_inner(
return Ok(None);
};
let prevent_rename =
mime_parser.is_mailinglist_message() || mime_parser.get_header(HeaderDef::Sender).is_some();
let prevent_rename = (mime_parser.is_mailinglist_message() && !mime_parser.was_encrypted())
|| mime_parser.get_header(HeaderDef::Sender).is_some();
// get From: (it can be an address list!) and check if it is known (for known From:'s we add
// the other To:/Cc: in the 3rd pass)
@@ -1201,6 +1201,8 @@ async fn decide_chat_assignment(
let chat_assignment = if should_trash {
ChatAssignment::Trash
} else if mime_parser.get_mailinglist_header().is_some() {
ChatAssignment::MailingListOrBroadcast
} else if let Some(grpid) = mime_parser.get_chat_group_id() {
if mime_parser.was_encrypted() {
ChatAssignment::GroupChat {
@@ -1228,8 +1230,6 @@ async fn decide_chat_assignment(
// Group ID is ignored, however.
ChatAssignment::AdHocGroup
}
} else if mime_parser.get_mailinglist_header().is_some() {
ChatAssignment::MailingList
} else if let Some(parent) = &parent_message {
if let Some((chat_id, chat_id_blocked)) =
lookup_chat_by_reply(context, mime_parser, parent, is_partial_download).await?
@@ -1333,15 +1333,17 @@ async fn do_chat_assignment(
}
}
}
ChatAssignment::MailingList => {
ChatAssignment::MailingListOrBroadcast => {
if let Some(mailinglist_header) = mime_parser.get_mailinglist_header() {
if let Some((new_chat_id, new_chat_id_blocked)) = create_or_lookup_mailinglist(
context,
allow_creation,
mailinglist_header,
mime_parser,
)
.await?
if let Some((new_chat_id, new_chat_id_blocked)) =
create_or_lookup_mailinglist_or_broadcast(
context,
allow_creation,
mailinglist_header,
from_id,
mime_parser,
)
.await?
{
chat_id = Some(new_chat_id);
chat_id_blocked = new_chat_id_blocked;
@@ -1380,7 +1382,7 @@ async fn do_chat_assignment(
// unblock the chat
if chat_id_blocked != Blocked::Not
&& create_blocked != Blocked::Yes
&& !matches!(chat_assignment, ChatAssignment::MailingList)
&& !matches!(chat_assignment, ChatAssignment::MailingListOrBroadcast)
{
if let Some(chat_id) = chat_id {
chat_id.set_blocked(context, create_blocked).await?;
@@ -1510,8 +1512,9 @@ async fn do_chat_assignment(
chat_id = Some(*new_chat_id);
chat_id_blocked = *new_chat_id_blocked;
}
ChatAssignment::MailingList => {
// Check if the message belongs to a broadcast list.
ChatAssignment::MailingListOrBroadcast => {
// Check if the message belongs to a broadcast channel
// (it can't be a mailing list, since it's outgoing)
if let Some(mailinglist_header) = mime_parser.get_mailinglist_header() {
let listid = mailinglist_header_listid(mailinglist_header)?;
chat_id = Some(
@@ -1521,7 +1524,7 @@ async fn do_chat_assignment(
} else {
let name =
compute_mailinglist_name(mailinglist_header, &listid, mime_parser);
chat::create_broadcast_list_ex(context, Nosync, listid, name).await?
chat::create_broadcast_ex(context, Nosync, listid, name).await?
},
);
}
@@ -1662,16 +1665,28 @@ async fn add_parts(
let is_location_kml = mime_parser.location_kml.is_some();
let is_mdn = !mime_parser.mdn_reports.is_empty();
let mut group_changes = apply_group_changes(
context,
mime_parser,
chat_id,
from_id,
to_ids,
past_ids,
&verified_encryption,
)
.await?;
let mut chat = Chat::load_from_db(context, chat_id).await?;
let mut group_changes = match chat.typ {
_ if chat.id.is_special() => GroupChangesInfo::default(),
Chattype::Single => GroupChangesInfo::default(),
Chattype::Mailinglist => GroupChangesInfo::default(),
Chattype::OutBroadcast => GroupChangesInfo::default(),
Chattype::Group => {
apply_group_changes(
context,
mime_parser,
&mut chat,
from_id,
to_ids,
past_ids,
&verified_encryption,
)
.await?
}
Chattype::InBroadcast => {
apply_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
}
};
let rfc724_mid_orig = &mime_parser
.get_rfc724_mid()
@@ -2771,21 +2786,15 @@ struct GroupChangesInfo {
async fn apply_group_changes(
context: &Context,
mime_parser: &mut MimeMessage,
chat_id: ChatId,
chat: &mut Chat,
from_id: ContactId,
to_ids: &[Option<ContactId>],
past_ids: &[Option<ContactId>],
verified_encryption: &VerifiedEncryption,
) -> Result<GroupChangesInfo> {
let to_ids_flat: Vec<ContactId> = to_ids.iter().filter_map(|x| *x).collect();
if chat_id.is_special() {
// Do not apply group changes to the trash chat.
return Ok(GroupChangesInfo::default());
}
let mut chat = Chat::load_from_db(context, chat_id).await?;
if chat.typ != Chattype::Group {
return Ok(GroupChangesInfo::default());
}
ensure!(chat.typ == Chattype::Group);
ensure!(!chat.id.is_special());
let mut send_event_chat_modified = false;
let (mut removed_id, mut added_id) = (None, None);
@@ -2801,7 +2810,7 @@ async fn apply_group_changes(
};
let chat_contacts =
HashSet::<ContactId>::from_iter(chat::get_chat_contacts(context, chat_id).await?);
HashSet::<ContactId>::from_iter(chat::get_chat_contacts(context, chat.id).await?);
let is_from_in_chat =
!chat_contacts.contains(&ContactId::SELF) || chat_contacts.contains(&from_id);
@@ -2814,11 +2823,12 @@ async fn apply_group_changes(
} else {
warn!(
context,
"Not marking chat {chat_id} as protected due to verification problem: {err:#}."
"Not marking chat {} as protected due to verification problem: {err:#}.",
chat.id
);
}
} else if !chat.is_protected() {
chat_id
chat.id
.set_protection(
context,
ProtectionStatus::Protected,
@@ -2838,7 +2848,7 @@ async fn apply_group_changes(
// rather than old display name.
// This could be fixed by looking up the contact with the highest
// `remove_timestamp` after applying Chat-Group-Member-Timestamps.
removed_id = lookup_key_contact_by_address(context, removed_addr, Some(chat_id)).await?;
removed_id = lookup_key_contact_by_address(context, removed_addr, Some(chat.id)).await?;
if let Some(id) = removed_id {
better_msg = if id == from_id {
silent = true;
@@ -2872,70 +2882,15 @@ async fn apply_group_changes(
}
}
let group_name_timestamp = mime_parser
.get_header(HeaderDef::ChatGroupNameTimestamp)
.and_then(|s| s.parse::<i64>().ok());
if let Some(old_name) = mime_parser
.get_header(HeaderDef::ChatGroupNameChanged)
.map(|s| s.trim())
.or(match group_name_timestamp {
Some(0) => None,
Some(_) => Some(chat.name.as_str()),
None => None,
})
{
if let Some(grpname) = mime_parser
.get_header(HeaderDef::ChatGroupName)
.map(|grpname| grpname.trim())
.filter(|grpname| grpname.len() < 200)
{
let grpname = &sanitize_single_line(grpname);
let chat_group_name_timestamp =
chat.param.get_i64(Param::GroupNameTimestamp).unwrap_or(0);
let group_name_timestamp = group_name_timestamp.unwrap_or(mime_parser.timestamp_sent);
// To provide group name consistency, compare names if timestamps are equal.
if (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
&& chat_id
.update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp)
.await?
&& grpname != &chat.name
{
info!(context, "Updating grpname for chat {chat_id}.");
context
.sql
.execute("UPDATE chats SET name=? WHERE id=?;", (grpname, chat_id))
.await?;
send_event_chat_modified = true;
}
if mime_parser
.get_header(HeaderDef::ChatGroupNameChanged)
.is_some()
{
let old_name = &sanitize_single_line(old_name);
better_msg.get_or_insert(
stock_str::msg_grp_name(context, old_name, grpname, from_id).await,
);
}
}
}
if let (Some(value), None) = (mime_parser.get_header(HeaderDef::ChatContent), &better_msg) {
if value == "group-avatar-changed" {
if let Some(avatar_action) = &mime_parser.group_avatar {
// this is just an explicit message containing the group-avatar,
// apart from that, the group-avatar is send along with various other messages
better_msg = match avatar_action {
AvatarAction::Delete => {
Some(stock_str::msg_grp_img_deleted(context, from_id).await)
}
AvatarAction::Change(_) => {
Some(stock_str::msg_grp_img_changed(context, from_id).await)
}
};
}
}
}
apply_chat_name_and_avatar_changes(
context,
mime_parser,
from_id,
chat,
&mut send_event_chat_modified,
&mut better_msg,
)
.await?;
if is_from_in_chat {
if chat.member_list_is_stale(context).await? {
@@ -2954,7 +2909,7 @@ async fn apply_group_changes(
transaction.execute(
"DELETE FROM chats_contacts
WHERE chat_id=?",
(chat_id,),
(chat.id,),
)?;
// Insert contacts with default timestamps of 0.
@@ -2963,7 +2918,7 @@ async fn apply_group_changes(
VALUES (?, ?)",
)?;
for contact_id in &new_members {
statement.execute((chat_id, contact_id))?;
statement.execute((chat.id, contact_id))?;
}
Ok(())
@@ -2975,7 +2930,7 @@ async fn apply_group_changes(
{
send_event_chat_modified |= update_chats_contacts_timestamps(
context,
chat_id,
chat.id,
Some(from_id),
to_ids,
past_ids,
@@ -3015,7 +2970,7 @@ async fn apply_group_changes(
chat::update_chat_contacts_table(
context,
mime_parser.timestamp_sent,
chat_id,
chat.id,
&new_members,
)
.await?;
@@ -3023,7 +2978,7 @@ async fn apply_group_changes(
}
}
chat_id
chat.id
.update_timestamp(
context,
Param::MemberListTimestamp,
@@ -3033,7 +2988,7 @@ async fn apply_group_changes(
}
let new_chat_contacts = HashSet::<ContactId>::from_iter(
chat::get_chat_contacts(context, chat_id)
chat::get_chat_contacts(context, chat.id)
.await?
.iter()
.copied(),
@@ -3063,43 +3018,12 @@ async fn apply_group_changes(
let group_changes_msgs = if self_added {
Vec::new()
} else {
group_changes_msgs(context, &added_ids, &removed_ids, chat_id).await?
group_changes_msgs(context, &added_ids, &removed_ids, chat.id).await?
};
if let Some(avatar_action) = &mime_parser.group_avatar {
if !new_chat_contacts.contains(&ContactId::SELF) {
warn!(
context,
"Received group avatar update for group chat {chat_id} we are not a member of."
);
} else if !new_chat_contacts.contains(&from_id) {
warn!(
context,
"Contact {from_id} attempts to modify group chat {chat_id} avatar without being a member.",
);
} else {
info!(context, "Group-avatar change for {chat_id}.");
if chat
.param
.update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
{
match avatar_action {
AvatarAction::Change(profile_image) => {
chat.param.set(Param::ProfileImage, profile_image);
}
AvatarAction::Delete => {
chat.param.remove(Param::ProfileImage);
}
};
chat.update_param(context).await?;
send_event_chat_modified = true;
}
}
}
if send_event_chat_modified {
context.emit_event(EventType::ChatModified(chat_id));
chatlist_events::emit_chatlist_item_changed(context, chat_id);
context.emit_event(EventType::ChatModified(chat.id));
chatlist_events::emit_chatlist_item_changed(context, chat.id);
}
Ok(GroupChangesInfo {
better_msg,
@@ -3113,6 +3037,109 @@ async fn apply_group_changes(
})
}
/// Applies incoming changes to the group's or broadcast channel's name and avatar.
///
/// - `send_event_chat_modified` is set to `true` if ChatModified event should be sent
/// - `better_msg` is filled with an info message about name change, if necessary
async fn apply_chat_name_and_avatar_changes(
context: &Context,
mime_parser: &MimeMessage,
from_id: ContactId,
chat: &mut Chat,
send_event_chat_modified: &mut bool,
better_msg: &mut Option<String>,
) -> Result<()> {
// ========== Apply chat name changes ==========
let group_name_timestamp = mime_parser
.get_header(HeaderDef::ChatGroupNameTimestamp)
.and_then(|s| s.parse::<i64>().ok());
if let Some(old_name) = mime_parser
.get_header(HeaderDef::ChatGroupNameChanged)
.map(|s| s.trim())
.or(match group_name_timestamp {
Some(0) => None,
Some(_) => Some(chat.name.as_str()),
None => None,
})
{
if let Some(grpname) = mime_parser
.get_header(HeaderDef::ChatGroupName)
.map(|grpname| grpname.trim())
.filter(|grpname| grpname.len() < 200)
{
let grpname = &sanitize_single_line(grpname);
let chat_group_name_timestamp =
chat.param.get_i64(Param::GroupNameTimestamp).unwrap_or(0);
let group_name_timestamp = group_name_timestamp.unwrap_or(mime_parser.timestamp_sent);
// To provide group name consistency, compare names if timestamps are equal.
if (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
&& chat
.id
.update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp)
.await?
&& grpname != &chat.name
{
info!(context, "Updating grpname for chat {}.", chat.id);
context
.sql
.execute("UPDATE chats SET name=? WHERE id=?;", (grpname, chat.id))
.await?;
*send_event_chat_modified = true;
}
if mime_parser
.get_header(HeaderDef::ChatGroupNameChanged)
.is_some()
{
let old_name = &sanitize_single_line(old_name);
better_msg.get_or_insert(
stock_str::msg_grp_name(context, old_name, grpname, from_id).await,
);
}
}
}
// ========== Apply chat avatar changes ==========
if let (Some(value), None) = (mime_parser.get_header(HeaderDef::ChatContent), &better_msg) {
if value == "group-avatar-changed" {
if let Some(avatar_action) = &mime_parser.group_avatar {
// this is just an explicit message containing the group-avatar,
// apart from that, the group-avatar is send along with various other messages
better_msg.get_or_insert(match avatar_action {
AvatarAction::Delete => stock_str::msg_grp_img_deleted(context, from_id).await,
AvatarAction::Change(_) => {
stock_str::msg_grp_img_changed(context, from_id).await
}
});
}
}
}
if let Some(avatar_action) = &mime_parser.group_avatar {
info!(context, "Group-avatar change for {}.", chat.id);
if chat
.param
.update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
{
match avatar_action {
AvatarAction::Change(profile_image) => {
chat.param.set(Param::ProfileImage, profile_image);
}
AvatarAction::Delete => {
chat.param.remove(Param::ProfileImage);
}
};
chat.update_param(context).await?;
*send_event_chat_modified = true;
}
}
Ok(())
}
/// Returns a list of strings that should be shown as info messages, informing about group membership changes.
async fn group_changes_msgs(
context: &Context,
@@ -3165,7 +3192,7 @@ fn mailinglist_header_listid(list_id_header: &str) -> Result<String> {
.to_string())
}
/// Create or lookup a mailing list chat.
/// Create or lookup a mailing list or incoming broadcast channel chat.
///
/// `list_id_header` contains the Id that must be used for the mailing list
/// and has the form `Name <Id>`, `<Id>` or just `Id`.
@@ -3174,10 +3201,11 @@ fn mailinglist_header_listid(list_id_header: &str) -> Result<String> {
///
/// `mime_parser` is the corresponding message
/// and is used to figure out the mailing list name from different header fields.
async fn create_or_lookup_mailinglist(
async fn create_or_lookup_mailinglist_or_broadcast(
context: &Context,
allow_creation: bool,
list_id_header: &str,
from_id: ContactId,
mime_parser: &MimeMessage,
) -> Result<Option<(ChatId, Blocked)>> {
let listid = mailinglist_header_listid(list_id_header)?;
@@ -3186,7 +3214,19 @@ async fn create_or_lookup_mailinglist(
return Ok(Some((chat_id, blocked)));
}
let name = compute_mailinglist_name(list_id_header, &listid, mime_parser);
let chattype = if mime_parser.was_encrypted() {
Chattype::InBroadcast
} else {
Chattype::Mailinglist
};
let name = if chattype == Chattype::InBroadcast {
mime_parser
.get_header(HeaderDef::ChatGroupName)
.unwrap_or("Broadcast Channel")
} else {
&compute_mailinglist_name(list_id_header, &listid, mime_parser)
};
if allow_creation {
// list does not exist but should be created
@@ -3204,9 +3244,9 @@ async fn create_or_lookup_mailinglist(
};
let chat_id = ChatId::create_multiuser_record(
context,
Chattype::Mailinglist,
chattype,
&listid,
&name,
name,
blocked,
ProtectionStatus::Unprotected,
param,
@@ -3227,6 +3267,15 @@ async fn create_or_lookup_mailinglist(
&[ContactId::SELF],
)
.await?;
if chattype == Chattype::InBroadcast {
chat::add_to_chat_contacts_table(
context,
mime_parser.timestamp_sent,
chat_id,
&[from_id],
)
.await?;
}
Ok(Some((chat_id, blocked)))
} else {
info!(context, "Creating list forbidden by caller.");
@@ -3372,6 +3421,39 @@ async fn apply_mailinglist_changes(
Ok(())
}
async fn apply_broadcast_changes(
context: &Context,
mime_parser: &MimeMessage,
chat: &mut Chat,
from_id: ContactId,
) -> Result<GroupChangesInfo> {
ensure!(chat.typ == Chattype::InBroadcast);
let mut send_event_chat_modified = false;
let mut better_msg = None;
apply_chat_name_and_avatar_changes(
context,
mime_parser,
from_id,
chat,
&mut send_event_chat_modified,
&mut better_msg,
)
.await?;
if send_event_chat_modified {
context.emit_event(EventType::ChatModified(chat.id));
chatlist_events::emit_chatlist_item_changed(context, chat.id);
}
Ok(GroupChangesInfo {
better_msg,
added_removed_id: None,
silent: false,
extra_msgs: vec![],
})
}
/// Creates ad-hoc group and returns chat ID on success.
async fn create_adhoc_group(
context: &Context,