[pmg-devel] [PATCH pmg-api v2 4/4] quarantine: use reinject_local_mail to deliver quarantined mail

[Stoiko Ivanov <s.ivanov@proxmox.com>]

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