Fix #2335 (delete job flooding) (#2372)

* Fix #2335 (delete job flooding)

The problem was:

- You are offline, and an ephemeral message is due to delete.
- load_imap_deletion_job() is called and returns a deletion job for it.
- The job fails because, well, we are offline.
- The job is saved into the database.
- load_imap_deletion_job() is called again, and so on.

* Add test
This commit is contained in:
Hocuri
2021-04-22 16:37:22 +02:00
committed by GitHub
parent 7bdae8b2c5
commit a4486d8c30
2 changed files with 34 additions and 4 deletions

View File

@@ -66,7 +66,6 @@ use async_std::task;
use serde::{Deserialize, Serialize};
use sqlx::Row;
use crate::chat::{lookup_by_contact_id, send_msg, ChatId};
use crate::constants::{
Viewtype, DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_CONTACT_ID_DEVICE, DC_CONTACT_ID_SELF,
};
@@ -77,6 +76,10 @@ use crate::message::{Message, MessageState, MsgId};
use crate::mimeparser::SystemMessage;
use crate::sql;
use crate::stock_str;
use crate::{
chat::{lookup_by_contact_id, send_msg, ChatId},
job,
};
#[derive(Debug, PartialEq, Eq, Copy, Clone, Serialize, Deserialize)]
pub enum Timer {
@@ -490,10 +493,12 @@ pub(crate) async fn load_imap_deletion_msgid(context: &Context) -> sql::Result<O
OR (ephemeral_timestamp != 0 AND ephemeral_timestamp <= ?) \
) \
AND server_uid != 0 \
AND NOT id IN (SELECT foreign_id FROM jobs WHERE action = ?)
LIMIT 1",
)
.bind(threshold_timestamp)
.bind(now),
.bind(now)
.bind(job::Action::DeleteMsgOnImap),
)
.await?;
@@ -537,6 +542,7 @@ pub(crate) async fn start_ephemeral_timers(context: &Context) -> sql::Result<()>
#[cfg(test)]
mod tests {
use crate::param::Params;
use async_std::task::sleep;
use super::*;
@@ -758,7 +764,31 @@ mod tests {
sleep(Duration::from_millis(1100)).await;
// Check checks that the msg was deleted locally
check_msg_was_deleted(&t, &chat, msg.sender_msg_id).await;
// Check that the msg will be deleted on the server
// First of all, set a server_uid so that DC thinks that it's actually possible to delete
t.sql
.execute(sqlx::query("UPDATE msgs SET server_uid=1 WHERE id=?").bind(msg.sender_msg_id))
.await
.unwrap();
let job = job::load_imap_deletion_job(&t).await.unwrap();
assert_eq!(
job,
Some(job::Job::new(
job::Action::DeleteMsgOnImap,
msg.sender_msg_id.to_u32(),
Params::new(),
0,
))
);
// Let's assume that executing the job fails on first try and the job is saved to the db
job.unwrap().save(&t).await.unwrap();
// Make sure that we don't get yet another job when loading from db
let job2 = job::load_imap_deletion_job(&t).await.unwrap();
assert_eq!(job2, None);
}
async fn check_msg_was_deleted(t: &TestContext, chat: &Chat, msg_id: MsgId) {

View File

@@ -130,7 +130,7 @@ impl From<Action> for Thread {
}
}
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct Job {
pub job_id: u32,
pub action: Action,
@@ -1030,7 +1030,7 @@ pub(crate) enum Connection<'a> {
Smtp(&'a mut Smtp),
}
async fn load_imap_deletion_job(context: &Context) -> sql::Result<Option<Job>> {
pub(crate) async fn load_imap_deletion_job(context: &Context) -> sql::Result<Option<Job>> {
let res = if let Some(msg_id) = load_imap_deletion_msgid(context).await? {
info!(context, "verbose (issue 2335): loading imap deletion job");
Some(Job::new(