[pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions

[pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions

[Stoiko Ivanov]

while reproducing #6126 (thanks to Hannes D. for pointing me there) I
remembered a few other cases where users were asking how to find out what
happened to a particular mail that was quarantined.

Changes were minimally tested on my setup.

Stoiko Ivanov (2):
  quarantine: add receiver to delivery/delete log-messages
  fix #6126: do not deliver/delete quarantined mails multiple times

 src/PMG/Quarantine.pm | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 1/2] quarantine: add receiver to delivery/delete log-messages

[Stoiko Ivanov]

The question: "What happened to the quarantined e-mail" comes up every
now and then in our support-channels, and it always feels a bit clumsy
to explain how to get to the fitting logline, by referring to the
general timeframe when a user did the request and to look for the
queue-ids and quarantine-file-ids for identifying a particular mail.

Adding the receiver of the mail to the log lines should make this a
bit more straight-forward.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Quarantine.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/PMG/Quarantine.pm b/src/PMG/Quarantine.pm
index d06a88d..28138ba 100644
--- a/src/PMG/Quarantine.pm
+++ b/src/PMG/Quarantine.pm
@@ -128,12 +128,12 @@ sub deliver_quarantined_mail {
     };
     my $err = $@;
     if ($err) {
-	my $msg = "deliver quarantined mail '$id' ($path) failed: $err";
+	my $msg = "deliver quarantined mail '$id' ($path) to $receiver failed: $err";
 	syslog('err', $msg);
 	die "$msg\n";
     }
 
-    syslog('info', "delivered quarantined mail '$id' ($path)");
+    syslog('info', "delivered quarantined mail '$id' ($path) to $receiver");
 
     return 1;
 }
@@ -141,6 +141,7 @@ sub deliver_quarantined_mail {
 sub delete_quarantined_mail {
     my ($dbh, $ref) = @_;
 
+    my $pmail = $ref->{pmail};
     my $filename = $ref->{file};
     my $spooldir = $PMG::MailQueue::spooldir;
     my $path = "$spooldir/$filename";
@@ -155,12 +156,12 @@ sub delete_quarantined_mail {
 	$sth->finish;
     };
     if (my $err = $@) {
-	my $msg = "delete quarantined mail '$id' ($path) failed: $err";
+	my $msg = "delete quarantined mail '$id' ($path) for $pmail failed: $err";
 	syslog ('err', $msg);
 	die "$msg\n";
     }
 
-    syslog ('info', "marked quarantined mail '$id' as deleted ($path)");
+    syslog ('info', "marked quarantined mail '$id' as deleted ($path) for $pmail");
 
     return 1;
 }
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api 2/2] fix #6126: do not deliver/delete quarantined mails multiple times

[Stoiko Ivanov]

This was reported through our enterprise support and was easy to
reproduce.

When clicking on the action-links in the spamreport multiple times the
action was equally done multiple times, which was surprising
especially when you deliver a quarantined mail more than once.

As quarantined mail-files are not deleted upon deliver/delete (mails
can have multiple recipients), but asynchronously after the quarantine
lifetime they are only marked with status 'D'.

Use this information to prevent multiple deliveries and superfluous
database updates.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Quarantine.pm | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/PMG/Quarantine.pm b/src/PMG/Quarantine.pm
index 28138ba..77f64a0 100644
--- a/src/PMG/Quarantine.pm
+++ b/src/PMG/Quarantine.pm
@@ -95,6 +95,12 @@ sub deliver_quarantined_mail {
 
     my $id = 'C' . $ref->{cid} . 'R' . $ref->{rid} . 'T' . $ref->{ticketid};;
 
+    if ($ref->{status} eq 'D') {
+	syslog('info', "quarantined mail '$id' ($path) has already been delivered or marked as " .
+	    "deleted for $receiver!");
+	return 1;
+    }
+
     my $parser = PMG::MIMEUtils::new_mime_parser({
 	nested => 1,
 	decode_bodies => 0,
@@ -148,6 +154,12 @@ sub delete_quarantined_mail {
 
     my $id = 'C' . $ref->{cid} . 'R' . $ref->{rid} . 'T' . $ref->{ticketid};;
 
+    if ($ref->{status} eq 'D') {
+	syslog('info', "quarantined mail '$id' ($path) has already been delivered or marked as " .
+	    "deleted for $pmail");
+	return 1;
+    }
+
     eval {
 	my $sth = $dbh->prepare(
 	    "UPDATE CMSReceivers SET Status='D', MTime = ? WHERE " .
-- 
2.39.5



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel