* [pmg-devel] [PATCH pmg-api 1/2] quarantine: add receiver to delivery/delete log-messages
2025-01-30 11:21 [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Stoiko Ivanov
@ 2025-01-30 11:21 ` Stoiko Ivanov
2025-01-30 11:21 ` [pmg-devel] [PATCH pmg-api 2/2] fix #6126: do not deliver/delete quarantined mails multiple times Stoiko Ivanov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2025-01-30 11:21 UTC (permalink / raw)
To: pmg-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pmg-devel] [PATCH pmg-api 2/2] fix #6126: do not deliver/delete quarantined mails multiple times
2025-01-30 11:21 [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Stoiko Ivanov
2025-01-30 11:21 ` [pmg-devel] [PATCH pmg-api 1/2] quarantine: add receiver to delivery/delete log-messages Stoiko Ivanov
@ 2025-01-30 11:21 ` Stoiko Ivanov
2025-02-18 9:37 ` [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Christian Ebner
2025-02-21 16:59 ` [pmg-devel] applied: " Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Stoiko Ivanov @ 2025-01-30 11:21 UTC (permalink / raw)
To: pmg-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions
2025-01-30 11:21 [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Stoiko Ivanov
2025-01-30 11:21 ` [pmg-devel] [PATCH pmg-api 1/2] quarantine: add receiver to delivery/delete log-messages Stoiko Ivanov
2025-01-30 11:21 ` [pmg-devel] [PATCH pmg-api 2/2] fix #6126: do not deliver/delete quarantined mails multiple times Stoiko Ivanov
@ 2025-02-18 9:37 ` Christian Ebner
2025-02-21 16:59 ` [pmg-devel] applied: " Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Christian Ebner @ 2025-02-18 9:37 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
On 1/30/25 12:21, Stoiko Ivanov wrote:
> 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.
Tested these patches by trying to send or delete multiple quarantined
mails via:
```
pmgsh create /quarantine/content --action deliver --id <ID>
pmgsh create /quarantine/content --action delete --id <ID>
```
and looking at the systemd journal outputs as well as the mail in the
stmp sink.
Works as intended, no issues found. Please consider this as
Tested-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pmg-devel] applied: [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions
2025-01-30 11:21 [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Stoiko Ivanov
` (2 preceding siblings ...)
2025-02-18 9:37 ` [pmg-devel] [PATCH pmg-api 0/2] fix #6126 and improve logging for quarantine actions Christian Ebner
@ 2025-02-21 16:59 ` Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2025-02-21 16:59 UTC (permalink / raw)
To: Stoiko Ivanov, pmg-devel
Am 30.01.25 um 12:21 schrieb 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(-)
>
applied series with Chris' T-b, thanks!
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
^ permalink raw reply [flat|nested] 5+ messages in thread