* [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support
@ 2023-03-17 18:44 Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail Stoiko Ivanov
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
Thanks to Dominik's feedback and testing this series so well we found one
issue:
* reinject_local_mail does encode the envelope-addresses (if they are not
purely ascii) - but the quarantine delivery passes the data already utf-8
encoded to it (since it's read this way from the database)
This would have been an issue previously as well - but since the
quarantine delivery never got smtputf8 support it was not noticed.
We discussed the rest of the feedback off-list - to summarize it:
* reinject_local_mail already does take the smtputf8 setting from pmg.conf
into consideration (pmg-api patch 2/5)
changes v2 -> v3:
* patch 5/5 for pmg-api was added to properly decode database results
before quarantine delivery or adding addresses to the user welcomelist
or blocklists
* other patches are left as they were
original cover-letter for v2:
This series is the improved second part of:
https://lists.proxmox.com/pipermail/pmg-devel/2023-January/002257.html
and incorporates a few other improvments, which came up during implementation.
Changes from v1:
1 dropped the already applied and released stop-gap fix
2 added 'reinject_local_mail` helper to scan and check the mail, instead
of checking with mail_needs_smtputf8 at all call-sites (Thanks to Thomas
for the suggestion!)
3 while add it went ahead and added smtputf8 as new option to the mail section
of pmg.conf (reason for the docs and gui patch)
4 during testing noted the duplication of reinject_mail in
deliver_quarantined_mail (with the latter not receiving any of the
improvments for the former) leading to the refactoring in api-patch 3 and 4
point 4 is independent of the rest, but review should be easier after looking
through the other patches.
the docs-patch is needed (as build-depends), due to the new pmg.conf option
original cover-letter for v1:
This series addresses an issue some of our users reported in our
community forum with pmg-api_7.2-3:
* the decision if smtputf8 is needed is based on the envelope-addresses
and the headers (if they contain non-ascii characters we use SMTPUTF8
* this breaks environments where SMTPUTF8 is disabled (mostly because
some downstream servers do not support this), but mails still contain
non-ascii data (while this is against the relevant rfc, which say that
header-data must contain only ascii characters (and should be encoded
with MIME-words otherwise), it is seemingly quite common in the wild)
one testmail I got from a user from the forum had the From header
correctly encoded, but added an X-DFrom header with the unencoded from.
The first patch simply drops the header-inspection and should enable
most of the reporters to receive mail again.
The second patch tries to address the smtputf8 issue a bit differently
than what we currently do
- For mails received via SMTP it simply sets SMTPUTF8 if the original
postfix processes sent pmg-smtp-filter the mail with the flag, and
does not set it otherwise
- For locally generated mail it detects if its needed by checking the
envelope-addresses and the headers for non-ascii characters.
This should follow postfix own functioning quite closely:
https://www.postfix.org/SMTPUTF8_README.html
processing by pmg-smtp-filter should not change the need for the flag,
since we don't rewrite envelope-addresses, and modify filed does
mime-encode the resulting header.
Sending as two patches, since the first one would be good to get out
soon (as it's affecting a few setups), while the second one might
benefit from a bit more testing (I did some tests, which all looked
good, but might have overlooked some cases)
pmg-api:
Stoiko Ivanov (5):
smtputf8: keep smtputf8 from incoming postfix, detect for local mail
config: make smtputf8 configurable through the API
reinject mail: improve error logging
quarantine: use reinject_local_mail to deliver quarantined mail
api: quarantine: decode addresses before delivery/userlisting
src/PMG/API2/Quarantine.pm | 12 ++++---
src/PMG/Config.pm | 7 +++++
src/PMG/Quarantine.pm | 64 +++++++++-----------------------------
src/PMG/RuleDB/Notify.pm | 2 +-
src/PMG/SMTP.pm | 3 +-
src/PMG/Utils.pm | 53 +++++++++++++++++++++----------
src/templates/main.cf.in | 4 +++
7 files changed, 73 insertions(+), 72 deletions(-)
pmg-docs:
Stoiko Ivanov (1):
doc-generator: add new option smtputf8
gen-pmg.conf.5-opts.pl | 1 +
1 file changed, 1 insertion(+)
pmg-gui:
Stoiko Ivanov (1):
mail proxy options: add smtputf8 checkbox
js/MailProxyOptions.js | 3 +++
1 file changed, 3 insertions(+)
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 2/5] config: make smtputf8 configurable through the API Stoiko Ivanov
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
This patch changes the detection if smtputf8 is needed as option to
the 'MAIL' command:
* for mail arriving through postfix it is only added if the mail
originally was received with it (Accept and BCC actions)
* for locally generated mail (Notify, reports, quarantine-link and
ndrs) it is decided based on utf8 characters in the mail-addresses
or headers - this is done by `reinject_local_mail`, as a new helper
This should match postfix own behavior in those cases quite
closely:
https://www.postfix.org/SMTPUTF8_README.html#using
Notable difference is that we check the complete e-mail address and
not only the domain part, but I assume non-ascii local-parts to be a
very fringe edge-case in environments where smtputf8 is not supported.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/API2/Quarantine.pm | 2 +-
src/PMG/RuleDB/Notify.pm | 2 +-
src/PMG/SMTP.pm | 3 ++-
src/PMG/Utils.pm | 28 ++++++++++++++++++++--------
4 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/PMG/API2/Quarantine.pm b/src/PMG/API2/Quarantine.pm
index fbb302a..6318082 100644
--- a/src/PMG/API2/Quarantine.pm
+++ b/src/PMG/API2/Quarantine.pm
@@ -1239,7 +1239,7 @@ my sub send_link_mail {
);
# we use an empty envelope sender (we don't want to receive NDRs)
- PMG::Utils::reinject_mail ($mail, '', [$receiver], undef, $fqdn);
+ PMG::Utils::reinject_local_mail ($mail, '', [$receiver], undef, $fqdn);
}
__PACKAGE__->register_method ({
diff --git a/src/PMG/RuleDB/Notify.pm b/src/PMG/RuleDB/Notify.pm
index 68f9b4e..c024840 100644
--- a/src/PMG/RuleDB/Notify.pm
+++ b/src/PMG/RuleDB/Notify.pm
@@ -256,7 +256,7 @@ sub execute {
print $fh "notify end\n";
} else {
my @targets = split(/\s*,\s*/, $to);
- my $qid = PMG::Utils::reinject_mail(
+ my $qid = PMG::Utils::reinject_local_mail(
$top, $from, \@targets, undef, $msginfo->{fqdn});
foreach (@targets) {
my $target = encode('UTF-8', $_);
diff --git a/src/PMG/SMTP.pm b/src/PMG/SMTP.pm
index fbf5c95..b7bc5d3 100644
--- a/src/PMG/SMTP.pm
+++ b/src/PMG/SMTP.pm
@@ -111,6 +111,7 @@ sub loop {
$self->{param}->{mail}->{$1} = $2;
} elsif ($opt =~ m/smtputf8/i) {
$self->{smtputf8} = 1;
+ $self->{param}->{mail}->{smtputf8} = 1;
$from = decode('UTF-8', $from);
} else {
#ignore everything else
@@ -314,7 +315,7 @@ EOF
Encoding => '7bit',
Description => 'Delivery report');
- my $qid = PMG::Utils::reinject_mail($ndr, '', [$sender], undef, $hostname);
+ my $qid = PMG::Utils::reinject_local_mail($ndr, '', [$sender], undef, $hostname);
if ($qid) {
syslog('info', "sent NDR for rejecting recipients - $qid");
} else {
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 9c6f841..49939c4 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -221,18 +221,28 @@ sub subst_values_for_header {
return $res;
}
-sub mail_needs_smtputf8 {
- my ($entity, $sender, $targets) = @_;
+# detects the need for setting smtputf8 based on addresses and headers
+sub reinject_local_mail {
+ my ($entity, $sender, $targets, $xforward, $me, $params) = @_;
+
+ my $needs_smtputf8 = 0;
- return 1 if ($sender =~ /[^\p{PosixPrint}]/);
+ $needs_smtputf8 = 1 if ($sender =~ /[^\p{PosixPrint}]/);
foreach my $target (@$targets) {
if ($target =~ /[^\p{PosixPrint}]/) {
- return 1;
+ $needs_smtputf8 = 1;
+ last;
}
}
- return 0;
+ if (!$needs_smtputf8 && $entity->head()->as_string() =~ /([^\p{PosixPrint}\n\r\t])/) {
+ $needs_smtputf8 = 1;
+ }
+
+ $params->{mail}->{smtputf8} = $needs_smtputf8;
+
+ return reinject_mail($entity, $sender, $targets, $xforward, $me, $params);
}
sub reinject_mail {
@@ -260,10 +270,12 @@ sub reinject_mail {
}
my $mail_opts = " BODY=8BITMIME";
- $mail_opts .= " SMTPUTF8" if mail_needs_smtputf8($entity, $sender, $targets);
my $sender_addr = encode('UTF-8', $smtp->_addr($sender));
-
if (defined($params->{mail})) {
+ if (delete $params->{mail}->{smtputf8}) {
+ $mail_opts .= " SMTPUTF8";
+ }
+
my $mailparams = $params->{mail};
for my $p (keys %$mailparams) {
$mail_opts .= " $p=$mailparams->{$p}";
@@ -1258,7 +1270,7 @@ sub finalize_report {
return;
}
# we use an empty envelope sender (we don't want to receive NDRs)
- PMG::Utils::reinject_mail ($top, '', [$receiver], undef, $data->{fqdn});
+ PMG::Utils::reinject_local_mail ($top, '', [$receiver], undef, $data->{fqdn});
}
sub lookup_timespan {
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 2/5] config: make smtputf8 configurable through the API
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 3/5] reinject mail: improve error logging Stoiko Ivanov
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
the flag is simply a boolean which is used to:
* add smtputf8_enable = no to postfix' main.cf if it is disabled
(the default is to enable it, and not adding it unconditionally,
should cause the fewest surprises for users with modified templates)
* decide if locally generated mail should be scanned for utf8 headers
and addresses (to set the parameter to the MAIL command)
This should match postfix own implementation w.r.t. smtputf8 behavior.
Additionally, since quite a few users need to disable it because
their downstream servers do not support it (Zimbra, OpenXchange,
MS Exchange), this should make for a better user experience.
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/Config.pm | 7 +++++++
src/PMG/Utils.pm | 29 +++++++++++++++++------------
src/templates/main.cf.in | 4 ++++
3 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index 699a622..f13bde9 100755
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -682,6 +682,11 @@ sub properties {
type => 'boolean',
default => 0
},
+ smtputf8 => {
+ description => "Enable SMTPUTF8 support in Postfix and detection for locally generated mail",
+ type => 'boolean',
+ default => 1
+ },
};
}
@@ -722,6 +727,7 @@ sub options {
dnsbl_threshold => { optional => 1 },
before_queue_filtering => { optional => 1 },
ndr_on_block => { optional => 1 },
+ smtputf8 => { optional => 1 },
};
}
@@ -1710,6 +1716,7 @@ my $pmg_service_params = {
mail => {
hide_received => 1,
ndr_on_block => 1,
+ smtputf8 => 1,
},
admin => {
dkim_selector => 1,
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 49939c4..37e76aa 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -221,27 +221,32 @@ sub subst_values_for_header {
return $res;
}
-# detects the need for setting smtputf8 based on addresses and headers
+# detects the need for setting smtputf8 based on pmg.conf, addresses and headers
sub reinject_local_mail {
- my ($entity, $sender, $targets, $xforward, $me, $params) = @_;
+ my ($entity, $sender, $targets, $xforward, $me) = @_;
+
+ my $cfg = PMG::Config->new();
- my $needs_smtputf8 = 0;
+ my $params;
+ if ( $cfg->get('mail', 'smtputf8' )) {
+ my $needs_smtputf8 = 0;
- $needs_smtputf8 = 1 if ($sender =~ /[^\p{PosixPrint}]/);
+ $needs_smtputf8 = 1 if ($sender =~ /[^\p{PosixPrint}]/);
+
+ foreach my $target (@$targets) {
+ if ($target =~ /[^\p{PosixPrint}]/) {
+ $needs_smtputf8 = 1;
+ last;
+ }
+ }
- foreach my $target (@$targets) {
- if ($target =~ /[^\p{PosixPrint}]/) {
+ if (!$needs_smtputf8 && $entity->head()->as_string() =~ /([^\p{PosixPrint}\n\r\t])/) {
$needs_smtputf8 = 1;
- last;
}
- }
- if (!$needs_smtputf8 && $entity->head()->as_string() =~ /([^\p{PosixPrint}\n\r\t])/) {
- $needs_smtputf8 = 1;
+ $params->{mail}->{smtputf8} = $needs_smtputf8;
}
- $params->{mail}->{smtputf8} = $needs_smtputf8;
-
return reinject_mail($entity, $sender, $targets, $xforward, $me, $params);
}
diff --git a/src/templates/main.cf.in b/src/templates/main.cf.in
index 190c913..46a5b6e 100644
--- a/src/templates/main.cf.in
+++ b/src/templates/main.cf.in
@@ -131,6 +131,10 @@ lmtp_tls_session_cache_database = btree:/var/lib/postfix/lmtp_tls_session_cache
unverified_recipient_reject_reason = Recipient address lookup failed
[% END %]
+[% IF ! pmg.mail.smtputf8 %]
+smtputf8_enable = no
+[% END %]
+
default_destination_concurrency_limit = 40
lmtp_destination_concurrency_limit = 20
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 3/5] reinject mail: improve error logging
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 2/5] config: make smtputf8 configurable through the API Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 4/5] quarantine: use reinject_local_mail to deliver quarantined mail Stoiko Ivanov
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
this patch unifies the error handling for mail and rcpt with
the data command: all now die with sensible error (which gets logged
in the error-handling of the eval), and it sets the respose message
and code for those commands as well.
additionally it adds a '\n' to all die statements.
this makes it possible to provide information what went wrong at
call-sites (instead of only having it in syslog)
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/Utils.pm | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
index 37e76aa..6405934 100644
--- a/src/PMG/Utils.pm
+++ b/src/PMG/Utils.pm
@@ -288,8 +288,10 @@ sub reinject_mail {
}
if (!$smtp->_MAIL("FROM:" . $sender_addr . $mail_opts)) {
- syslog('err', "smtp error - got: %s %s", $smtp->code, scalar ($smtp->message));
- die "smtp from: ERROR";
+ my @msgs = $smtp->message;
+ $resmess = $msgs[$#msgs];
+ $rescode = $smtp->code;
+ die sprintf("smtp from error - got: %s %s\n", $rescode, $resmess);
}
foreach my $target (@$targets) {
@@ -304,8 +306,10 @@ sub reinject_mail {
$rcpt_addr = encode('UTF-8', $smtp->_addr($target));
if (!$smtp->_RCPT("TO:" . $rcpt_addr . $rcpt_opts)) {
- syslog ('err', "smtp error - got: %s %s", $smtp->code, scalar($smtp->message));
- die "smtp to: ERROR";
+ my @msgs = $smtp->message;
+ $resmess = $msgs[$#msgs];
+ $rescode = $smtp->code;
+ die sprintf("smtp to error - got: %s %s\n", $rescode, $resmess);
}
}
@@ -326,13 +330,13 @@ sub reinject_mail {
($resid) = $resmess =~ m/Ok: queued as ([0-9A-Z]+)/;
$rescode = $smtp->code;
if (!$resid) {
- die sprintf("unexpected SMTP result - got: %s %s : WARNING", $smtp->code, $resmess);
+ die sprintf("unexpected SMTP result - got: %s %s : WARNING\n", $smtp->code, $resmess);
}
} else {
my @msgs = $smtp->message;
$resmess = $msgs[$#msgs];
$rescode = $smtp->code;
- die sprintf("sending data failed - got: %s %s : ERROR", $smtp->code, $resmess);
+ die sprintf("sending data failed - got: %s %s : ERROR\n", $smtp->code, $resmess);
}
};
my $err = $@;
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 4/5] quarantine: use reinject_local_mail to deliver quarantined mail
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (2 preceding siblings ...)
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 3/5] reinject mail: improve error logging Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 5/5] api: quarantine: decode addresses before delivery/userlisting Stoiko Ivanov
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
the current delivery looks quite similar to reinject_local_mail,
apart from the database handling and sending the mail-contents from a
file instead of a MIME::Entity.
reinject_mail has received a few improvments over time, which never
made it to this implementation - e.g. in:
* ebd31d3e74d9417375b86766ee300be493044d39
* ad1c6bcea94cbaf8d4862bcb05874b59c656c632
While reparsing the mail might seem expensive, the quarantine code
does so multiple times when users click in the quarantine GUI (see
PMG::HTMLMail, and the attachment quarantine)
The issue of MIME::Parser being lossy [0] (parsing and then printing
the entity, might not return the original mail byte-by-byte), is
already present in our code-base anyways (when the mail gets
quarantined (or sent on) it is from a parsed MIME::Entity).
[0] https://metacpan.org/pod/MIME::Tools
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/Quarantine.pm | 64 ++++++++++---------------------------------
1 file changed, 15 insertions(+), 49 deletions(-)
diff --git a/src/PMG/Quarantine.pm b/src/PMG/Quarantine.pm
index aa6b948..bd5e63b 100644
--- a/src/PMG/Quarantine.pm
+++ b/src/PMG/Quarantine.pm
@@ -2,7 +2,6 @@ package PMG::Quarantine;
use strict;
use warnings;
-use Net::SMTP;
use Encode qw(encode);
use PVE::SafeSyslog;
@@ -11,6 +10,7 @@ use PVE::Tools;
use PMG::Utils;
use PMG::RuleDB;
use PMG::MailQueue;
+use PMG::MIMEUtils;
sub add_to_blackwhite {
my ($dbh, $username, $listname, $addrs, $delete) = @_;
@@ -98,55 +98,24 @@ sub deliver_quarantined_mail {
my $id = 'C' . $ref->{cid} . 'R' . $ref->{rid} . 'T' . $ref->{ticketid};;
- my $sender = 'postmaster'; # notify postmaster if something fails
-
- my $smtp;
-
- eval {
- my $smtp = Net::SMTP->new ('127.0.0.1', Port => 10025, Hello => 'quarantine') ||
- die "unable to connect to localhost at port 10025\n";
-
- my $resid;
-
- if (!$smtp->mail($sender)) {
- die sprintf("smtp from error - got: %s %s\n", $smtp->code, $smtp->message);
- }
-
- if (!$smtp->to($receiver)) {
- die sprintf("smtp to error - got: %s %s\n", $smtp->code, $smtp->message);
- }
+ my $parser = PMG::MIMEUtils::new_mime_parser({
+ nested => 1,
+ decode_bodies => 0,
+ extract_uuencode => 0,
+ dumpdir => "/tmp/.quarantine-$id-$receiver-$$/",
+ });
- $smtp->data();
+ my $entity = $parser->parse_open("$path");
+ PMG::MIMEUtils::fixup_multipart($entity);
- my $header = 1;
-
- open(my $fh, '<', $path) || die "unable to open file '$path' - $!\n";
-
- while (defined(my $line = <$fh>)) {
- chomp $line;
- if ($header && ($line =~ m/^\s*$/)) {
- $header = 0;
- }
+ my $sender = 'postmaster'; # notify postmaster if something fails
- # skip Delivered-To and Return-Path (avoid problem with postfix
- # forwarding loop detection (man local))
- next if ($header && (($line =~ m/^Delivered-To:/i) || ($line =~ m/^Return-Path:/i)));
+ eval {
+ my ($qid, $code, $mess) = PMG::Utils::reinject_local_mail(
+ $entity, $sender, [$receiver], undef, 'quarantine');
- # rfc821 requires this
- $line =~ s/^\./\.\./mg;
- $smtp->datasend("$line\n");
- }
- close($fh);
-
- if ($smtp->dataend()) {
- my (@msgs) = $smtp->message;
- my ($last_msg) = $msgs[$#msgs];
- ($resid) = $last_msg =~ m/Ok: queued as ([0-9A-Z]+)/;
- if (!$resid) {
- die sprintf("smtp error - got: %s %s\n", $smtp->code, $smtp->message);
- }
- } else {
- die sprintf("sending data failed - got: %s %s\n", $smtp->code, $smtp->message);
+ if (!$qid) {
+ die "$mess\n";
}
my $sth = $dbh->prepare(
@@ -156,9 +125,6 @@ sub deliver_quarantined_mail {
$sth->finish;
};
my $err = $@;
-
- $smtp->quit if $smtp;
-
if ($err) {
my $msg = "deliver quarantined mail '$id' ($path) failed: $err";
syslog('err', $msg);
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-api v3 5/5] api: quarantine: decode addresses before delivery/userlisting
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (3 preceding siblings ...)
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 4/5] quarantine: use reinject_local_mail to deliver quarantined mail Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-gui v3 1/1] mail proxy options: add smtputf8 checkbox Stoiko Ivanov
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
With the change of using reinject_local_mail for the quarantine
delivery the issue of not properly decoding the entries we get from
the database before delivering became apparent
The database returns utf-8 encoded strings, reinject_local_mail and
add_to_blackwhite expects perl-strings (with wide characters) and
encodes them (a second time) - this patch decodes the database strings
before passing it on.
add_to_black_white is used in a few API calls (via
read_or_modify_user_bw_list), therefore the approach of decode (from
database), and encode (for database) was chosen.
Reported-by: Dominik Csapak <d.csapak@proxomox.com>
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
src/PMG/API2/Quarantine.pm | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/PMG/API2/Quarantine.pm b/src/PMG/API2/Quarantine.pm
index 6318082..7101795 100644
--- a/src/PMG/API2/Quarantine.pm
+++ b/src/PMG/API2/Quarantine.pm
@@ -1182,15 +1182,17 @@ __PACKAGE__->register_method ({
my $ref = $get_and_check_mail->($id, $rpcenv, $dbh);
my $sender = try_decode_utf8($get_real_sender->($ref));
+ my $pmail = try_decode_utf8($ref->{pmail});
+ my $receiver = try_decode_utf8($ref->{receiver} // $ref->{pmail});
if ($action eq 'whitelist') {
- PMG::Quarantine::add_to_blackwhite($dbh, $ref->{pmail}, 'WL', [ $sender ]);
- PMG::Quarantine::deliver_quarantined_mail($dbh, $ref, $ref->{receiver} // $ref->{pmail});
+ PMG::Quarantine::add_to_blackwhite($dbh, $pmail, 'WL', [ $sender ]);
+ PMG::Quarantine::deliver_quarantined_mail($dbh, $ref, $receiver);
} elsif ($action eq 'blacklist') {
- PMG::Quarantine::add_to_blackwhite($dbh, $ref->{pmail}, 'BL', [ $sender ]);
+ PMG::Quarantine::add_to_blackwhite($dbh, $pmail, 'BL', [ $sender ]);
PMG::Quarantine::delete_quarantined_mail($dbh, $ref);
} elsif ($action eq 'deliver') {
- PMG::Quarantine::deliver_quarantined_mail($dbh, $ref, $ref->{receiver} // $ref->{pmail});
+ PMG::Quarantine::deliver_quarantined_mail($dbh, $ref, $receiver);
} elsif ($action eq 'delete') {
PMG::Quarantine::delete_quarantined_mail($dbh, $ref);
} else {
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-gui v3 1/1] mail proxy options: add smtputf8 checkbox
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (4 preceding siblings ...)
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 5/5] api: quarantine: decode addresses before delivery/userlisting Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-docs v3 1/1] doc-generator: add new option smtputf8 Stoiko Ivanov
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
js/MailProxyOptions.js | 3 +++
1 file changed, 3 insertions(+)
diff --git a/js/MailProxyOptions.js b/js/MailProxyOptions.js
index b151350..7c0b7c6 100644
--- a/js/MailProxyOptions.js
+++ b/js/MailProxyOptions.js
@@ -80,6 +80,9 @@ Ext.define('PMG.MailProxyOptions', {
me.add_boolean_row('before_queue_filtering', gettext('Before Queue Filtering'));
+ me.add_boolean_row('smtputf8', gettext('SMTPUTF8 support'),
+ { defaultValue: 1 });
+
var baseurl = '/config/mail';
me.selModel = Ext.create('Ext.selection.RowModel', {});
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] [PATCH pmg-docs v3 1/1] doc-generator: add new option smtputf8
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (5 preceding siblings ...)
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-gui v3 1/1] mail proxy options: add smtputf8 checkbox Stoiko Ivanov
@ 2023-03-17 18:44 ` Stoiko Ivanov
2023-03-23 11:18 ` [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Dominik Csapak
2023-03-27 10:33 ` [pmg-devel] applied-series: " Thomas Lamprecht
8 siblings, 0 replies; 10+ messages in thread
From: Stoiko Ivanov @ 2023-03-17 18:44 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
gen-pmg.conf.5-opts.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/gen-pmg.conf.5-opts.pl b/gen-pmg.conf.5-opts.pl
index 23cad7c..1aff46a 100755
--- a/gen-pmg.conf.5-opts.pl
+++ b/gen-pmg.conf.5-opts.pl
@@ -42,6 +42,7 @@ my $key_groups = {
banner => 1,
before_queue_filtering => 1,
ndr_on_block => 1,
+ smtputf8 => 1,
}],
'mail-tls' => [
'mail' , {
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (6 preceding siblings ...)
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-docs v3 1/1] doc-generator: add new option smtputf8 Stoiko Ivanov
@ 2023-03-23 11:18 ` Dominik Csapak
2023-03-27 10:33 ` [pmg-devel] applied-series: " Thomas Lamprecht
8 siblings, 0 replies; 10+ messages in thread
From: Dominik Csapak @ 2023-03-23 11:18 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
ok this version fixes my issue from the last one
(accept vs quarantine + deliver)
the second issue i had was non-existant, i did not look
into the correct patch...
so all in all LGTM, consider this series:
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pmg-devel] applied-series: [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
` (7 preceding siblings ...)
2023-03-23 11:18 ` [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Dominik Csapak
@ 2023-03-27 10:33 ` Thomas Lamprecht
8 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2023-03-27 10:33 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
Am 17/03/2023 um 19:44 schrieb Stoiko Ivanov:
> pmg-api:
> Stoiko Ivanov (5):
> smtputf8: keep smtputf8 from incoming postfix, detect for local mail
> config: make smtputf8 configurable through the API
> reinject mail: improve error logging
> quarantine: use reinject_local_mail to deliver quarantined mail
> api: quarantine: decode addresses before delivery/userlisting
>
> src/PMG/API2/Quarantine.pm | 12 ++++---
> src/PMG/Config.pm | 7 +++++
> src/PMG/Quarantine.pm | 64 +++++++++-----------------------------
> src/PMG/RuleDB/Notify.pm | 2 +-
> src/PMG/SMTP.pm | 3 +-
> src/PMG/Utils.pm | 53 +++++++++++++++++++++----------
> src/templates/main.cf.in | 4 +++
> 7 files changed, 73 insertions(+), 72 deletions(-)
>
> pmg-docs:
> Stoiko Ivanov (1):
> doc-generator: add new option smtputf8
>
> gen-pmg.conf.5-opts.pl | 1 +
> 1 file changed, 1 insertion(+)
>
> pmg-gui:
> Stoiko Ivanov (1):
> mail proxy options: add smtputf8 checkbox
>
> js/MailProxyOptions.js | 3 +++
> 1 file changed, 3 insertions(+)
>
Seems I missed to reply here:
applied series, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-03-27 10:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-17 18:44 [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 1/5] smtputf8: keep smtputf8 from incoming postfix, detect for local mail Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 2/5] config: make smtputf8 configurable through the API Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 3/5] reinject mail: improve error logging Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 4/5] quarantine: use reinject_local_mail to deliver quarantined mail Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-api v3 5/5] api: quarantine: decode addresses before delivery/userlisting Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-gui v3 1/1] mail proxy options: add smtputf8 checkbox Stoiko Ivanov
2023-03-17 18:44 ` [pmg-devel] [PATCH pmg-docs v3 1/1] doc-generator: add new option smtputf8 Stoiko Ivanov
2023-03-23 11:18 ` [pmg-devel] [PATCH pmg-api v3 0/5] improve local mail injection and add smtputf8 support Dominik Csapak
2023-03-27 10:33 ` [pmg-devel] applied-series: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox