Regression fix: Handle dstdomain duplicates and overlapping names better

Since 3.2 changes to dstdomain overlap detection teh case of duplicate
wildcards has become a fatal error needlessly.

This silently ignores all exact duplicates, even if they are wildcards.

Also, adjust the message display to always display the longer of the
domains first. Since we are dealing with sub-domains it is the most
reliable indicator of which should be removed to safely fix the detected
issue.

diff --git a/src/acl/DomainData.cc b/src/acl/DomainData.cc
index b435e5a944fbf8c4ae9e384ed14ef0ab66666b90..c58156f1dd26495cc91eb771cdbfb53d6be8530b 100644 (file)
--- a/src/acl/DomainData.cc
+++ b/src/acl/DomainData.cc
@@ -98,16 +98,24 @@ aclDomainCompare(T const &a, T const &b)
         if (ret == 0) {
             // When a.example.com comes after .example.com in an ACL
             // sub-domain is ignored. That is okay. Just important
-            debugs(28, DBG_IMPORTANT, "WARNING: '" << d3 << "' is a subdomain of '" << d4 << "'");
-            debugs(28, DBG_IMPORTANT, "WARNING: because of this '" << d3 << "' is ignored to keep splay tree searching predictable");
-            debugs(28, DBG_IMPORTANT, "WARNING: You should remove '" << (*d3=='.'?d4:d3) << "' from the ACL named '" << AclMatchedName << "'");
+            bool d3big = (strlen(d3) > strlen(d4)); // Always suggest removing the longer one.
+            debugs(28, DBG_IMPORTANT, "WARNING: '" << (d3big?d3:d4) << "' is a subdomain of '" << (d3big?d4:d3) << "'");
+            debugs(28, DBG_IMPORTANT, "WARNING: You should remove '" << (d3big?d3:d4) << "' from the ACL named '" << AclMatchedName << "'");
+            debugs(28, 2, HERE << "Ignore '" << d3 << "' to keep splay tree searching predictable");
         }
     } else if (ret == 0) {
+        // It may be an exact duplicate. No problem. Just drop.
+        if (strcmp(d1,d2)==0) {
+            debugs(28, 2, "WARNING: '" << d2 << "' is duplicated in the list.");
+            debugs(28, 2, "WARNING: You should remove one '" << d2 << "' from the ACL named '" << AclMatchedName << "'");
+            return ret;
+        }
         // When a.example.com comes before .example.com in an ACL
         // discarding the wildcard is critically bad.
-        debugs(28, DBG_CRITICAL, "ERROR: '" << d1 << "' is a subdomain of '" << d2 << "'");
-        debugs(28, DBG_CRITICAL, "ERROR: because of this '" << d2 << "' is ignored to keep splay tree searching predictable");
-        debugs(28, DBG_CRITICAL, "ERROR: You should remove '" << (*d1=='.'?d2:d1) << "' from the ACL named '" << AclMatchedName << "'");
+        // or Maybe even both are wildcards. Things are very weird in those cases.
+        bool d1big = (strlen(d1) > strlen(d2)); // Always suggest removing the longer one.
+        debugs(28, DBG_CRITICAL, "ERROR: '" << (d1big?d1:d2) << "' is a subdomain of '" << (d1big?d2:d1) << "'");
+        debugs(28, DBG_CRITICAL, "ERROR: You need to remove '" << (d1big?d1:d2) << "' from the ACL named '" << AclMatchedName << "'");
         self_destruct();
     }