From 91c0c2ac57629b291c19eb697aa1218d6333a511 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Wed, 10 Sep 2025 10:42:48 +0100 Subject: [PATCH] mailparsing: fallback to Content-Type::name for attachment name Adjusts how we resolve the attachment name when processing mime attachment parameters: * First check Content-Disposition::filename * Then check Content-Type::name De-couple parsing out the `Content-Id` header from having `Content-Disposition`. In the lua bindings; remove the synthesized filename fallback; we now just leave it nil to match the rust logic. --- crates/mailparsing/src/mimepart.rs | 119 +++++++++++++++--- crates/mod-mimepart/src/mimepart.rs | 4 +- crates/mod-mimepart/test_attachments.lua | 2 +- .../mimepart/get_simple_structure.md | 5 +- 4 files changed, 104 insertions(+), 26 deletions(-) diff --git a/crates/mailparsing/src/mimepart.rs b/crates/mailparsing/src/mimepart.rs index a28aa426..530f663b 100644 --- a/crates/mailparsing/src/mimepart.rs +++ b/crates/mailparsing/src/mimepart.rs @@ -37,6 +37,7 @@ pub struct MimePart<'a> { outro: SharedString<'a>, } +#[derive(PartialEq, Debug)] pub struct Rfc2045Info { pub encoding: ContentTransferEncoding, pub charset: Result, @@ -75,7 +76,9 @@ impl Rfc2045Info { } }; + let mut ct_name = None; let charset = if let Some(ct) = &content_type { + ct_name = ct.get("name"); ct.get("charset") } else { None @@ -91,32 +94,41 @@ impl Rfc2045Info { (true, false) }; - let content_disposition = match headers.content_disposition() { - Ok(cd) => cd, + let mut inline = false; + let mut cd_file_name = None; + + match headers.content_disposition() { + Ok(Some(cd)) => { + inline = cd.value == "inline"; + cd_file_name = cd.get("filename"); + } + Ok(None) => {} + Err(_) => { + invalid_mime_headers = true; + } + }; + + let content_id = match headers.content_id() { + Ok(cid) => cid.map(|cid| cid.0), Err(_) => { invalid_mime_headers = true; None } }; - let attachment_options = match content_disposition { - Some(cd) => { - let inline = cd.value == "inline"; - let content_id = match headers.content_id() { - Ok(cid) => cid, - Err(_) => { - invalid_mime_headers = true; - None - } - }; - let file_name = cd.get("filename"); - Some(AttachmentOptions { - file_name, - inline, - content_id: content_id.map(|cid| cid.0), - }) - } - None => None, + let file_name = match (cd_file_name, ct_name) { + (Some(name), _) | (None, Some(name)) => Some(name), + (None, None) => None, + }; + + let attachment_options = if inline || file_name.is_some() || content_id.is_some() { + Some(AttachmentOptions { + file_name, + inline, + content_id, + }) + } else { + None }; Self { @@ -1408,6 +1420,73 @@ Ok( ); } + #[test] + fn attachment_name_order_prefers_content_disposition() { + let message = concat!( + "Content-Type: multipart/mixed;\r\n", + " boundary=\"woot\"\r\n", + "\r\n", + "--woot\r\n", + "Content-Type: text/plain;\r\n", + " charset=\"us-ascii\"\r\n", + "\r\n", + "Hello, I am the main message content\r\n", + "--woot\r\n", + "Content-Disposition: attachment;\r\n", + " filename=cdname\r\n", + "Content-Type: application/octet-stream;\r\n", + " name=ctname\r\n", + "Content-Transfer-Encoding: base64\r\n", + "\r\n", + "u6o=\r\n", + "--woot--\r\n" + ); + let part = MimePart::parse(message).unwrap(); + let structure = part.simplified_structure().unwrap(); + + k9::assert_equal!( + structure.attachments[0].rfc2045_info().attachment_options, + Some(AttachmentOptions { + content_id: None, + inline: false, + file_name: Some("cdname".to_string()), + }) + ); + } + + #[test] + fn attachment_name_accepts_content_type_name() { + let message = concat!( + "Content-Type: multipart/mixed;\r\n", + " boundary=\"woot\"\r\n", + "\r\n", + "--woot\r\n", + "Content-Type: text/plain;\r\n", + " charset=\"us-ascii\"\r\n", + "\r\n", + "Hello, I am the main message content\r\n", + "--woot\r\n", + "Content-Disposition: attachment\r\n", + "Content-Type: application/octet-stream;\r\n", + " name=ctname\r\n", + "Content-Transfer-Encoding: base64\r\n", + "\r\n", + "u6o=\r\n", + "--woot--\r\n" + ); + let part = MimePart::parse(message).unwrap(); + let structure = part.simplified_structure().unwrap(); + + k9::assert_equal!( + structure.attachments[0].rfc2045_info().attachment_options, + Some(AttachmentOptions { + content_id: None, + inline: false, + file_name: Some("ctname".to_string()), + }) + ); + } + #[test] fn funky_headers() { let message = concat!( diff --git a/crates/mod-mimepart/src/mimepart.rs b/crates/mod-mimepart/src/mimepart.rs index 357cc6fa..e0d2e147 100644 --- a/crates/mod-mimepart/src/mimepart.rs +++ b/crates/mod-mimepart/src/mimepart.rs @@ -122,7 +122,7 @@ impl UserData for PartRef { let a_part = a.resolve().map_err(any_err)?; let attach = lua.create_table()?; - let mut file_name = format!("attachment{}", a.ptr.id_string()); + let mut file_name = None; let mut inline = true; let mut content_id = None; let mut content_type = None; @@ -130,7 +130,7 @@ impl UserData for PartRef { let info = a_part.rfc2045_info(); if let Some(mut opts) = info.attachment_options { if let Some(name) = opts.file_name.take() { - file_name = name; + file_name.replace(name); } inline = opts.inline; content_id = opts.content_id; diff --git a/crates/mod-mimepart/test_attachments.lua b/crates/mod-mimepart/test_attachments.lua index e69c6089..cf1c2c0f 100644 --- a/crates/mod-mimepart/test_attachments.lua +++ b/crates/mod-mimepart/test_attachments.lua @@ -28,7 +28,7 @@ end utils.assert_eq(attachments, { { - 'attachment1', + nil, 'text/plain', 'I am a plain text file with no options specified', }, diff --git a/docs/reference/mimepart/get_simple_structure.md b/docs/reference/mimepart/get_simple_structure.md index e4c2b299..dc033530 100644 --- a/docs/reference/mimepart/get_simple_structure.md +++ b/docs/reference/mimepart/get_simple_structure.md @@ -25,9 +25,8 @@ Each attachment table entry has the following fields: * `file_name` - The suggested name to use when saving the attachment. If the `Content-Disposition` header defined the file name, then that will be used. - Otherwise, a name will be synthesized based on the position of the - attachment within the MIME tree and will look something like `attachment1` - or `attachment2.3`. + Otherwise, the `Content-Type` `name` parameter will be used. If neither is + present, then the `file_name` field will not be set (effectively `nil`). * `inline` - will be `true` if the attachment was marked as having an inline disposition, `false` otherwise. * `content_id` - if the `Content-ID` header is defined, this field will hold