all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Stoiko Ivanov <s.ivanov@proxmox.com>,
	Hannes Laimer <h.laimer@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox 1/1] email: add small function to send multi-part emails using sendmail
Date: Thu, 20 Aug 2020 16:08:19 +0200	[thread overview]
Message-ID: <a5bd1a81-7d1d-89ae-27b7-cd2f15f4c21b@proxmox.com> (raw)
In-Reply-To: <20200820143403.17201a47@rosa.proxmox.com>

On 20.08.20 14:34, Stoiko Ivanov wrote:
> Thanks for the patch!
> 
> some comments/suggestions on e-mail inline:
> 
> On Thu, 20 Aug 2020 11:23:50 +0200
> Hannes Laimer <h.laimer@proxmox.com> wrote:
> 
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>>  proxmox/src/tools/email.rs | 130 +++++++++++++++++++++++++++++++++++++
>>  proxmox/src/tools/mod.rs   |   1 +
>>  2 files changed, 131 insertions(+)
>>  create mode 100644 proxmox/src/tools/email.rs
>>
>> diff --git a/proxmox/src/tools/email.rs b/proxmox/src/tools/email.rs
>> new file mode 100644
>> index 0000000..b73a6d6
>> --- /dev/null
>> +++ b/proxmox/src/tools/email.rs
>> @@ -0,0 +1,130 @@
>> +//! Email related utilities.
>> +
>> +use std::process::{Command, Stdio};
>> +use anyhow::{bail, Error};
>> +use std::io::Write;
>> +use crate::tools::time::time;
>> +
>> +
>> +/// Sends multi-part mail with text and/or html to a list of recipients
>> +///
>> +/// ``sendmail`` is used for sending the mail.
>> +pub fn sendmail(mailto: Vec<&str>,
>> +                subject: &str,
>> +                text: Option<&str>,
>> +                html: Option<&str>,
>> +                mailfrom: Option<&str>,
>> +                author: Option<&str>) -> Result<(), Error> {
>> +    let mail_regex = regex::Regex::new(r"^[a-zA-Z\.0-9]+@[a-zA-Z\.0-9]+$").unwrap();
>> +
>> +    if mailto.is_empty() {
>> +        bail!("At least one recipient has to be specified!")
>> +    }
>> +
>> +    for recipient in &mailto {
>> +        if !mail_regex.is_match(recipient) {
>> +            bail!("'{}' is not a valid email address", recipient)
>> +        }
>> +    }
>> +
>> +    let mailfrom = mailfrom.unwrap_or("root");
>> +    if !mailfrom.eq("root") && !mail_regex.is_match(mailfrom) {
>> +        bail!("'{}' is not a valid email address", mailfrom)
>> +    }
>> +
>> +    let recipients = mailto.join(",");
>> +    let author = author.unwrap_or("Proxmox Backup Server");
>> +
>> +    let mut sendmail_process = match Command::new("/usr/sbin/sendmail")
>> +        .arg("-B")
>> +        .arg("8BITMIME")
>> +        .arg("-f")
>> +        .arg(mailfrom)
>> +        .arg("--")
>> +        .arg(&recipients)
>> +        .stdin(Stdio::piped())
>> +        .spawn() {
>> +        Err(err) => bail!("could not spawn sendmail process: {}", err),
>> +        Ok(process) => process
>> +    };
>> +    let mut body = String::new();
>> +    let boundary = format!("----_=_NextPart_001_{}", time()?);
>> +
>> +    body.push_str("Content-Type: multipart/alternative;\n");
>> +    body.push_str(&format!("\tboundary=\"{}\"\n", boundary));
>> +    body.push_str("MIME-Version: 1.0\n");
>> +    body.push_str(&format!("FROM: {} <{}>\n", author, mailfrom));
>> +    body.push_str(&format!("TO: {}\n", &recipients));
>> +    body.push_str(&format!("SUBJECT: {}\n", subject));
> 
> iirc the header needs to be encoded if it contains non-ascii characters
> (the content-type charset only relates to the body) - I assume that
> postfix/exim would do the correct thing these days (and send the mail with
> smtputf-8 extension (though I'm not sure what would happen if the
> receiving server does not support that)
> - would suggest to test sending such a mail with a non-ascii character
> in the subject.
> 
> writing the header names (TO FROM SUBJECT) in all-caps seems odd to my
> eyes (but I guess this is pretty cosmetic)
> 
> I would suggest adding a Date header (though postfix does that for locally
> submitted e-mail) - else the mails are more likely to be considered spam.
> 
> Finally - if you only have a text/plain part I would only send that part
> (without the wrapping in multipart/alternative) (similarly for a
> single text/html part without text/plain part).
> 
> 
>> +    body.push('\n');
>> +    body.push_str("This is a multi-part message in MIME format.\n\n");
>> +    body.push_str(&format!("--{}\n", boundary));
>> +    if let Some(text) = text {
>> +        body.push_str("Content-Type: text/plain;\n");
>> +        body.push_str("\tcharset=\"UTF8\"\n");
> While most MTA, scanners, MUAs won't care much - the charset should
> probably be written as 'UTF-8' (see https://tools.ietf.org/html/rfc3629
> (section 8))
> 

You're right with your feedback, but I guess that most stems from the original
Perl implementation[0] Hannes used as model.

We (meaning one of you two ;P) should overhaul that one too then.

[0]: https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/Tools.pm;h=f9270d9b10340eec789346e81f4a926772ccabfd;hb=HEAD#l1439




      reply	other threads:[~2020-08-20 14:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-20  9:23 [pbs-devel] [PATCH proxmox 0/1] sendmail function Hannes Laimer
2020-08-20  9:23 ` [pbs-devel] [PATCH proxmox 1/1] email: add small function to send multi-part emails using sendmail Hannes Laimer
2020-08-20 12:34   ` Stoiko Ivanov
2020-08-20 14:08     ` Thomas Lamprecht [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5bd1a81-7d1d-89ae-27b7-cd2f15f4c21b@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=h.laimer@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=s.ivanov@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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