Before they were trashed. Note that for unencrypted ones DC works as expected creating the requested
group immediately because Chat-Group-Id is duplicated in the Message-Id header and Subject is
fetched.
Also add a test on downloading a message later. Although it doesn't reproduce #4700 for some reason,
it fails w/o the fix because before a message state was changing to `InSeen` after a full download
which doesn't look correct. The result of a full message download should be such as if it was fully
downloaded initially.
9bd7ab72 brings a possibility of group membership inconsistency to the original Hocuri's algo
described and implemented in e12e026b in sake of security so that nobody can add themselves to a
group by forging "InReplyTo" and other headers. This commit fixes the problem by removing group
members locally if we see a discrepancy with the "To" list in the received message as it is better
for privacy than adding absent members locally. But it shouldn't be a big problem if somebody missed
a member addition, because they will likely recreate the member list from the next received
message. The problem occurs only if that "somebody" managed to reply earlier. Really, it's a problem
for big groups with high message rate, but let it be for now.
Also:
- Query chat contacts from the db only once.
- Update chat contacts in the only transaction, otherwise we can just break the chat contact list
halfway.
- Allow classic MUA messages to remove group members if a parent message is missing. Currently it
doesn't matter because unrelated messages go to new ad-hoc groups, but let this logic be outside
of apply_group_changes(). Just in case if there will be a MUA preserving "Chat-Group-ID" header
f.e.
Such a message may be assigned to a wrong chat (e.g. undecipherable group msgs often get assigned to
the 1:1 chat with the sender). Add `DownloadState::Undecipherable` so that messages referencing
undecipherable ones don't go to that wrong chat too. Also do not reply to not fully downloaded
messages. Before `Message.error` was checked for that purpose, but a message can be error for many
reasons.
It can be not good for membership consistency if we missed a message adding a member, but improves
security because nobody can add themselves to a group from now on.
1:1 chat may be blocked while the contact is not
if 1:1 chat was created as a result of scanning
a verified group join QR code with the contact
as the inviter. In this case 1:1 chat is blocked to hide it
while the contact is unblocked.
- If we don't know the parent (=In-Reply-To) message, then completely recreate the group member list
(i.e. use the member list of the incoming message) (because we assume that we missed some messages
& have a wrong group state).
- If the message has a "Chat-Group-Member-Removed: member@example.com" header, then remove this
member.
- If the message has a "Chat-Group-Member-Added: member@example.com" header, then add this member.
That means:
- Remove checks for the presense of `ContactId::SELF` in the group. Thus all recipients of a message
take the same decision about group membership changes, no matter if they are in the group
currently. This fixes a situation when a recipient thinks it's not a member because it missed a
message about its addition before.
NOTE: But always recreate membership list if SELF has been added. The older versions of DC don't
always set "In-Reply-To" to the latest message they sent, but to the latest delivered message (so
it's a race), so we need this heuristic currently.
- Recreate the group member list if we don't know the parent (=In-Reply-To) message, even if the
sender isn't in the group as per our view, because we missed some messages and our view may be
stale.
If a message is unsigned or signed with an unknown key, `MimeMessage::was_encrypted()` returns
false. So, it mustn't be checked when deciding whether to look into
`MimeMessage::decoded_data`. Looking through git history one can see that it's just a wrong check
left in the code for historical reasons.
Other MUAs don't set add/remove headers, so the only way for them to re-add us to the group is to
add us to To/CC/wherever. Previously it worked only for other members that are still in the group so
that they properly handled our re-addition, but we didn't.
Some Dovecot servers are configured
to alias "INBOX.DeltaChat" and "DeltaChat" to the same folder.
In this case Delta Chat moves new emails from "INBOX"
to "DeltaChat", but then discovers the same mail in "INBOX.DeltaChat"
and tries to move it to "DeltaChat" again.
Each time a MOVE command is issued to move the message
from "INBOX.DeltaChat" to "DeltaChat", the message gets a new UID.
To prevent such IMAP move loop between aliased folders,
we do not move the message if we have already seen it on IMAP,
i.e. we have its Message-ID in the `imap` table.
Note that we do not use `rfc724_mid_exists`,
because it checks the `msgs` table and would consider
BCC-self messages seen even before we see them in the Inbox,
preventing their move to the DeltaChat folder.
Duplicate messages and messages without Message-IDs
are not moved anymore, but this is better
than having an infinite move loop.
receive_imf() calls add_parts()
which INSERTs or UPDATEs the message using UPSERT [1].
It then uses last_insert_rowid() to get
the ID of the inserted message.
However, it is incorrect to use last_insert_rowid()
if an UPDATE was executed instead of INSERT.
The solution is to use `RETURNING id` clause
to make the UPSERT statement return message ID in any case [2].
The fix is tested in test_webxdc_update_for_not_downloaded_instance()
and with a debug_assert!.
[1] https://www.sqlite.org/lang_UPSERT.html
[2] https://sqlite.org/forum/forumpost/9ce3bc1c4a85c15f
This commit adds new stock strings
"I added member ...",
"I removed member ..." and
"I left the group" that are sent over the network
and are visible in classic MUAs like Thunderbird.
Member name in these messages uses authname
instead of the display name,
so the name set locally does not get leaked when
a member is added or removed.
New algorithm improves group consistency
in cases of missing messages,
restored old backups and replies from classic MUAs.
Co-authored-by: Hocuri <hocuri@gmx.de>
Co-authored-by: link2xt <link2xt@testrun.org>
Otherwise sending a message without plaintext part
resets the signature. It is particularly dangerous
in multidevice case, because it's easy to accidentally
reset the signature on your other device with a non-text message.
This patch adds new C APIs
dc_get_next_msgs() and dc_wait_next_msgs(),
and their JSON-RPC counterparts
get_next_msgs() and wait_next_msgs().
New configuration "last_msg_id"
tracks the last message ID processed by the bot.
get_next_msgs() returns message IDs above
the "last_msg_id".
wait_next_msgs() waits for new message notification
and calls get_next_msgs().
wait_next_msgs() can be used to build
a separate message processing loop
independent of the event loop.
Async Python API get_fresh_messages_in_arrival_order()
is deprecated in favor of get_next_messages().
Introduced Python APIs:
- Account.wait_next_incoming_message()
- Message.is_from_self()
- Message.is_from_device()
Introduced Rust APIs:
- Context.set_config_u32()
- Context.get_config_u32()
* Don't let blocking be bypassed using groups
Fix#4313
* Fix another bug: A blocked group was sometimes not unblocked when an unblocked contact sent a message into it.
Moved custom ToSql trait including Send + Sync from lib.rs to sql.rs.
Replaced most params! and paramsv! macro usage with tuples.
Replaced paramsv! and params_iterv! with params_slice!,
because there is no need to construct a vector.
This removes the message that needed to be supplied to LogExt::log_err
calls. This was from a time before we adopted anyhow and now we are
better off using anyhow::Context::context for the message: it is more
consistent, composes better and is less custom.
The benefit of the composition can be seen in the FFI calls which need
to both log the error as well as return it to the caller via
the set_last_error mechanism.
It also removes the LogExt::ok_or_log_msg funcion for the same reason,
the message is obsoleted by anyhow's context.
We do not make all transactions
[IMMEDIATE](https://www.sqlite.org/lang_transaction.html#deferred_immediate_and_exclusive_transactions)
for more parallelism -- at least read transactions can be made DEFERRED to run in parallel
w/o any drawbacks. But if we make write transactions DEFERRED also w/o any external locking,
then they are upgraded from read to write ones on the first write statement. This has some
drawbacks:
- If there are other write transactions, we block the thread and the db connection until
upgraded. Also if some reader comes then, it has to get next, less used connection with a
worse per-connection page cache.
- If a transaction is blocked for more than busy_timeout, it fails with SQLITE_BUSY.
- Configuring busy_timeout is not the best way to manage transaction timeouts, we would
prefer it to be integrated with Rust/tokio asyncs. Moreover, SQLite implements waiting
using sleeps.
- If upon a successful upgrade to a write transaction the db has been modified by another
one, the transaction has to be rolled back and retried. It is an extra work in terms of
CPU/battery.
- Maybe minor, but we lose some fairness in servicing write transactions, i.e. we service
them in the order of the first write statement, not in the order they come.
The only pro of making write transactions DEFERRED w/o the external locking is some
parallelism between them. Also we have an option to make write transactions IMMEDIATE, also
w/o the external locking. But then the most of cons above are still valid. Instead, if we
perform all write transactions under an async mutex, the only cons is losing some
parallelism for write transactions.
.call() interface is safer because it ensures
that blocking operations on SQL connection
are called within tokio::task::block_in_place().
Previously some code called blocking operations
in async context, e.g. add_parts() in receive_imf module.
The underlying implementation of .call()
can later be replaced with an implementation
that does not require block_in_place(),
e.g. a worker pool,
without changing the code using the .call() interface.
Gmail archives messages marked as `\Deleted` by default if those messages aren't in the Trash. But
if move them to the Trash instead, they will be auto-deleted in 30 days.
