Not clear why `GROUP BY` was needed -- a chat can't appear in the selected rows multiple times
because we do JOIN on the last message in the chat, there can't be multiple last messages.
This speeds up chatlisting by 3--4 times on my biggest profile, so the difference is visually
noticeable, particularly after dropping disk caches. Before, `msgs_index2` was used which has less
ordering and requires a full scan of `msgs` table.
These messages are AUTH-encrypted and shalln't contain profile data as per the SecureJoin v3 spec,
otherwise the profile data can be seen e.g. by other broadcast subscribers.
An Iroh-Node-Addr message (realtime advertisement) may arrive earlier than the referenced webxdc in
case of multi-transport. By not creating a tombstone we still can receive it later on another
transport.
This way they also can be processed by `markseen_msgs()` resulting in self-MDNs which improves
multi-device synchronization. MDNs to contacts won't really be sent because silent group changes
messages are self-removal messages which don't request MDNs.
Otherwise it's not possible to see the full message text if it's too long in a received edit. On the
sender side let it still be text so that the message can be edited again.
Maybe it's even good that MDNs will be sent for pre-messages only having placeholder text with the
viewtype and size, at least this way we notify the contact that we've noticed the message. Anyway,
with adding message previews the problem will be solved for the corresponding viewtypes.
- Ignore "vc-request-with-auth" if the sender is blocked.
- Ignore SecureJoin messages on an observing device if the joiner is blocked. Even if it's a
"vc-contact-confirm" message, it might be issued by another device before the joiner was blocked
on the observing device. Still, to avoid membership inconsistency on devices, don't ignore
"vg-member-added".
- Ignore "vc-request-pubkey" if the sender is blocked by address. We don't know sender's key yet, so
we should handle the sender as an address contact at this point.
The sender can generate a new key and still join, but it's at least an extra work and also, in case
of a group, other members will see that there's a new unknown member and be more careful.
I.e. don't fail `remove_contact_from_chat()` for such a contact because there may be a race
condition with a remote removal of the contact done without trace, but don't send a removal message
and sync message in this case and don't emit a `ChatModified` event.
If a contact is already a past member, we still send a removal message to the chat, this is safe and
protects from lost removal messages, so there's no need to complicate the code in this case.
Follow-up to 970222f. We shouldn't update the message state to `OutDelivered` if message sending
failed before and the message was re-sent successfully only to a new broadcast member.
`OutPreparing` is deprecated since 2024-12-07, replace it with `OutFailed`, such messages are
probably not interesting anymore. `OutMdnRcvd` is not used in the db for new messages since
a30c6ae1f7, `OutDelivered` is stored instead.
If we missed a message changing the primary transport, we shouldn't remove it when applying
a SyncTransports message, such a state doesn't look correct even if it's temporary.
We shouldn't just revert ad7f873c68 because then we won't be able to
normally transfer a backup from a device having clock a bit in the future.
INDEXED BY msgs_index7 is to prevent SQLite from downgrading to using msgs_index2 which has less
ordering.
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.
This makes such contacts appear in the contact list. `IncomingTo` is used because
`ChatId::accept_ex()` does so for groups, so sending a group message is effectively accepting the
group again in regards to contact searchability.
This fixes up b549e7633d which made it impossible to find contacts
from groups even if we've written there.
rfc724_mid_exists() returns the message id not only when the message is fully downloaded, so we need
to check this additionally. Also improve test_receive_pre_message to check that receiving the
pre-message again doesn't affect the existing message.
Before, if the user fixed their clock incorrectly set to the future, they needed to delete
previously sent messages or wait until this future comes again so that new sent messages are added
to the bottom. Strictly speaking, the problem isn't fixable because we don't know if messages were
incorrectly timestamped into the future or they are timestamped correctly and the clock is now
incorrectly set to the past. Anyway, adding messages to the middle of the chat isn't a good way to
inform the user about the problem.
We don't want to send an outgoing message from the 2nd device (`ac1_clone`) before receiving the
incoming one and expect that the messages will be ordered correctly on the 1st device (`ac1`). Let's
ensure the correct message order locally in the first place. I checked logs of a failed test run and
it indeed happened that `ac1_clone` sent the message earlier, so it can't reference the incoming
message and `tweak_sort_timestamp()` does nothing on `ac1`, so the messages can't be ordered
correctly considering that smeared clocks on the devices are diferent.
The part of logic there adjusting the sort timestamp forward if the parent message has a greater
sort timestamp wasn't tested explicitly by any test. I only saw one unrelated "golden test" failure
when commented it out.
(Related to #8027)
The original idea was to always lock `write_mutex` before acquiring an `InnerPool.semaphore` permit
to avoid ABBA deadlocks, but when refactoring a PR for b696a242fc,
that was forgotten.
This doesn't really change the program flow as we have `Context::housekeeping_mutex` anyway,
just simplifies the code.
If one of broadcast owner's devices didn't add a new subscriber for any reason, e.g. because of
missing SecureJoin messages, this device shall add "member added" messages when syncing the member
list from the `SetPgpContacts` message.
Timestamp renewal was introduced in 1dbf924c6a "feat:
chat::resend_msgs: Guarantee strictly increasing time in the Date header" so that re-sent messages
can be deduplicated on the reciver side, but the deduplication logic doesn't depend on "Date"
anymore.
The webxdc file name itself isn't informative for users. Still, send and display it if the webxdc
manifest can't be parsed, it's better than sending "Mini App" and this isn't a normal case anyway.
This includes forwarding of long messages. Also this fixes sending, but more likely resending of
forwarded messages for which the original message was deleted, because now we save HTML to the db
immediately when creating a forwarded message.
Co-authored-by: Hocuri <hocuri@gmx.de>
It has a really complex logic, so it's better to avoid calling it if possible than think which side
effects and performance penalties it has. It was never called here before adding forwarding messages
across contexts (accounts).
For now, do this only for `OneOneChat` and `MailingListOrBroadcast`, this is enough to correctly
support messages from modern Delta Chat versions sending Intended Recipient Fingerprint subpackets
and single-recipient messages from modern versions of other MUAs.
It only looks up contacts by address in the given chat, so `_fallback_to_chat` suffix is more
informative. It's obvious that such a lookup is done by address, there are no other reasonable
options.
This is an important thing forgotten to be checked in 332527089.
Also there's another test which currently doesn't work as we want: outgoing encrypted messages
continue to arrive to ad-hoc group even if we already have contact's key. This should be fixed by
sending and receiving Intended Recipient Fingerprint subpackets.
The good thing is that apparently there are no scenarios requiring the contact to update their
software, the user should just update own devices.
This is not possible for webxdcs and vCards currently however, so add workarounds for them:
- Use translated "Mini App" as the webxdc name.
- Use just "👤" instead of the vCard summary (i.e. the vCard contact name).
I.e. create a non-replyable ad-hoc group in such cases. Unencrypted replies to encrypted messages
are a security issue and "Composing a Reply Message" from RFC 9787 and "Replying and Forwarding
Guidance" from RFC 9788 forbid such replies.
If From is an address-contact, it mustn't be able to modify an encrypted group. If From is a
key-contact, it mustn't be added to members of an unencrypted group.
Otherwise if the user reads messages being offline and then the device comes online, sent MDNs will
remove all notifications from other devices even if new messages have arrived. Notifications not
removed at all look more acceptable.
We currently synchronize "seen" status of messages by setting `\Seen` flag on IMAP and then looking
for new `\Seen` flags using `CONDSTORE` IMAP extension. This approach has multiple disadvantages:
- It requires that the server supports `CONDSTORE` extension. For example Maddy does not support
CONDSTORE yet: https://github.com/foxcpp/maddy/issues/727
- It leaks the seen status to the server without any encryption.
- It requires more than just store-and-forward queues and prevents replacing IMAP with simpler
protocols like POP3 or UUCP or some HTTP-based API for queue polling.
A simpler approach is to send MDNs to self when `Config::BccSelf` (aka multidevice) is enabled,
regardless of whether the message requested and MDN. If MDN was requested and we have MDNs enabled,
then also send to the message sender, but MDN to self is sent regardless of whether read receipts
are actually enabled.
`sync_seen_flags()` and `CONDSTORE` check is better completely removed, maybe after one
release. `store_seen_flags_on_imap()` can be kept for unencrypted non-chat messages.
One potential problem with sending MDNs is that it may trigger ratelimits on some providers and
count as another recipient.
84161f4202 promotes group members to `Origin::IncomingTo` when
accepting it, instead of `CreateChat` as before, but this changes almost nothing because it happens
rarely that the user only accepts a group and writes nothing there soon. Now if a message has
multiple recipients, i.e. it's a 3-or-more-member group, or if it's a broadcast message, we don't
scale up its recipients to `Origin::OutgoingTo`.
Also remove "you can now call 'configure'" from the REPL output, probably users of the REPL tool can
read the code documentation to know when 'configure' should be run.
Otherwise it's not possible to write tests reliably because sync messages may be executed multiple
times if they arrive from different transports. This should fix flaky
`test_transport_synchronization`.
Also always emit `TransportsModified` if the primary transport is changed by a sync message, even if
it doesn't contain `SyncData::Transports`.
Also don't decrease `add_timestamp` in `save_transport()` if nothing else changes, this doesn't make
sense.
This way, if the sync message updates transports, the check for a new primary transport is done
against the updated transport list which is more reliable.
This fixes the bug when a new transport doesn't become primary on the 2nd device because INBOX from
the new transport isn't fully fetched. Now the `Transports` sync message is received from the old
transport, but as it has updated "From", it updates the primary transport correspondingly. NB: I/O
for the new primary transport isn't immediately started however, this needs a separate fix.
This is to fix tests failing with `OSError: [Errno 9] Bad file descriptor`. Maybe stdout closes
earlier than stderr, before the test finishes, not sure. For reference, the previous commit removing
print()s is 800edc6fce.
There was a comment that group messages should always be downloaded to avoid inconsistent group
state, but this is solved by the group consistency algo nowadays in the sense that inconsistent
group state won't spread to other members if we send to the group.
`Config::OnlyFetchMvbox` should be checked before `MvboxMove` because the latter makes no sense in
presense of `OnlyFetchMvbox` and even grayed out in the UIs in this case. Otherwise users will see
an error mentioning the wrong setting.
When accepting a chat, its members are promoted to `Origin::CreateChat`, but for groups it makes
sense to use lower origin because users don't always check all members before accepting a chat and
may not want to have the group members mixed with existing contacts. `IncomingTo` fits here by its
definition: "additional To:'s of incoming message of known sender", i.e. we assume that the sender
of some message is known to the user. This way we can show contacts coming from groups in the bottom
of contact list, maybe even add some separator later. It makes sense not to hide such contacts
completely, otherwise if the user remembers the contact name, but not the chat it's a member of, it
would be difficult to find the contact.
This records the curent behavior: after accepting a group with unknown contacts the contact list
contains them mixed with contacts for which 1:1 chats exist, i.e. new contacts from the group are
neither hidden nor sorted down.
All users of this function expect a nonempty list to be returned, otherwise they fail with hard to
understand errors like "No connection attempts were made", so it's better to fail early if DNS
resolution failed and the cache doesn't contain resolutions as well.
And don't add a `SecurejoinWait` info message at all if we know Alice's key from the start. If we
don't remove this info message, it appears in the chat after "Messages are end-to-end encrypted..."
which is quite confusing when Bob can already send messages to Alice.
Add `chat::forward_msgs_2ctx()` which takes another context as a parameter and forwards messages to
it and its jsonrpc wrapper `CommandApi::forward_messages_to_account()`.
We periodically forget to remove new params from forwarded messages as this can't be catched by
existing tests, some examples:
bfc08abe88a1837aeb8c56b2361f01
This may leak confidential data. Instead, it's better to explicitly list params that we want to
forward, then if we forget to forward some param, a test on forwarding messages carrying the new
functionality will break, or the bug will be reported earlier, it's easier to notice that some info
is missing than some extra info is leaked.
This makes `Contact::get_all()` and `Chatlist::try_load()` case-insensitive for non-ASCII chat and
contact names as well. The same approach as in f6f4ccc6ea "feat:
Case-insensitive search for non-ASCII messages (#5052)" is used: `chats.name_normalized` and
`contacts.name_normalized` colums are added which store lowercased/normalized names (for a contact,
if the name is unset, it's a normalized authname). If a normalized name is the same as the
chat/contact name, it's not stored to reduce the db size. A db migration is added for 10000 random
chats and the same number of the most recently seen contacts, for users it will probably migrate all
chats/contacts and for bots which may have more data it's not important.
Many clients don't send it currently, so it is unlikely that servers depend on it:
https://mastodon.social/@cks/114690055923939576.
For "implicit TLS", do not turn it off yet, it will serve as a fallback in case of rare server that
needs it. If the server only supports STARTTLS and requires SNI then it is really weird, likely
should not happen.
This effectively readds the old `imap_rfc724_mid` built only on `rfc724_mid`, otherwise
`sql::prune_tombstones()` which is called from `housekeeping()` becomes very slow because of no
suitable index.
`chat::set_chat_profile_image()` already checks that the group has grpid, still it makes sense to
check that a message is encrypted when sending, in case if the chat has a profile image in the db
for some reason.
iequidoo
