Emails sent multiple times to the same address can be rejected

[TheWitness]

Describe the bug

We recently got into a mailer issue the other day where we had the same Email in a Notification list, and the same Email in a notification_extra using an upper case email. When calling the mailer with this condition, the upstream mail services rejected the Email since it had duplicated Email addresses.

EMAIL@domain.com,email@domain.com

That example was rejected upstream.

Expected behavior

Cacti should per-process the Email list, convert the Emails to lower case, and de-duplicate them prior to sending to the upstream mail service.

[netniV]

Technically, we can't de-dupe that situation. Both are valid independent email addresses according to the RFC.

[TheWitness]

Ouch. Okay, maybe we issue a warning then? I was able to cleanup the issue (In thold), and I plan on disabling legacy notifications.

strtolower($mail) == strtolower($other_email)

[netniV]

Taken from https://stackoverflow.com/a/9808332/697159

From RFC 5321, section 2.3.11:

The standard mailbox naming convention is defined to be
"local-part@domain"; contemporary usage permits a much broader set of
applications than simple "user names". Consequently, and due to a
long history of problems when intermediate hosts have attempted to
optimize transport by modifying them, the local-part MUST be
interpreted and assigned semantics only by the host specified in the
domain part of the address.

So yes, the part before the "@" could be case-sensitive, since it is entirely under the control of the host system. In practice though, no widely used mail systems distinguish different addresses based on case.

The part after the @ sign however is the domain and according to RFC 1035, section 3.1,

"Name servers and resolvers must compare [domains] in a case-insensitive manner"

In short, you are safe to treat email addresses as case-insensitive.

[netniV]

Maybe we ignore the RFC on that one since most systems do these days. But we need to make sure it's documented.

[TheWitness]

I think we should. It's a minor thing. Looking at the code parse_email_details(), could de-duplicate there, just have to be careful. Maybe make the strlower($email) the index of the array.

[TheWitness]

How does this look?

diff --git a/lib/functions.php b/lib/functions.php
index 08b383488..447020ccd 100644
--- a/lib/functions.php
+++ b/lib/functions.php
@@ -4728,7 +4728,7 @@ function parse_email_details($emails, $max_records = 0, $details = array()) {

                                foreach($emails as $email) {
                                        $email_array = split_emaildetail($email);
-                                       $details[] = $email_array;
+                                       $details[$email_array['email']] = $email_array;
                                }
                        } else {
                                $has_name  = array_key_exists('name', $check_email);
@@ -4742,13 +4742,14 @@ function parse_email_details($emails, $max_records = 0, $details = array()) {
                                        $email = array_key_exists(0, $check_email) ? $check_email[0] : '';
                                }

-                               $details[] = array('name' => trim($name), 'email' => trim($email));
+                               $details[trim(strtolower($email))] = array('name' => trim($name), 'email' => trim(strtolower($email)));
                        }
                }
        }

        if ($max_records == 1) {
-               $results = count($details) ? $details[0] : array();
+               $index = array_keys($details)[0];
+               $results = count($details) ? $details[$index] : array();
        } elseif ($max_records != 0 && $max_records < count($details)) {
                $results = array();
                foreach ($details as $d) {
@@ -4787,7 +4788,7 @@ function split_emaildetail($email) {
                $rname = $email[1];
        }

-       return array('name' => $rname, 'email' => $rmail);
+       return array('name' => $rname, 'email' => strtolower($rmail));
 }

 function create_emailtext($e) {

[netniV]

I would personally just use reset($details) to get the first item. Saves messing around getting the keys to get the first element.

[netniV]

Also, I would only set the element once, but report duplicates if there is a difference in the values? For example, if you have the same email but different names....

[TheWitness]

Interesting. Let me think about it.

[TheWitness]

Okay, used reset, but not detecting the duplicate name for now. We can put that into the backlog.