Remove duplicate suppression for smtp?
-
@jimp, Per request I am following up on Redmine issue 14031 here in the forum. I am interested in exploring alternatives that might allow notify_all_remote() to continue to be used by non root entities.
In reading the code, it appears that the queue, which is what is requiring root privilege, exists only to suppress duplicate sends for the last smtp message. The first question that comes to mind, is the duplicate suppression system for smtp messages still necessary? Could it simply be removed?
It seems like a group of differing notifications generally happen at nearly the same time, so it's a bit unclear how many duplicates are actually being suppressed. Also, since we are not suppressing duplicates for any of the other notification delivery systems (pushover, slack, telegram), doing this for smtp only seems a bit strange.
Setting that aside, if removing the duplicate suppression system altogether seems too much, what about performing the writability check into notify_via_smtp() rather than in notices_sendqueue()? With this approach, if the file is not writable, notify_via_smtp() could simply call send_smtp_message() directly. This would safely restore the prior functionality.
The believe the following code addresses the entire issue, including the original spamming problem, in a simple way:
--- notices.inc.old 2023-04-23 09:44:28.161806000 -0700 +++ notices.inc 2023-04-26 11:58:18.546586000 -0700 @@ -356,7 +356,9 @@ /* Store last message sent to avoid spamming */ @file_put_contents("/var/db/notices_lastmsg.txt", $message); - if (!$force) { + if ($force == false && + file_exists("{$g['vardb_path']}/notifyqueue.messages") && + is_writable("{$g['vardb_path']}/notifyqueue.messages")) { notify_via_queue_add($message, 'mail'); $ret = true; } else {
Thoughts?
-