diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 04981bd2b..ea25b5979 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1246,7 +1246,7 @@ pub unsafe extern "C" fn dc_forward_msgs( let ffi_context = &*context; ffi_context .with_inner(|ctx| chat::forward_msgs(ctx, ids, chat_id)) - .unwrap_or(()) + .unwrap_or_log_default(ctx, "Failed to forard message") } #[no_mangle] diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 2c6a1c21f..b02e0dfe4 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -864,7 +864,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E let mut msg_ids = [0; 1]; let chat_id = arg2.parse()?; msg_ids[0] = arg1.parse()?; - chat::forward_msgs(context, &msg_ids, chat_id); + chat::forward_msgs(context, &msg_ids, chat_id)?; } "markseen" => { ensure!(!arg1.is_empty(), "Argument missing."); diff --git a/src/chat.rs b/src/chat.rs index b8c5f859d..166d2489f 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -784,10 +784,7 @@ pub fn send_msg(context: &Context, chat_id: u32, msg: &mut Message) -> Result Result<(), Error> { + ensure!(!msg_ids.is_empty(), "empty msgs_ids: no one to forward to"); + ensure!( + chat_id > DC_CHAT_ID_LAST_SPECIAL, + "can not forward to special chat" + ); let mut created_db_entries = Vec::new(); let mut curr_timestamp: i64; @@ -1790,7 +1789,7 @@ pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) { new_msg_id = chat .prepare_msg_raw(context, &mut msg, fresh10) .unwrap_or_default(); - job_send_msg(context, new_msg_id); + job_send_msg(context, new_msg_id)?; } created_db_entries.push(chat_id); created_db_entries.push(new_msg_id); @@ -1803,6 +1802,8 @@ pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) { msg_id: created_db_entries[i + 1], }); } + + Ok(()) } pub fn get_chat_contact_cnt(context: &Context, chat_id: u32) -> usize { diff --git a/src/job.rs b/src/job.rs index 339e18193..b76e13ab6 100644 --- a/src/job.rs +++ b/src/job.rs @@ -8,6 +8,7 @@ use crate::configure::*; use crate::constants::*; use crate::context::Context; use crate::dc_tools::*; +use crate::error::Error; use crate::events::Event; use crate::imap::*; use crate::imex::*; @@ -144,7 +145,7 @@ impl Job { .filter_map(|addr| match lettre::EmailAddress::new(addr.to_string()) { Ok(addr) => Some(addr), Err(err) => { - eprintln!("WARNING: invalid recipient: {} {:?}", addr, err); + warn!(context, "invalid recipient: {} {:?}", addr, err); None } }) @@ -156,21 +157,21 @@ impl Job { if 0 != self.foreign_id && !message::exists(context, self.foreign_id) { warn!( context, - "Message {} for job {} does not exist", self.foreign_id, self.job_id, + "Not sending Message {} as it was deleted", self.foreign_id ); return; }; // hold the smtp lock during sending of a job and // its ok/error response processing. Note that if a message - // was sent we need to mark it in the database as we + // was sent we need to mark it in the database ASAP as we // otherwise might send it twice. let mut sock = context.smtp.lock().unwrap(); - if 0 == sock.send(context, recipients_list, body) { + if !sock.send(context, recipients_list, body) { sock.disconnect(); self.try_again_later(-1i32, sock.error.clone()); } else { - dc_delete_file(context, filename); + // smtp success, update DB as quick as possible if 0 != self.foreign_id { message::update_msg_state( context, @@ -190,6 +191,8 @@ impl Job { msg_id: self.foreign_id, }); } + // now also delete the generated file + dc_delete_file(context, filename); } } else { warn!(context, "Missing recipients for job {}", self.job_id,); @@ -322,7 +325,12 @@ impl Job { self.try_again_later(3i32, None); } ImapResult::Success => { - send_mdn(context, msg.id); + if let Err(err) = send_mdn(context, msg.id) { + warn!( + context, + "could not send out mdn for {}: {}", msg.id, err + ); + } } ImapResult::Failed | ImapResult::AlreadyDone => {} } @@ -601,109 +609,101 @@ pub fn job_action_exists(context: &Context, action: Action) -> bool { /* special case for DC_JOB_SEND_MSG_TO_SMTP */ #[allow(non_snake_case)] -pub fn job_send_msg(context: &Context, msg_id: u32) -> libc::c_int { - let mut success = 0; +pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> { + let mut mimefactory = MimeFactory::load_msg(context, msg_id)?; - /* load message data */ - let mimefactory = MimeFactory::load_msg(context, msg_id); - if mimefactory.is_err() { - warn!( - context, - "Cannot load data to send, maybe the message is deleted in between.", - ); - } else { - let mut mimefactory = mimefactory.unwrap(); - // no redo, no IMAP. moreover, as the data does not exist, there is no need in calling dc_set_msg_failed() - if chat::msgtype_has_file(mimefactory.msg.type_0) { - let file_param = mimefactory - .msg - .param - .get(Param::File) - .map(|s| s.to_string()); - if let Some(pathNfilename) = file_param { - if (mimefactory.msg.type_0 == Viewtype::Image - || mimefactory.msg.type_0 == Viewtype::Gif) - && !mimefactory.msg.param.exists(Param::Width) - { - mimefactory.msg.param.set_int(Param::Width, 0); - mimefactory.msg.param.set_int(Param::Height, 0); + ensure!(!mimefactory.recipients_addr.is_empty(), + "msg {} has no recipients, can not smtp-sent", msg_id + ); - if let Ok(buf) = dc_read_file(context, pathNfilename) { - if let Ok((width, height)) = dc_get_filemeta(&buf) { - mimefactory.msg.param.set_int(Param::Width, width as i32); - mimefactory.msg.param.set_int(Param::Height, height as i32); - } - } - mimefactory.msg.save_param_to_disk(context); - } - } - } - /* create message */ - if let Err(msg) = unsafe { mimefactory.render() } { - let e = msg.to_string(); - message::set_msg_failed(context, msg_id, Some(e)); - } else if 0 - != mimefactory - .msg - .param - .get_int(Param::GuranteeE2ee) - .unwrap_or_default() - && !mimefactory.out_encrypted - { - /* unrecoverable */ - warn!( - context, - "e2e encryption unavailable {} - {:?}", - msg_id, - mimefactory.msg.param.get_int(Param::GuranteeE2ee), - ); - message::set_msg_failed( - context, - msg_id, - Some("End-to-end-encryption unavailable unexpectedly."), - ); - } else { - if !vec_contains_lowercase(&mimefactory.recipients_addr, &mimefactory.from_addr) { - mimefactory.recipients_names.push("".to_string()); - mimefactory - .recipients_addr - .push(mimefactory.from_addr.to_string()); - } - if mimefactory.out_gossiped { - chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time()); - } - if 0 != mimefactory.out_last_added_location_id { - if let Err(err) = - location::set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time()) - { - error!(context, "Failed to set kml sent_timestamp: {:?}", err); - } - if !mimefactory.msg.hidden { - if let Err(err) = location::set_msg_location_id( - context, - mimefactory.msg.id, - mimefactory.out_last_added_location_id, - ) { - error!(context, "Failed to set msg_location_id: {:?}", err); - } - } - } - if mimefactory.out_encrypted - && mimefactory - .msg - .param - .get_int(Param::GuranteeE2ee) - .unwrap_or_default() - == 0 + if chat::msgtype_has_file(mimefactory.msg.type_0) { + let file_param = mimefactory + .msg + .param + .get(Param::File) + .map(|s| s.to_string()); + if let Some(pathNfilename) = file_param { + if (mimefactory.msg.type_0 == Viewtype::Image + || mimefactory.msg.type_0 == Viewtype::Gif) + && !mimefactory.msg.param.exists(Param::Width) { - mimefactory.msg.param.set_int(Param::GuranteeE2ee, 1); + mimefactory.msg.param.set_int(Param::Width, 0); + mimefactory.msg.param.set_int(Param::Height, 0); + + if let Ok(buf) = dc_read_file(context, pathNfilename) { + if let Ok((width, height)) = dc_get_filemeta(&buf) { + mimefactory.msg.param.set_int(Param::Width, width as i32); + mimefactory.msg.param.set_int(Param::Height, height as i32); + } + } mimefactory.msg.save_param_to_disk(context); } - success = add_smtp_job(context, Action::SendMsgToSmtp, &mut mimefactory); } } - success + /* create message */ + if let Err(msg) = unsafe { mimefactory.render() } { + let e = msg.to_string(); + message::set_msg_failed(context, msg_id, Some(e)); + return Err(msg); + } + if 0 != mimefactory + .msg + .param + .get_int(Param::GuranteeE2ee) + .unwrap_or_default() + && !mimefactory.out_encrypted + { + /* unrecoverable */ + message::set_msg_failed( + context, + msg_id, + Some("End-to-end-encryption unavailable unexpectedly."), + ); + bail!( + "e2e encryption unavailable {} - {:?}", + msg_id, + mimefactory.msg.param.get_int(Param::GuranteeE2ee), + ); + } + if !vec_contains_lowercase(&mimefactory.recipients_addr, &mimefactory.from_addr) { + mimefactory.recipients_names.push("".to_string()); + mimefactory + .recipients_addr + .push(mimefactory.from_addr.to_string()); + } + if mimefactory.out_gossiped { + chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time()); + } + if 0 != mimefactory.out_last_added_location_id { + if let Err(err) = location::set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time()) + { + error!(context, "Failed to set kml sent_timestamp: {:?}", err); + } + if !mimefactory.msg.hidden { + if let Err(err) = location::set_msg_location_id( + context, + mimefactory.msg.id, + mimefactory.out_last_added_location_id, + ) { + error!(context, "Failed to set msg_location_id: {:?}", err); + } + } + } + if mimefactory.out_encrypted + && mimefactory + .msg + .param + .get_int(Param::GuranteeE2ee) + .unwrap_or_default() + == 0 + { + mimefactory.msg.param.set_int(Param::GuranteeE2ee, 1); + mimefactory.msg.save_param_to_disk(context); + } + add_smtp_job(context, Action::SendMsgToSmtp, &mut mimefactory)?; + + Ok(()) } pub fn perform_imap_jobs(context: &Context) { @@ -955,16 +955,20 @@ fn connect_to_inbox(context: &Context, inbox: &Imap) -> libc::c_int { ret_connected } -fn send_mdn(context: &Context, msg_id: u32) { - if let Ok(mut mimefactory) = MimeFactory::load_mdn(context, msg_id) { - if unsafe { mimefactory.render() }.is_ok() { - add_smtp_job(context, Action::SendMdn, &mut mimefactory); - } - } +fn send_mdn(context: &Context, msg_id: u32) -> Result<(), Error> { + let mut mimefactory = MimeFactory::load_mdn(context, msg_id)?; + unsafe { mimefactory.render()? }; + add_smtp_job(context, Action::SendMdn, &mut mimefactory)?; + + Ok(()) } #[allow(non_snake_case)] -fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> libc::c_int { +fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> Result<(), Error> { + ensure!( + !mimefactory.recipients_addr.is_empty(), + "no recipients for smtp job set" + ); let mut param = Params::new(); let bytes = unsafe { std::slice::from_raw_parts( @@ -972,16 +976,7 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> (*mimefactory.out).len, ) }; - let bpath = match context.new_blob_file(&mimefactory.rfc724_mid, bytes) { - Ok(path) => path, - Err(err) => { - error!( - context, - "Could not write {} smtp-message, error {}", mimefactory.rfc724_mid, err - ); - return 0; - } - }; + let bpath = context.new_blob_file(&mimefactory.rfc724_mid, bytes)?; info!(context, "add_smtp_job file written: {:?}", bpath); let recipients = mimefactory.recipients_addr.join("\x1e"); param.set(Param::File, &bpath); @@ -997,7 +992,8 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> param, 0, ); - 1 + + Ok(()) } pub fn job_add( diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 4b69118b3..85735f088 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -141,52 +141,40 @@ impl<'a> MimeFactory<'a> { } /******************************************************************************* - * Render + * Render a basic email ******************************************************************************/ // restrict unsafe to parts, introduce wrapmime helpers where appropriate pub unsafe fn render(&mut self) -> Result<(), Error> { if self.loaded == Loaded::Nothing || !self.out.is_null() { bail!("Invalid use of mimefactory-object."); } - let context = &self.context; - - /* create basic mail - *************************************************************************/ - - let from: *mut mailimf_mailbox_list = mailimf_mailbox_list_new_empty(); - mailimf_mailbox_list_add( - from, - mailimf_mailbox_new( - if !self.from_displayname.is_empty() { - dc_encode_header_words(&self.from_displayname).strdup() - } else { - ptr::null_mut() - }, - self.from_addr.strdup(), - ), + ensure!( + !self.recipients_names.is_empty() && !self.recipients_addr.is_empty(), + "message has no recipients" ); - let mut to: *mut mailimf_address_list = ptr::null_mut(); - if !self.recipients_names.is_empty() && !self.recipients_addr.is_empty() { - to = mailimf_address_list_new_empty(); - let name_iter = self.recipients_names.iter(); - let addr_iter = self.recipients_addr.iter(); - for (name, addr) in name_iter.zip(addr_iter) { - mailimf_address_list_add( - to, - mailimf_address_new( - MAILIMF_ADDRESS_MAILBOX as libc::c_int, - mailimf_mailbox_new( - if !name.is_empty() { - dc_encode_header_words(&name).strdup() - } else { - ptr::null_mut() - }, - addr.strdup(), - ), - ptr::null_mut(), + + let context = &self.context; + let from = wrapmime::new_mailbox_list(&self.from_displayname, &self.from_addr); + + let to = mailimf_address_list_new_empty(); + let name_iter = self.recipients_names.iter(); + let addr_iter = self.recipients_addr.iter(); + for (name, addr) in name_iter.zip(addr_iter) { + mailimf_address_list_add( + to, + mailimf_address_new( + MAILIMF_ADDRESS_MAILBOX as libc::c_int, + mailimf_mailbox_new( + if !name.is_empty() { + dc_encode_header_words(&name).strdup() + } else { + ptr::null_mut() + }, + addr.strdup(), ), - ); - } + ptr::null_mut(), + ), + ); } let references_list = if !self.references.is_empty() { dc_str_to_clist(&self.references, " ") @@ -198,6 +186,7 @@ impl<'a> MimeFactory<'a> { } else { ptr::null_mut() }; + let imf_fields = mailimf_fields_new_with_data_all( mailimf_get_date(self.timestamp as i64), from, diff --git a/src/smtp.rs b/src/smtp.rs index a4d8ad34a..f2287b749 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -125,18 +125,22 @@ impl Smtp { } } + /// SMTP-Send a prepared mail to recipients. + /// returns boolean whether send was successful. pub fn send<'a>( &mut self, context: &Context, recipients: Vec, - body: Vec, - ) -> usize { + message: Vec, + ) -> bool { + let message_len = message.len(); + if let Some(ref mut transport) = self.transport { let envelope = Envelope::new(self.from.clone(), recipients).expect("invalid envelope"); let mail = SendableEmail::new( envelope, "mail-id".into(), // TODO: random id - body, + message, ); match transport.send(mail) { @@ -145,17 +149,20 @@ impl Smtp { "Message was sent to SMTP server".into(), )); self.transport_connected = true; - 1 + true } Err(err) => { - warn!(context, "SMTP failed to send message: {}", err); + warn!( + context, + "SMTP failed message size {}, error: {}", message_len, err + ); self.error = Some(format!("{}", err)); - 0 + false } } } else { - // TODO: log error - 0 + warn!(context, "SMTP Failed to send message to {:?}", recipients); + false } } } diff --git a/src/wrapmime.rs b/src/wrapmime.rs index ae303f699..e0a3f5402 100644 --- a/src/wrapmime.rs +++ b/src/wrapmime.rs @@ -3,6 +3,7 @@ use std::ffi::CString; use std::ptr; use crate::contact::addr_normalize; +use crate::dc_strencode::*; use crate::dc_tools::*; use crate::error::Error; use mmime::clist::*; @@ -480,6 +481,24 @@ pub fn content_type_needs_encoding(content: *const mailmime_content) -> bool { } } +pub fn new_mailbox_list(displayname: &str, addr: &str) -> *mut mailimf_mailbox_list { + let mbox: *mut mailimf_mailbox_list = unsafe { mailimf_mailbox_list_new_empty() }; + unsafe { + mailimf_mailbox_list_add( + mbox, + mailimf_mailbox_new( + if !displayname.is_empty() { + dc_encode_header_words(&displayname).strdup() + } else { + ptr::null_mut() + }, + addr.strdup(), + ), + ); + } + mbox +} + #[cfg(test)] mod tests { use super::*;