refactor rfc724_mid parsing and creation to avoid char*, add tests

This commit is contained in:
holger krekel
2019-09-19 16:38:47 +02:00
parent 89eef0158d
commit 724ba6776e
4 changed files with 88 additions and 58 deletions

View File

@@ -995,6 +995,19 @@ impl<'a> MimeParser<'a> {
assert_eq!(self.parts.len(), 1); assert_eq!(self.parts.len(), 1);
} }
pub fn get_rfc724_mid(&mut self) -> Option<String> {
// get Message-ID from header
if let Some(field) = self.lookup_field_typ("Message-ID", MAILIMF_FIELD_MESSAGE_ID) {
unsafe {
let fld_message_id = (*field).fld_data.fld_message_id;
if !fld_message_id.is_null() {
return Some(as_str((*fld_message_id).mid_value).to_string());
}
}
}
None
}
} }
impl<'a> Drop for MimeParser<'a> { impl<'a> Drop for MimeParser<'a> {
@@ -1641,6 +1654,27 @@ mod tests {
} }
} }
#[test]
fn test_get_rfc724_mid_exists() {
let context = dummy_context();
let raw = include_bytes!("../test-data/message/mail_with_message_id.txt");
let mut mimeparser = MimeParser::new(&context.ctx);
unsafe { mimeparser.parse(&raw[..]) };
assert_eq!(
mimeparser.get_rfc724_mid(),
Some("2dfdbde7@example.org".into())
);
}
#[test]
fn test_get_rfc724_mid_not_exists() {
let context = dummy_context();
let raw = include_bytes!("../test-data/message/issue_523.txt");
let mut mimeparser = MimeParser::new(&context.ctx);
unsafe { mimeparser.parse(&raw[..]) };
assert_eq!(mimeparser.get_rfc724_mid(), None);
}
#[test] #[test]
fn test_mimeparser_with_context() { fn test_mimeparser_with_context() {
unsafe { unsafe {

View File

@@ -85,8 +85,6 @@ pub unsafe fn dc_receive_imf(
let mut add_delete_job: libc::c_int = 0; let mut add_delete_job: libc::c_int = 0;
let mut insert_msg_id = 0; let mut insert_msg_id = 0;
// Message-ID from the header
let rfc724_mid = std::ptr::null_mut();
let mut sent_timestamp = 0; let mut sent_timestamp = 0;
let mut created_db_entries = Vec::new(); let mut created_db_entries = Vec::new();
let mut create_event_to_send = Some(CreateEvent::MsgsChanged); let mut create_event_to_send = Some(CreateEvent::MsgsChanged);
@@ -96,12 +94,9 @@ pub unsafe fn dc_receive_imf(
// helper method to handle early exit and memory cleanup // helper method to handle early exit and memory cleanup
let cleanup = |context: &Context, let cleanup = |context: &Context,
rfc724_mid: *mut libc::c_char,
create_event_to_send: &Option<CreateEvent>, create_event_to_send: &Option<CreateEvent>,
created_db_entries: &Vec<(usize, usize)>, created_db_entries: &Vec<(usize, usize)>,
rr_event_to_send: &Vec<(u32, u32)>| { rr_event_to_send: &Vec<(u32, u32)>| {
free(rfc724_mid.cast());
if let Some(create_event_to_send) = create_event_to_send { if let Some(create_event_to_send) = create_event_to_send {
for (chat_id, msg_id) in created_db_entries { for (chat_id, msg_id) in created_db_entries {
let event = match create_event_to_send { let event = match create_event_to_send {
@@ -184,6 +179,28 @@ pub unsafe fn dc_receive_imf(
} }
// Add parts // Add parts
let rfc724_mid = match mime_parser.get_rfc724_mid() {
Some(x) => x,
None => {
// missing Message-IDs may come if the mail was set from this account with another
// client that relies in the SMTP server to generate one.
// true eg. for the Webmailer used in all-inkl-KAS
match dc_create_incoming_rfc724_mid(sent_timestamp, from_id, &to_ids) {
Some(x) => x.to_string(),
None => {
error!(context, "can not create incoming rfc724_mid");
cleanup(
context,
&create_event_to_send,
&created_db_entries,
&rr_event_to_send,
);
return;
}
}
}
};
if mime_parser.get_last_nonmeta().is_some() { if mime_parser.get_last_nonmeta().is_some() {
if let Err(err) = add_parts( if let Err(err) = add_parts(
context, context,
@@ -195,7 +212,7 @@ pub unsafe fn dc_receive_imf(
server_folder.as_ref(), server_folder.as_ref(),
server_uid, server_uid,
&mut to_ids, &mut to_ids,
rfc724_mid, &rfc724_mid,
&mut sent_timestamp, &mut sent_timestamp,
&mut from_id, &mut from_id,
from_id_blocked, from_id_blocked,
@@ -213,7 +230,6 @@ pub unsafe fn dc_receive_imf(
cleanup( cleanup(
context, context,
rfc724_mid,
&create_event_to_send, &create_event_to_send,
&created_db_entries, &created_db_entries,
&rr_event_to_send, &rr_event_to_send,
@@ -263,14 +279,11 @@ pub unsafe fn dc_receive_imf(
info!( info!(
context, context,
"received message {} has Message-Id: {}", "received message {} has Message-Id: {}", server_uid, rfc724_mid
server_uid,
to_string(rfc724_mid)
); );
cleanup( cleanup(
context, context,
rfc724_mid,
&create_event_to_send, &create_event_to_send,
&created_db_entries, &created_db_entries,
&rr_event_to_send, &rr_event_to_send,
@@ -287,7 +300,7 @@ unsafe fn add_parts(
server_folder: impl AsRef<str>, server_folder: impl AsRef<str>,
server_uid: u32, server_uid: u32,
to_ids: &mut Vec<u32>, to_ids: &mut Vec<u32>,
mut rfc724_mid: *mut libc::c_char, rfc724_mid: &str,
sent_timestamp: &mut i64, sent_timestamp: &mut i64,
from_id: &mut u32, from_id: &mut u32,
from_id_blocked: i32, from_id_blocked: i32,
@@ -336,40 +349,20 @@ unsafe fn add_parts(
} }
} }
// get Message-ID; if the header is lacking one, generate one based on fields that do never
// change. (missing Message-IDs may come if the mail was set from this account with another
// client that relies in the SMTP server to generate one.
// true eg. for the Webmailer used in all-inkl-KAS)
if let Some(field) = mime_parser.lookup_field_typ("Message-ID", MAILIMF_FIELD_MESSAGE_ID) {
let fld_message_id = (*field).fld_data.fld_message_id;
if !fld_message_id.is_null() {
rfc724_mid = dc_strdup((*fld_message_id).mid_value)
}
}
if rfc724_mid.is_null() {
rfc724_mid = dc_create_incoming_rfc724_mid(*sent_timestamp, *from_id, to_ids);
if rfc724_mid.is_null() {
cleanup(mime_in_reply_to, mime_references);
bail!("Cannot create Message-ID");
}
}
// check, if the mail is already in our database - if so, just update the folder/uid // check, if the mail is already in our database - if so, just update the folder/uid
// (if the mail was moved around) and finish. (we may get a mail twice eg. if it is // (if the mail was moved around) and finish. (we may get a mail twice eg. if it is
// moved between folders. make sure, this check is done eg. before securejoin-processing) */ // moved between folders. make sure, this check is done eg. before securejoin-processing) */
let mut old_server_folder = std::ptr::null_mut(); let mut old_server_folder = std::ptr::null_mut();
let mut old_server_uid = 0; let mut old_server_uid = 0;
let rfc724_mid_s = as_str(rfc724_mid);
if 0 != dc_rfc724_mid_exists( if 0 != dc_rfc724_mid_exists(
context, context,
&rfc724_mid_s, &rfc724_mid,
&mut old_server_folder, &mut old_server_folder,
&mut old_server_uid, &mut old_server_uid,
) { ) {
if as_str(old_server_folder) != server_folder.as_ref() || old_server_uid != server_uid { if as_str(old_server_folder) != server_folder.as_ref() || old_server_uid != server_uid {
dc_update_server_uid(context, &rfc724_mid_s, server_folder.as_ref(), server_uid); dc_update_server_uid(context, &rfc724_mid, server_folder.as_ref(), server_uid);
} }
free(old_server_folder.cast()); free(old_server_folder.cast());
@@ -668,7 +661,7 @@ unsafe fn add_parts(
} }
stmt.execute(params![ stmt.execute(params![
as_str(rfc724_mid), rfc724_mid,
server_folder.as_ref(), server_folder.as_ref(),
server_uid as libc::c_int, server_uid as libc::c_int,
*chat_id as libc::c_int, *chat_id as libc::c_int,
@@ -699,13 +692,8 @@ unsafe fn add_parts(
])?; ])?;
txt_raw = None; txt_raw = None;
*insert_msg_id = sql::get_rowid_with_conn( *insert_msg_id =
context, sql::get_rowid_with_conn(context, conn, "msgs", "rfc724_mid", &rfc724_mid);
conn,
"msgs",
"rfc724_mid",
as_str(rfc724_mid),
);
created_db_entries.push((*chat_id as usize, *insert_msg_id as usize)); created_db_entries.push((*chat_id as usize, *insert_msg_id as usize));
} }
Ok(()) Ok(())
@@ -1871,11 +1859,11 @@ unsafe fn is_msgrmsg_rfc724_mid_in_list(context: &Context, mid_list: *const clis
while !cur.is_null() { while !cur.is_null() {
if 0 != is_msgrmsg_rfc724_mid( if 0 != is_msgrmsg_rfc724_mid(
context, context,
(if !cur.is_null() { if !cur.is_null() {
(*cur).data as_str((*cur).data as *const libc::c_char)
} else { } else {
ptr::null_mut() ""
}) as *const libc::c_char, },
) { ) {
return 1; return 1;
} }
@@ -1890,15 +1878,15 @@ unsafe fn is_msgrmsg_rfc724_mid_in_list(context: &Context, mid_list: *const clis
} }
/// Check if a message is a reply to any messenger message. /// Check if a message is a reply to any messenger message.
fn is_msgrmsg_rfc724_mid(context: &Context, rfc724_mid: *const libc::c_char) -> libc::c_int { fn is_msgrmsg_rfc724_mid(context: &Context, rfc724_mid: &str) -> libc::c_int {
if rfc724_mid.is_null() { if rfc724_mid.is_empty() {
return 0; return 0;
} }
context context
.sql .sql
.exists( .exists(
"SELECT id FROM msgs WHERE rfc724_mid=? AND msgrmsg!=0 AND chat_id>9;", "SELECT id FROM msgs WHERE rfc724_mid=? AND msgrmsg!=0 AND chat_id>9;",
params![as_str(rfc724_mid)], params![rfc724_mid],
) )
.unwrap_or_default() as libc::c_int .unwrap_or_default() as libc::c_int
} }

View File

@@ -572,9 +572,9 @@ pub fn dc_create_incoming_rfc724_mid(
message_timestamp: i64, message_timestamp: i64,
contact_id_from: u32, contact_id_from: u32,
contact_ids_to: &Vec<u32>, contact_ids_to: &Vec<u32>,
) -> *mut libc::c_char { ) -> Option<String> {
if contact_ids_to.is_empty() { if contact_ids_to.is_empty() {
return ptr::null_mut(); return None;
} }
/* find out the largest receiver ID (we could also take the smallest, but it should be unique) */ /* find out the largest receiver ID (we could also take the smallest, but it should be unique) */
let largest_id_to = contact_ids_to.iter().max().copied().unwrap_or_default(); let largest_id_to = contact_ids_to.iter().max().copied().unwrap_or_default();
@@ -583,8 +583,7 @@ pub fn dc_create_incoming_rfc724_mid(
"{}-{}-{}@stub", "{}-{}-{}@stub",
message_timestamp, contact_id_from, largest_id_to message_timestamp, contact_id_from, largest_id_to
); );
Some(result)
unsafe { result.strdup() }
} }
/// Function generates a Message-ID that can be used for a new outgoing message. /// Function generates a Message-ID that can be used for a new outgoing message.
@@ -1791,10 +1790,6 @@ mod tests {
#[test] #[test]
fn test_dc_create_incoming_rfc724_mid() { fn test_dc_create_incoming_rfc724_mid() {
let res = dc_create_incoming_rfc724_mid(123, 45, &vec![6, 7]); let res = dc_create_incoming_rfc724_mid(123, 45, &vec![6, 7]);
assert_eq!(as_str(res), "123-45-7@stub"); assert_eq!(res, Some("123-45-7@stub".into()));
unsafe {
free(res.cast());
}
} }
} }

View File

@@ -0,0 +1,13 @@
Return-Path: <x@testrun.org>
Received: from hq5.merlinux.eu
by hq5.merlinux.eu (Dovecot) with LMTP id yRKOBakcfV1AewAAPzvFDg
; Sat, 14 Sep 2019 19:00:25 +0200
Received: from localhost (unknown 7.165.105.24])
by hq5.merlinux.eu (Postfix) with ESMTPSA id 8D9844E023;
Sat, 14 Sep 2019 19:00:22 +0200 (CEST)
message-id: <2dfdbde7@example.org>
Date: Sat, 14 Sep 2019 19:00:13 +0200
From: lmn <x@tux.org>
To: abc <abc@bcd.com>, def <def@def.de>,
jik