Skip to content

Commit 3580e1c

Browse files
committed
midb: process deletions without delay
IMAP clients get really confused if a message is expunged but then reappears as a supposedly new message, but carrying an old UID. Suppose the state of the IMAP INBOX is: ``` a fetch 1:* (uid) * 1 FETCH (UID 2359) * 2 FETCH (UID 2360) * 3 FETCH (UID 2361) * 4 FETCH (UID 2362) * 5 FETCH (UID 2363) * 6 FETCH (UID 2364) * 7 FETCH (UID 2365) * 8 FETCH (UID 2366) * 9 FETCH (UID 2367) * 10 FETCH (UID 2368) * 10 EXISTS ``` Deleting 1-5 may produce a response like so: ``` ``` a store 1:5 +flags \deleted * 1 FETCH (FLAGS (\Deleted)) * 2 FETCH (FLAGS (\Deleted)) * 3 FETCH (FLAGS (\Deleted)) * 4 FETCH (FLAGS (\Deleted)) * 5 FETCH (FLAGS (\Deleted)) a OK STORE completed a expunge * 5 EXPUNGE #uid2363 * 4 EXPUNGE #uid2362 * 3 EXPUNGE #uid2361 * 2 EXPUNGE #uid2360 * 1 EXPUNGE #uid2359 * 8 EXISTS * 0 RECENT a OK EXPUNGE completed ``` After the EXPUNGE status lines, the IMAP client view is that INBOX contains five messages (uids 2364-2368). ``8 EXISTS`` then indicates that three new messages have come into existence. Per RFC 3501 §2.3.1.1, these must necessarily have higher UIDs >2368. This condition is violated as a result of midb's delayed M-DELE processing (the IMAP server's view is as follows): ``` a fetch 1:* (uid) * 1 FETCH (UID 2361) * 2 FETCH (UID 2362) * 3 FETCH (UID 2363) * 4 FETCH (UID 2364) * 5 FETCH (UID 2365) * 6 FETCH (UID 2366) * 7 FETCH (UID 2367) * 8 FETCH (UID 2368) ``` References: DESK-3220, DESK-3345
1 parent c1780c3 commit 3580e1c

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed

exch/midb/mail_engine.cpp

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2233,7 +2233,6 @@ static int me_minst(int argc, char **argv, int sockd) try
22332233
*/
22342234
static int me_mdele(int argc, char **argv, int sockd)
22352235
{
2236-
int i;
22372236
BOOL b_partial;
22382237
EID_ARRAY message_ids;
22392238

@@ -2244,17 +2243,25 @@ static int me_mdele(int argc, char **argv, int sockd)
22442243
auto pidb = me_get_idb(argv[1]);
22452244
if (pidb == nullptr)
22462245
return MIDB_E_HASHTABLE_FULL;
2246+
auto pidb_transact = gx_sql_begin(pidb->psqlite, txn_mode::write);
2247+
if (!pidb_transact)
2248+
return false;
22472249
auto folder_id = me_get_folder_id(pidb.get(), argv[2]);
22482250
if (folder_id == 0)
22492251
return MIDB_E_NO_FOLDER;
22502252
unsigned int user_id = 0;
22512253
if (!mysql_adaptor_get_user_ids(pidb->username.c_str(), &user_id, nullptr, nullptr))
22522254
return MIDB_E_SSGETID;
2255+
/*
2256+
* Modern SQLite (3.35) supports `DELETE FROM messages WHERE
2257+
* mid_string=? RETURNING message_id,folder_id`, but RHEL9 only has
2258+
* 3.34 still. So we need two separate queries.
2259+
*/
22532260
auto pstmt = gx_sql_prep(pidb->psqlite, "SELECT message_id,"
22542261
" folder_id FROM messages WHERE mid_string=?");
22552262
if (pstmt == nullptr)
22562263
return MIDB_E_SQLPREP;
2257-
for (i=3; i<argc; i++) {
2264+
for (int i = 3; i < argc; ++i) {
22582265
sqlite3_reset(pstmt);
22592266
sqlite3_bind_text(pstmt, 1, argv[i], -1, SQLITE_STATIC);
22602267
if (SQLITE_ROW != pstmt.step() ||
@@ -2263,8 +2270,36 @@ static int me_mdele(int argc, char **argv, int sockd)
22632270
message_ids.pids[message_ids.count++] =
22642271
rop_util_make_eid_ex(1, sqlite3_column_int64(pstmt, 0));
22652272
}
2273+
/*
2274+
* exmdb notifications may only arrive belatedly (or not at all),
2275+
* therefore we should explicitly drop the messages from midb.sqlite
2276+
* _now_, lest
2277+
*
2278+
* - the *same* midb client asking for a message listing
2279+
* after M-DELE could see messages again that really ought to be gone
2280+
* - the *same* IMAP client asking for a message listing after EXPUNGE
2281+
* could see messages with IMAPUIDs that were just deleted, which can
2282+
* upset clients.
2283+
*
2284+
* While resynchronization can still "bring back" those messages (and
2285+
* break midb client expectations), resync via RSYM/RSYF is considered
2286+
* a debugging tool, and resync-upon-first-open of midb.sqlite is when
2287+
* no clients are connected.
2288+
*/
2289+
pstmt = gx_sql_prep(pidb->psqlite, "DELETE FROM messages WHERE mid_string=?");
2290+
if (pstmt == nullptr)
2291+
return MIDB_E_SQLPREP;
2292+
for (int i = 3; i < argc; ++i) {
2293+
pstmt.reset();
2294+
pstmt.bind_text(1, argv[i]);
2295+
if (pstmt.step() != SQLITE_DONE)
2296+
/* ignore */;
2297+
}
22662298
pstmt.finalize();
2299+
if (pidb_transact.commit() != SQLITE_OK)
2300+
/* ignore */;
22672301
pidb.reset();
2302+
22682303
if (!exmdb_client->delete_messages(argv[1], CP_ACP, nullptr,
22692304
rop_util_make_eid_ex(1, folder_id), &message_ids, TRUE, &b_partial))
22702305
return MIDB_E_MDB_DELETEMESSAGES;

mra/imap/parser.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,11 @@ void imap_parser_echo_modify(imap_context *pcontext, STREAM *pstream)
12811281
hl_hold.unlock();
12821282

12831283
imap_parser_echo_expunges(*pcontext, pstream, f_expunged);
1284+
/*
1285+
* 3501 §7: "the server checks the mailbox for new messages as part of
1286+
* command execution. […] If new messages are found, the server sends
1287+
* untagged EXISTS and RECENT responses."
1288+
*/
12841289
if (pcontext->contents.refresh(*pcontext, pcontext->selected_folder,
12851290
f_expunged.size() > 0) == 0) {
12861291
auto outlen = gx_snprintf(buff, std::size(buff),

0 commit comments

Comments
 (0)