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 dc86c44e336907ac9607da5d3d07bec21749a45a..3d5aa33eed5f288134d4095bc0114e7550b987ae 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();
     }