* [pve-devel] [PATCH common v2 1/2] sendmail-helper: only send multipart if necessary
2020-09-03 12:09 [pve-devel] [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications Stoiko Ivanov
@ 2020-09-03 12:09 ` Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 2/2] sendmail helper: allow empty display name in from Stoiko Ivanov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2020-09-03 12:09 UTC (permalink / raw)
To: pve-devel
PVE::Tools::sendmail currently always sends a multipart/alternative message
irrespective of the actual content of the mail (e.g. a plain-text only mail
need not be sent as multipart message).
Additionally a few small refactorings based on the discussion
in https://lists.proxmox.com/pipermail/pbs-devel/2020-August/000423.html
and commited in 66004f22c6475ceb0146cf2df1f380f9f0274be4 in the
rust proxmox repository git://git.proxmox.com/git/proxmox.git
were carried over.
tested by creating a backup of a VM and setting an e-mail address, having
ha-manager send a mail after fencing as well as sending a few small mails via
'perl -e'
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PVE/Tools.pm | 41 ++++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index f9270d9..3b5b199 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -25,6 +25,8 @@ use Text::ParseWords;
use String::ShellQuote;
use Time::HiRes qw(usleep gettimeofday tv_interval alarm);
use Scalar::Util 'weaken';
+use Date::Format qw(time2str);
+
use PVE::Syscall;
# avoid warning when parsing long hex values with hex()
@@ -1460,23 +1462,36 @@ sub sendmail {
open (MAIL, "|-", "sendmail", "-B", "8BITMIME", "-f", $mailfrom, "--", @$mailto) ||
die "unable to open 'sendmail' - $!";
+ my $date = time2str('%a, %d %b %Y %H:%M:%S %z', time());
+
+ my $is_multipart = $text && $html;
+
# multipart spec see https://www.ietf.org/rfc/rfc1521.txt
my $boundary = "----_=_NextPart_001_".int(time).$$;
- print MAIL "Content-Type: multipart/alternative;\n";
- print MAIL "\tboundary=\"$boundary\"\n";
- print MAIL "MIME-Version: 1.0\n";
+ if ($subject =~ /[^[:ascii:]]/) {
+ $subject = Encode::encode('MIME-Header', $subject);
+ }
- print MAIL "FROM: $author <$mailfrom>\n";
- print MAIL "TO: $rcvrtxt\n";
- print MAIL "SUBJECT: $subject\n";
- print MAIL "\n";
- print MAIL "This is a multi-part message in MIME format.\n\n";
- print MAIL "--$boundary\n";
+ if ($subject =~ /[^[:ascii:]]/ || $is_multipart) {
+ print MAIL "MIME-Version: 1.0\n";
+ }
+ print MAIL "From: $author <$mailfrom>\n";
+ print MAIL "To: $rcvrtxt\n";
+ print MAIL "Date: $date\n";
+ print MAIL "Subject: $subject\n";
+
+ if ($is_multipart) {
+ print MAIL "Content-Type: multipart/alternative;\n";
+ print MAIL "\tboundary=\"$boundary\"\n";
+ print MAIL "\n";
+ print MAIL "This is a multi-part message in MIME format.\n\n";
+ print MAIL "--$boundary\n";
+ }
if (defined($text)) {
print MAIL "Content-Type: text/plain;\n";
- print MAIL "\tcharset=\"UTF8\"\n";
+ print MAIL "\tcharset=\"UTF-8\"\n";
print MAIL "Content-Transfer-Encoding: 8bit\n";
print MAIL "\n";
@@ -1486,18 +1501,18 @@ sub sendmail {
print MAIL $text;
- print MAIL "\n--$boundary\n";
+ print MAIL "\n--$boundary\n" if $is_multipart;
}
if (defined($html)) {
print MAIL "Content-Type: text/html;\n";
- print MAIL "\tcharset=\"UTF8\"\n";
+ print MAIL "\tcharset=\"UTF-8\"\n";
print MAIL "Content-Transfer-Encoding: 8bit\n";
print MAIL "\n";
print MAIL $html;
- print MAIL "\n--$boundary--\n";
+ print MAIL "\n--$boundary--\n" if $is_multipart;
}
close(MAIL);
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [pve-devel] [PATCH common v2 2/2] sendmail helper: allow empty display name in from
2020-09-03 12:09 [pve-devel] [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 1/2] sendmail-helper: only send multipart if necessary Stoiko Ivanov
@ 2020-09-03 12:09 ` Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH manager v2 1/1] use PVE::Tools::sendmail for update notifications Stoiko Ivanov
2020-09-07 8:06 ` [pve-devel] applied-series: [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt " Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2020-09-03 12:09 UTC (permalink / raw)
To: pve-devel
This patch enables the sendmail helper sub to send emails with a non-existant
display name in the from address. This is used to replace the direct call to
the sendmail binary in pve-manager/PVE/API2/APT.pm.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PVE/Tools.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 3b5b199..e04b504 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1457,7 +1457,7 @@ sub sendmail {
die "illegal character in mailfrom address\n"
if $mailfrom =~ $mail_re;
- $author = $author || 'Proxmox VE';
+ $author = $author // 'Proxmox VE';
open (MAIL, "|-", "sendmail", "-B", "8BITMIME", "-f", $mailfrom, "--", @$mailto) ||
die "unable to open 'sendmail' - $!";
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [pve-devel] [PATCH manager v2 1/1] use PVE::Tools::sendmail for update notifications
2020-09-03 12:09 [pve-devel] [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 1/2] sendmail-helper: only send multipart if necessary Stoiko Ivanov
2020-09-03 12:09 ` [pve-devel] [PATCH common v2 2/2] sendmail helper: allow empty display name in from Stoiko Ivanov
@ 2020-09-03 12:09 ` Stoiko Ivanov
2020-09-07 8:06 ` [pve-devel] applied-series: [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt " Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2020-09-03 12:09 UTC (permalink / raw)
To: pve-devel
instead of calling /usr/sbin/sendmail directly.
Tested with:
```
echo "[]" > /var/lib/pve-manager/pkgupdates
pvesh create /nodes/rosa/apt/update --notify 1
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
PVE/API2/APT.pm | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 859c5f42..984c8103 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -344,15 +344,9 @@ __PACKAGE__->register_method({
my $hostname = `hostname -f` || PVE::INotify::nodename();
chomp $hostname;
my $mailfrom = $dcconf->{email_from} || "root";
+ my $subject = "New software packages available ($hostname)";
- my $data = "Content-Type: text/plain;charset=\"UTF8\"\n";
- $data .= "Content-Transfer-Encoding: 8bit\n";
- $data .= "FROM: <$mailfrom>\n";
- $data .= "TO: $mailto\n";
- $data .= "SUBJECT: New software packages available ($hostname)\n";
- $data .= "\n";
-
- $data .= "The following updates are available:\n\n";
+ my $data = "The following updates are available:\n\n";
my $count = 0;
foreach my $p (sort {$a->{Package} cmp $b->{Package} } @$pkglist) {
@@ -365,14 +359,9 @@ __PACKAGE__->register_method({
}
}
- return if !$count;
-
- my $fh = IO::File->new("|sendmail -B 8BITMIME -f $mailfrom $mailto") ||
- die "unable to open 'sendmail' - $!";
-
- print $fh $data;
+ return if !$count;
- $fh->close() || die "unable to close 'sendmail' - $!";
+ PVE::Tools::sendmail($mailto, $subject, $data, undef, $mailfrom, '');
foreach my $pi (@$pkglist) {
$pi->{NotifyStatus} = $pi->{Version};
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [pve-devel] applied-series: [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications
2020-09-03 12:09 [pve-devel] [PATCH common/manager v2] adapt PVE::Tools::sendmail to match rust-implementation and use it for apt update notifications Stoiko Ivanov
` (2 preceding siblings ...)
2020-09-03 12:09 ` [pve-devel] [PATCH manager v2 1/1] use PVE::Tools::sendmail for update notifications Stoiko Ivanov
@ 2020-09-07 8:06 ` Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2020-09-07 8:06 UTC (permalink / raw)
To: Proxmox VE development discussion, Stoiko Ivanov
On 03.09.20 14:09, Stoiko Ivanov wrote:
> v1->v2:
> * dropped the moving of the email regex for reuse in the sendmail helper:
> we use local usernames (mostly 'root') quite extensively in our code-base and
> our users probably do so as well (for backup notifications)
> * replaced the direct invocation of /usr/sbin/sendmail by a call to the helper
> in pve-manager/PVE/API2/APT.pm
> * adapted the sendmail-helper to allow for an empty display name in the from
> header, since the apt update notification mail got sent without one.
>
> The second patch in pve-common can be squashed into the first one, if
> preferred.
The separation in two patches as you did was good and warranted.
>
> original cover-letter for v1:
> The 2 patches adapt PVE::Tools::sendmail to closely match the recently merged
> implementation in our rust repository - see [0].
>
> I moved the email regex from JSONSchema to Tools to reuse it for the sendmail
> function (and eliminate one of the few email-address regexes in our codebase).
>
> I did not add a dependency on libtimedate-perl (where Date::Format is), since
> we already use Date::Parse in PVE::Certificate, without explicit dependency,
> and it gets pulled in via libwww-perl -> libhttp-date-perl -> libtimedate-perl.
>
> Glad to send an update for the dependency of course.
should still note that, depending on indirect dependency chains isn't to ideal.
>
> [0] https://lists.proxmox.com/pipermail/pbs-devel/2020-August/000423.html
>
> Stoiko Ivanov (2):
> move email regex from JSONSchema to Tools
> sendmail-helper: only send multipart if necessary
>
> src/PVE/JSONSchema.pm | 4 ++--
> src/PVE/Tools.pm | 49 +++++++++++++++++++++++++++++--------------
> 2 files changed, 35 insertions(+), 18 deletions(-)
>
> pve-common:
> Stoiko Ivanov (2):
> sendmail-helper: only send multipart if necessary
> sendmail helper: allow empty display name in from
>
> src/PVE/Tools.pm | 43 +++++++++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 14 deletions(-)
>
> pve-manager:
> Stoiko Ivanov (1):
> use PVE::Tools::sendmail for update notifications
>
> PVE/API2/APT.pm | 19 ++++---------------
> 1 file changed, 4 insertions(+), 15 deletions(-)
>
applied series, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread