* [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting
@ 2023-12-13 16:42 Lukas Wagner
2023-12-13 16:42 ` [pve-devel] [PATCH proxmox 2/2] notify: smtp: add `Auto-Submitted` header to email body Lukas Wagner
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lukas Wagner @ 2023-12-13 16:42 UTC (permalink / raw)
To: pve-devel
For mails forwarded by `proxmox-mail-forward` to an SMTP target, the
original message was nested as a 'message/rfc822' message part.
Originally this approach was chosen to avoid having to rewrite
message headers.
Good email-clients, such as Thunderbird can display these inline.
Other, more limited clients will show these messages as an attached
.eml file, which is not really a good user experience.
This patch changes the approach for message forwarding to be more like
forwarding mails in a mail client. We create a new message and
add the original message body as a body. Additionally, we also copy
over all message headers that are relevant to correctly display the
original message body (e.g. Content-Type, Content-Transfer-Encoding)
Tested with a couple of different email messages (varying in
structure, body parts, encoding, etc.) against the following SMTP
relays:
- gmail
- outlook
- our own webmail service
Originally reported in our community forum:
https://forum.proxmox.com/threads/proxmox-mail-forward-sends-mails-as-eml.137710/
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
proxmox-mail-forward needs a bump once this is applied.
proxmox-notify/src/endpoints/smtp.rs | 99 ++++++++++++++++++++++++----
1 file changed, 88 insertions(+), 11 deletions(-)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 064c9f9..4b8ff2d 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -1,8 +1,9 @@
+use std::time::Duration;
+
use lettre::message::{Mailbox, MultiPart, SinglePart};
use lettre::transport::smtp::client::{Tls, TlsParameters};
use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
use serde::{Deserialize, Serialize};
-use std::time::Duration;
use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{api, Updater};
@@ -231,17 +232,93 @@ impl Endpoint for SmtpEndpoint {
}
#[cfg(feature = "mail-forwarder")]
Content::ForwardedMail { ref raw, title, .. } => {
- email_builder = email_builder.subject(title);
+ use lettre::message::header::{ContentTransferEncoding, HeaderName, HeaderValue};
+ use lettre::message::Body;
- // Forwarded messages are embedded inline as 'message/rfc822'
- // this let's us avoid rewriting any headers (e.g. From)
- email_builder
- .singlepart(
- SinglePart::builder()
- .header(ContentType::parse("message/rfc822").unwrap())
- .body(raw.to_owned()),
- )
- .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?
+ let parsed_message = mail_parser::Message::parse(raw)
+ .ok_or_else(|| Error::Generic("could not parse forwarded email".to_string()))?;
+
+ let root_part = parsed_message
+ .part(0)
+ .ok_or_else(|| Error::Generic("root message part not present".to_string()))?;
+
+ let raw_body = parsed_message
+ .raw_message()
+ .get(root_part.offset_body..root_part.offset_end)
+ .ok_or_else(|| Error::Generic("could not get raw body content".to_string()))?;
+
+ // We assume that the original message content is already properly
+ // encoded, thus we add the original message body 'Binary' encoding.
+ // This prohibits lettre from trying to re-encode our raw body data.
+ // lettre will automatically set the `Content-Transfer-Encoding: binary` header,
+ // which we need to remove. The actual transfer encoding is later
+ // copied from the original message headers.
+ let body =
+ Body::new_with_encoding(raw_body.to_vec(), ContentTransferEncoding::Binary)
+ .map_err(|_| Error::Generic("could not create body".into()))?;
+ let mut message = email_builder
+ .subject(title)
+ .body(body)
+ .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?;
+ message
+ .headers_mut()
+ .remove_raw("Content-Transfer-Encoding");
+
+ // Copy over all headers that are relevant to display the original body correctly.
+ // Unfortunately this is a bit cumbersome, as we use separate crates for mail parsing (mail-parser)
+ // and creating/sending mails (lettre).
+ // Note: Other MIME-Headers, such as Content-{ID,Description,Disposition} are only used
+ // for body-parts in multipart messages, so we can ignore them for the messages headers.
+ // Since we send the original raw body, the part-headers will be included any way.
+ for header in parsed_message.headers() {
+ let header_name = header.name.as_str();
+ // Email headers are case-insensitive, so convert to lowercase...
+ let value = match header_name.to_lowercase().as_str() {
+ "content-type" => {
+ if let mail_parser::HeaderValue::ContentType(ct) = header.value() {
+ // mail_parser does not give us access to the full decoded and unfolded
+ // header value, so we unfortunately need to reassemble it ourselves.
+ // Meh.
+ let mut value = ct.ctype().to_string();
+ if let Some(subtype) = ct.subtype() {
+ value.push('/');
+ value.push_str(subtype);
+ }
+ if let Some(attributes) = ct.attributes() {
+ for attribute in attributes {
+ value.push_str(&format!(
+ "; {}=\"{}\"",
+ attribute.0, attribute.1
+ ));
+ }
+ }
+ Some(value)
+ } else {
+ None
+ }
+ }
+ "content-transfer-encoding" | "mime-version" => {
+ if let mail_parser::HeaderValue::Text(text) = header.value() {
+ Some(text.to_string())
+ } else {
+ None
+ }
+ }
+ _ => None,
+ };
+
+ if let Some(value) = value {
+ match HeaderName::new_from_ascii(header_name.into()) {
+ Ok(name) => {
+ let header = HeaderValue::new(name, value);
+ message.headers_mut().insert_raw(header);
+ }
+ Err(e) => log::error!("could not set header: {e}"),
+ }
+ }
+ }
+
+ message
}
};
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH proxmox 2/2] notify: smtp: add `Auto-Submitted` header to email body
2023-12-13 16:42 [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
@ 2023-12-13 16:42 ` Lukas Wagner
2024-01-08 10:43 ` [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
2024-01-09 13:26 ` Wolfgang Bumiller
2 siblings, 0 replies; 5+ messages in thread
From: Lukas Wagner @ 2023-12-13 16:42 UTC (permalink / raw)
To: pve-devel
`Auto-Submitted` is defined in the rfc 5436 [1] and describes how
an automatic response (f.e. ooo replies, etc.) should behave on the
emails. When using `Auto-Submitted: auto-generated` (or any value
other than `none`) automatic replies won't be triggered.
[1]: https://www.rfc-editor.org/rfc/rfc3834.html
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/endpoints/smtp.rs | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 4b8ff2d..15724d3 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -1,5 +1,6 @@
use std::time::Duration;
+use lettre::message::header::{HeaderName, HeaderValue};
use lettre::message::{Mailbox, MultiPart, SinglePart};
use lettre::transport::smtp::client::{Tls, TlsParameters};
use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
@@ -199,7 +200,7 @@ impl Endpoint for SmtpEndpoint {
email_builder = email_builder.to(parse_address(&recipient)?);
}
- let email = match ¬ification.content {
+ let mut email = match ¬ification.content {
Content::Template {
title_template,
body_template,
@@ -232,7 +233,7 @@ impl Endpoint for SmtpEndpoint {
}
#[cfg(feature = "mail-forwarder")]
Content::ForwardedMail { ref raw, title, .. } => {
- use lettre::message::header::{ContentTransferEncoding, HeaderName, HeaderValue};
+ use lettre::message::header::ContentTransferEncoding;
use lettre::message::Body;
let parsed_message = mail_parser::Message::parse(raw)
@@ -322,6 +323,15 @@ impl Endpoint for SmtpEndpoint {
}
};
+ // `Auto-Submitted` is defined in RFC 5436 and describes how
+ // an automatic response (f.e. ooo replies, etc.) should behave on the
+ // emails. When using `Auto-Submitted: auto-generated` (or any value
+ // other than `none`) automatic replies won't be triggered.
+ email.headers_mut().insert_raw(HeaderValue::new(
+ HeaderName::new_from_ascii_str("Auto-Submitted"),
+ "auto-generated;".into(),
+ ));
+
transport
.send(&email)
.map_err(|err| Error::NotifyFailed(self.name().into(), err.into()))?;
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting
2023-12-13 16:42 [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
2023-12-13 16:42 ` [pve-devel] [PATCH proxmox 2/2] notify: smtp: add `Auto-Submitted` header to email body Lukas Wagner
@ 2024-01-08 10:43 ` Lukas Wagner
2024-01-09 13:26 ` Wolfgang Bumiller
2 siblings, 0 replies; 5+ messages in thread
From: Lukas Wagner @ 2024-01-08 10:43 UTC (permalink / raw)
To: pve-devel
ping
On 12/13/23 17:42, Lukas Wagner wrote:
> For mails forwarded by `proxmox-mail-forward` to an SMTP target, the
> original message was nested as a 'message/rfc822' message part.
> Originally this approach was chosen to avoid having to rewrite
> message headers.
> Good email-clients, such as Thunderbird can display these inline.
> Other, more limited clients will show these messages as an attached
> .eml file, which is not really a good user experience.
>
> This patch changes the approach for message forwarding to be more like
> forwarding mails in a mail client. We create a new message and
> add the original message body as a body. Additionally, we also copy
> over all message headers that are relevant to correctly display the
> original message body (e.g. Content-Type, Content-Transfer-Encoding)
>
> Tested with a couple of different email messages (varying in
> structure, body parts, encoding, etc.) against the following SMTP
> relays:
> - gmail
> - outlook
> - our own webmail service
>
> Originally reported in our community forum:
> https://forum.proxmox.com/threads/proxmox-mail-forward-sends-mails-as-eml.137710/
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> proxmox-mail-forward needs a bump once this is applied.
>
> proxmox-notify/src/endpoints/smtp.rs | 99 ++++++++++++++++++++++++----
> 1 file changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 064c9f9..4b8ff2d 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -1,8 +1,9 @@
> +use std::time::Duration;
> +
> use lettre::message::{Mailbox, MultiPart, SinglePart};
> use lettre::transport::smtp::client::{Tls, TlsParameters};
> use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
> use serde::{Deserialize, Serialize};
> -use std::time::Duration;
>
> use proxmox_schema::api_types::COMMENT_SCHEMA;
> use proxmox_schema::{api, Updater};
> @@ -231,17 +232,93 @@ impl Endpoint for SmtpEndpoint {
> }
> #[cfg(feature = "mail-forwarder")]
> Content::ForwardedMail { ref raw, title, .. } => {
> - email_builder = email_builder.subject(title);
> + use lettre::message::header::{ContentTransferEncoding, HeaderName, HeaderValue};
> + use lettre::message::Body;
>
> - // Forwarded messages are embedded inline as 'message/rfc822'
> - // this let's us avoid rewriting any headers (e.g. From)
> - email_builder
> - .singlepart(
> - SinglePart::builder()
> - .header(ContentType::parse("message/rfc822").unwrap())
> - .body(raw.to_owned()),
> - )
> - .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?
> + let parsed_message = mail_parser::Message::parse(raw)
> + .ok_or_else(|| Error::Generic("could not parse forwarded email".to_string()))?;
> +
> + let root_part = parsed_message
> + .part(0)
> + .ok_or_else(|| Error::Generic("root message part not present".to_string()))?;
> +
> + let raw_body = parsed_message
> + .raw_message()
> + .get(root_part.offset_body..root_part.offset_end)
> + .ok_or_else(|| Error::Generic("could not get raw body content".to_string()))?;
> +
> + // We assume that the original message content is already properly
> + // encoded, thus we add the original message body 'Binary' encoding.
> + // This prohibits lettre from trying to re-encode our raw body data.
> + // lettre will automatically set the `Content-Transfer-Encoding: binary` header,
> + // which we need to remove. The actual transfer encoding is later
> + // copied from the original message headers.
> + let body =
> + Body::new_with_encoding(raw_body.to_vec(), ContentTransferEncoding::Binary)
> + .map_err(|_| Error::Generic("could not create body".into()))?;
> + let mut message = email_builder
> + .subject(title)
> + .body(body)
> + .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?;
> + message
> + .headers_mut()
> + .remove_raw("Content-Transfer-Encoding");
> +
> + // Copy over all headers that are relevant to display the original body correctly.
> + // Unfortunately this is a bit cumbersome, as we use separate crates for mail parsing (mail-parser)
> + // and creating/sending mails (lettre).
> + // Note: Other MIME-Headers, such as Content-{ID,Description,Disposition} are only used
> + // for body-parts in multipart messages, so we can ignore them for the messages headers.
> + // Since we send the original raw body, the part-headers will be included any way.
> + for header in parsed_message.headers() {
> + let header_name = header.name.as_str();
> + // Email headers are case-insensitive, so convert to lowercase...
> + let value = match header_name.to_lowercase().as_str() {
> + "content-type" => {
> + if let mail_parser::HeaderValue::ContentType(ct) = header.value() {
> + // mail_parser does not give us access to the full decoded and unfolded
> + // header value, so we unfortunately need to reassemble it ourselves.
> + // Meh.
> + let mut value = ct.ctype().to_string();
> + if let Some(subtype) = ct.subtype() {
> + value.push('/');
> + value.push_str(subtype);
> + }
> + if let Some(attributes) = ct.attributes() {
> + for attribute in attributes {
> + value.push_str(&format!(
> + "; {}=\"{}\"",
> + attribute.0, attribute.1
> + ));
> + }
> + }
> + Some(value)
> + } else {
> + None
> + }
> + }
> + "content-transfer-encoding" | "mime-version" => {
> + if let mail_parser::HeaderValue::Text(text) = header.value() {
> + Some(text.to_string())
> + } else {
> + None
> + }
> + }
> + _ => None,
> + };
> +
> + if let Some(value) = value {
> + match HeaderName::new_from_ascii(header_name.into()) {
> + Ok(name) => {
> + let header = HeaderValue::new(name, value);
> + message.headers_mut().insert_raw(header);
> + }
> + Err(e) => log::error!("could not set header: {e}"),
> + }
> + }
> + }
> +
> + message
> }
> };
>
--
- Lukas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting
2023-12-13 16:42 [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
2023-12-13 16:42 ` [pve-devel] [PATCH proxmox 2/2] notify: smtp: add `Auto-Submitted` header to email body Lukas Wagner
2024-01-08 10:43 ` [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
@ 2024-01-09 13:26 ` Wolfgang Bumiller
2024-01-10 8:28 ` Lukas Wagner
2 siblings, 1 reply; 5+ messages in thread
From: Wolfgang Bumiller @ 2024-01-09 13:26 UTC (permalink / raw)
To: Lukas Wagner; +Cc: pve-devel
On Wed, Dec 13, 2023 at 05:42:00PM +0100, Lukas Wagner wrote:
> For mails forwarded by `proxmox-mail-forward` to an SMTP target, the
> original message was nested as a 'message/rfc822' message part.
> Originally this approach was chosen to avoid having to rewrite
> message headers.
> Good email-clients, such as Thunderbird can display these inline.
> Other, more limited clients will show these messages as an attached
> .eml file, which is not really a good user experience.
>
> This patch changes the approach for message forwarding to be more like
> forwarding mails in a mail client. We create a new message and
> add the original message body as a body. Additionally, we also copy
> over all message headers that are relevant to correctly display the
> original message body (e.g. Content-Type, Content-Transfer-Encoding)
>
> Tested with a couple of different email messages (varying in
> structure, body parts, encoding, etc.) against the following SMTP
> relays:
> - gmail
> - outlook
> - our own webmail service
>
> Originally reported in our community forum:
> https://forum.proxmox.com/threads/proxmox-mail-forward-sends-mails-as-eml.137710/
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> proxmox-mail-forward needs a bump once this is applied.
>
> proxmox-notify/src/endpoints/smtp.rs | 99 ++++++++++++++++++++++++----
> 1 file changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
> index 064c9f9..4b8ff2d 100644
> --- a/proxmox-notify/src/endpoints/smtp.rs
> +++ b/proxmox-notify/src/endpoints/smtp.rs
> @@ -1,8 +1,9 @@
> +use std::time::Duration;
> +
> use lettre::message::{Mailbox, MultiPart, SinglePart};
> use lettre::transport::smtp::client::{Tls, TlsParameters};
> use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
> use serde::{Deserialize, Serialize};
> -use std::time::Duration;
>
> use proxmox_schema::api_types::COMMENT_SCHEMA;
> use proxmox_schema::{api, Updater};
> @@ -231,17 +232,93 @@ impl Endpoint for SmtpEndpoint {
> }
> #[cfg(feature = "mail-forwarder")]
> Content::ForwardedMail { ref raw, title, .. } => {
> - email_builder = email_builder.subject(title);
> + use lettre::message::header::{ContentTransferEncoding, HeaderName, HeaderValue};
> + use lettre::message::Body;
>
> - // Forwarded messages are embedded inline as 'message/rfc822'
> - // this let's us avoid rewriting any headers (e.g. From)
> - email_builder
> - .singlepart(
> - SinglePart::builder()
> - .header(ContentType::parse("message/rfc822").unwrap())
> - .body(raw.to_owned()),
> - )
> - .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?
> + let parsed_message = mail_parser::Message::parse(raw)
> + .ok_or_else(|| Error::Generic("could not parse forwarded email".to_string()))?;
> +
> + let root_part = parsed_message
> + .part(0)
> + .ok_or_else(|| Error::Generic("root message part not present".to_string()))?;
> +
> + let raw_body = parsed_message
> + .raw_message()
> + .get(root_part.offset_body..root_part.offset_end)
> + .ok_or_else(|| Error::Generic("could not get raw body content".to_string()))?;
> +
> + // We assume that the original message content is already properly
> + // encoded, thus we add the original message body 'Binary' encoding.
> + // This prohibits lettre from trying to re-encode our raw body data.
> + // lettre will automatically set the `Content-Transfer-Encoding: binary` header,
> + // which we need to remove. The actual transfer encoding is later
> + // copied from the original message headers.
> + let body =
> + Body::new_with_encoding(raw_body.to_vec(), ContentTransferEncoding::Binary)
> + .map_err(|_| Error::Generic("could not create body".into()))?;
> + let mut message = email_builder
> + .subject(title)
> + .body(body)
> + .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?;
> + message
> + .headers_mut()
> + .remove_raw("Content-Transfer-Encoding");
> +
> + // Copy over all headers that are relevant to display the original body correctly.
> + // Unfortunately this is a bit cumbersome, as we use separate crates for mail parsing (mail-parser)
> + // and creating/sending mails (lettre).
> + // Note: Other MIME-Headers, such as Content-{ID,Description,Disposition} are only used
> + // for body-parts in multipart messages, so we can ignore them for the messages headers.
> + // Since we send the original raw body, the part-headers will be included any way.
> + for header in parsed_message.headers() {
> + let header_name = header.name.as_str();
> + // Email headers are case-insensitive, so convert to lowercase...
> + let value = match header_name.to_lowercase().as_str() {
> + "content-type" => {
> + if let mail_parser::HeaderValue::ContentType(ct) = header.value() {
> + // mail_parser does not give us access to the full decoded and unfolded
> + // header value, so we unfortunately need to reassemble it ourselves.
> + // Meh.
urgh
> + let mut value = ct.ctype().to_string();
> + if let Some(subtype) = ct.subtype() {
> + value.push('/');
> + value.push_str(subtype);
> + }
> + if let Some(attributes) = ct.attributes() {
> + for attribute in attributes {
> + value.push_str(&format!(
> + "; {}=\"{}\"",
> + attribute.0, attribute.1
> + ));
should be more efficient to use
let _ = write!(value, "...");
instead of `push_str()` on a temporarily allocated (format!()) string.
> + }
> + }
> + Some(value)
> + } else {
> + None
> + }
> + }
> + "content-transfer-encoding" | "mime-version" => {
> + if let mail_parser::HeaderValue::Text(text) = header.value() {
> + Some(text.to_string())
> + } else {
> + None
> + }
> + }
> + _ => None,
> + };
> +
> + if let Some(value) = value {
> + match HeaderName::new_from_ascii(header_name.into()) {
> + Ok(name) => {
> + let header = HeaderValue::new(name, value);
> + message.headers_mut().insert_raw(header);
> + }
> + Err(e) => log::error!("could not set header: {e}"),
> + }
> + }
> + }
> +
> + message
> }
> };
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting
2024-01-09 13:26 ` Wolfgang Bumiller
@ 2024-01-10 8:28 ` Lukas Wagner
0 siblings, 0 replies; 5+ messages in thread
From: Lukas Wagner @ 2024-01-10 8:28 UTC (permalink / raw)
To: Wolfgang Bumiller; +Cc: pve-devel
Thanks for the review!
On 1/9/24 14:26, Wolfgang Bumiller wrote:
> On Wed, Dec 13, 2023 at 05:42:00PM +0100, Lukas Wagner wrote:
>> For mails forwarded by `proxmox-mail-forward` to an SMTP target, the
>> original message was nested as a 'message/rfc822' message part.
>> Originally this approach was chosen to avoid having to rewrite
>> message headers.
>> Good email-clients, such as Thunderbird can display these inline.
>> Other, more limited clients will show these messages as an attached
>> .eml file, which is not really a good user experience.
>>
>> This patch changes the approach for message forwarding to be more like
>> forwarding mails in a mail client. We create a new message and
>> add the original message body as a body. Additionally, we also copy
>> over all message headers that are relevant to correctly display the
>> original message body (e.g. Content-Type, Content-Transfer-Encoding)
>>
>> Tested with a couple of different email messages (varying in
>> structure, body parts, encoding, etc.) against the following SMTP
>> relays:
>> - gmail
>> - outlook
>> - our own webmail service
>>
>> Originally reported in our community forum:
>> https://forum.proxmox.com/threads/proxmox-mail-forward-sends-mails-as-eml.137710/
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>
>> Notes:
>> proxmox-mail-forward needs a bump once this is applied.
>>
>> proxmox-notify/src/endpoints/smtp.rs | 99 ++++++++++++++++++++++++----
>> 1 file changed, 88 insertions(+), 11 deletions(-)
>>
>> diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
>> index 064c9f9..4b8ff2d 100644
>> --- a/proxmox-notify/src/endpoints/smtp.rs
>> +++ b/proxmox-notify/src/endpoints/smtp.rs
>> @@ -1,8 +1,9 @@
>> +use std::time::Duration;
>> +
>> use lettre::message::{Mailbox, MultiPart, SinglePart};
>> use lettre::transport::smtp::client::{Tls, TlsParameters};
>> use lettre::{message::header::ContentType, Message, SmtpTransport, Transport};
>> use serde::{Deserialize, Serialize};
>> -use std::time::Duration;
>>
>> use proxmox_schema::api_types::COMMENT_SCHEMA;
>> use proxmox_schema::{api, Updater};
>> @@ -231,17 +232,93 @@ impl Endpoint for SmtpEndpoint {
>> }
>> #[cfg(feature = "mail-forwarder")]
>> Content::ForwardedMail { ref raw, title, .. } => {
>> - email_builder = email_builder.subject(title);
>> + use lettre::message::header::{ContentTransferEncoding, HeaderName, HeaderValue};
>> + use lettre::message::Body;
>>
>> - // Forwarded messages are embedded inline as 'message/rfc822'
>> - // this let's us avoid rewriting any headers (e.g. From)
>> - email_builder
>> - .singlepart(
>> - SinglePart::builder()
>> - .header(ContentType::parse("message/rfc822").unwrap())
>> - .body(raw.to_owned()),
>> - )
>> - .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?
>> + let parsed_message = mail_parser::Message::parse(raw)
>> + .ok_or_else(|| Error::Generic("could not parse forwarded email".to_string()))?;
>> +
>> + let root_part = parsed_message
>> + .part(0)
>> + .ok_or_else(|| Error::Generic("root message part not present".to_string()))?;
>> +
>> + let raw_body = parsed_message
>> + .raw_message()
>> + .get(root_part.offset_body..root_part.offset_end)
>> + .ok_or_else(|| Error::Generic("could not get raw body content".to_string()))?;
>> +
>> + // We assume that the original message content is already properly
>> + // encoded, thus we add the original message body 'Binary' encoding.
>> + // This prohibits lettre from trying to re-encode our raw body data.
>> + // lettre will automatically set the `Content-Transfer-Encoding: binary` header,
>> + // which we need to remove. The actual transfer encoding is later
>> + // copied from the original message headers.
>> + let body =
>> + Body::new_with_encoding(raw_body.to_vec(), ContentTransferEncoding::Binary)
>> + .map_err(|_| Error::Generic("could not create body".into()))?;
>> + let mut message = email_builder
>> + .subject(title)
>> + .body(body)
>> + .map_err(|err| Error::NotifyFailed(self.name().into(), Box::new(err)))?;
>> + message
>> + .headers_mut()
>> + .remove_raw("Content-Transfer-Encoding");
>> +
>> + // Copy over all headers that are relevant to display the original body correctly.
>> + // Unfortunately this is a bit cumbersome, as we use separate crates for mail parsing (mail-parser)
>> + // and creating/sending mails (lettre).
>> + // Note: Other MIME-Headers, such as Content-{ID,Description,Disposition} are only used
>> + // for body-parts in multipart messages, so we can ignore them for the messages headers.
>> + // Since we send the original raw body, the part-headers will be included any way.
>> + for header in parsed_message.headers() {
>> + let header_name = header.name.as_str();
>> + // Email headers are case-insensitive, so convert to lowercase...
>> + let value = match header_name.to_lowercase().as_str() {
>> + "content-type" => {
>> + if let mail_parser::HeaderValue::ContentType(ct) = header.value() {
>> + // mail_parser does not give us access to the full decoded and unfolded
>> + // header value, so we unfortunately need to reassemble it ourselves.
>> + // Meh.
>
> urgh
>
urgh indeed...
>> + let mut value = ct.ctype().to_string();
>> + if let Some(subtype) = ct.subtype() {
>> + value.push('/');
>> + value.push_str(subtype);
>> + }
>> + if let Some(attributes) = ct.attributes() {
>> + for attribute in attributes {
>> + value.push_str(&format!(
>> + "; {}=\"{}\"",
>> + attribute.0, attribute.1
>> + ));
>
> should be more efficient to use
> let _ = write!(value, "...");
> instead of `push_str()` on a temporarily allocated (format!()) string.
Right, thanks! Will send a v2.
>
>> + }
>> + }
>> + Some(value)
>> + } else {
>> + None
>> + }
>> + }
>> + "content-transfer-encoding" | "mime-version" => {
>> + if let mail_parser::HeaderValue::Text(text) = header.value() {
>> + Some(text.to_string())
>> + } else {
>> + None
>> + }
>> + }
>> + _ => None,
>> + };
>> +
>> + if let Some(value) = value {
>> + match HeaderName::new_from_ascii(header_name.into()) {
>> + Ok(name) => {
>> + let header = HeaderValue::new(name, value);
>> + message.headers_mut().insert_raw(header);
>> + }
>> + Err(e) => log::error!("could not set header: {e}"),
>> + }
>> + }
>> + }
>> +
>> + message
>> }
>> };
>>
>> --
>> 2.39.2
--
- Lukas
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-10 8:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-13 16:42 [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
2023-12-13 16:42 ` [pve-devel] [PATCH proxmox 2/2] notify: smtp: add `Auto-Submitted` header to email body Lukas Wagner
2024-01-08 10:43 ` [pve-devel] [PATCH proxmox 1/2] notify: smtp: forward original message instead nesting Lukas Wagner
2024-01-09 13:26 ` Wolfgang Bumiller
2024-01-10 8:28 ` Lukas Wagner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox