feat: Send webxdc status updates in InBroadcast-s (#7679) #7836
Conversation
iequidoo
force-pushed
the
iequidoo/broadcast-webxdc-updates
branch
from
adaf527 to
c6600b5
Compare
Broadcast subscribers can't change the chat name, so sending the "Chat-Group-Name{,-Timestamp}"
headers looks unnecessary. That could be useful for other subscriber's devices, but having only the
chat name isn't enough anyway, at least knowing the secret is necessary which is sent by the
broadcast owner.
They are already applied by the broadcast owner and other subscriber's devices, no changes are needed on the receiver side.
iequidoo
force-pushed
the
iequidoo/broadcast-webxdc-updates
branch
from
c6600b5 to
aed8d8c
Compare
We don't remember which status updates are "initial" (sent initially by the broadcast owner) and which were sent by subscribers, so don't resend any of them to avoid accidental sharing of confidential data.
| /// A 1:1 chat, i.e. a normal chat with a single contact. | ||
| /// | ||
| /// Created by [`ChatId::create_for_contact`]. | ||
| #[default] |
There was a problem hiding this comment.
This is to avoid adding Option<Chattype> to Message. We don't really have messages ouside of chats in the db.
Hocuri
left a comment
Hocuri
left a comment
There was a problem hiding this comment.
Thanks for implementing!
Not sure about d250e70, because it means that when a Webxdc is resent in a channel, its state will never show for new subscribers. And I don't think that it's a problem that subscribers might send confidential status updates into a channel.
OTOH, it could be weird that the channel owner relays webxdc updates only if clicking "Resend" on the message; could lead to bugs.
| if let Some(reason) = chat.why_cant_send_ex(context, &skip_fn).await? { | ||
| if msg.param.get_cmd() == SystemMessage::WebxdcStatusUpdate { | ||
| // Already checked in `send_webxdc_status_update_struct()`. | ||
| } else if let Some(reason) = chat.why_cant_send_ex(context, &skip_fn).await? { |
There was a problem hiding this comment.
Why not just add | SystemMessage::WebxdcStatusUpdate to this code above:
CantSendReason::InBroadcast => {
matches!(
msg.param.get_cmd(),
SystemMessage::MemberRemovedFromGroup | SystemMessage::SecurejoinMessage
)
}
?
Then the let skip_fn = |reason: &CantSendReason| *reason == CantSendReason::InBroadcast; change in webxdc.rs also wouldn't be necessary.
| // This is safe: see `ChatId::delete_ex()`, `None` chat type can't happen. | ||
| chat_typ: row | ||
| .get::<_, Option<_>>("chat_typ")? | ||
| .unwrap_or(Chattype::Single), |
There was a problem hiding this comment.
I don't think it makes sense to load all kind of chat data every time we load a message. chat_visibility and chat_blocked also has only one usage each in the entire codebase (tests excluded), both of which could be easily removed.
Also, chat_typ is only needed when (re)sending messages, which doesn't happen as often as receiving messages.
| assert_eq!(bob_instance.chat_id, bob_chat_id); | ||
|
|
||
| let provider = BackupProvider::prepare(bob).await?; | ||
| let bob1 = &tcm.unconfigured().await; |
There was a problem hiding this comment.
Should be called bob2 for consistency with other tests.
FTR, it would be possible to get a second Bob by calling &tcm.bob().await again, but the way done here is fine, too.
| .await?; | ||
| bob.flush_status_updates().await?; | ||
| let sent_msg = bob.pop_sent_msg().await; | ||
| let sent_msg = bob.pop_sent_msg_opt(Duration::ZERO).await.unwrap(); |
| StatusUpdateSerial::MAX, | ||
| None, | ||
| ) | ||
| .await? |
There was a problem hiding this comment.
I'm not very used to the webxdc code, but: Doesn't this change mean that no status updates will be sent into OutBroadcasts at all anymore? And, isn't the change in src/chat.rs enough to prevent resending into OutBroadcasts?
2 participants
They are already applied by the broadcast owner and other subscriber's devices, no changes are
needed on the receiver side.
Checked by hand as well. Close #7679
Also removed sending "Chat-Group-Name*" headers by broadcast subscribers, it looks unnecessary at first glance, see the commit message.