* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal