auth: Make sure that we use strict weak records ordering in the API

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index c589773400cb62b97d960ee474ccf834245c2dd4..03d45bc2f151263a21f0ee0624aa45688cd2925a 100644 (file)
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -374,6 +374,9 @@ static void fillZone(const DNSName& zonename, HttpResponse* resp, bool doRRSets)
         records.push_back(rr);
       }
       sort(records.begin(), records.end(), [](const DNSResourceRecord& a, const DNSResourceRecord& b) {
+              /* if you ever want to update this comparison function,
+                 please be aware that you will also need to update the conditions in the code merging
+                 the records and comments below */
               if (a.qname == b.qname) {
                   return b.qtype < a.qtype;
               }
@@ -389,6 +392,9 @@ static void fillZone(const DNSName& zonename, HttpResponse* resp, bool doRRSets)
         comments.push_back(comment);
       }
       sort(comments.begin(), comments.end(), [](const Comment& a, const Comment& b) {
+              /* if you ever want to update this comparison function,
+                 please be aware that you will also need to update the conditions in the code merging
+                 the records and comments below */
               if (a.qname == b.qname) {
                   return b.qtype < a.qtype;
               }
@@ -1224,9 +1230,8 @@ static void gatherRecordsFromZone(const std::string& zonestring, vector<DNSResou
 static void checkDuplicateRecords(vector<DNSResourceRecord>& records) {
   sort(records.begin(), records.end(),
     [](const DNSResourceRecord& rec_a, const DNSResourceRecord& rec_b) -> bool {
-      return rec_a.qname.toString() > rec_b.qname.toString() || \
-        rec_a.qtype.getCode() > rec_b.qtype.getCode() || \
-        rec_a.content < rec_b.content;
+      /* we need _strict_ weak ordering */
+      return std::tie(rec_a.qname, rec_a.qtype, rec_a.content) < std::tie(rec_b.qname, rec_b.qtype, rec_b.content);
     }
   );
   DNSResourceRecord previous;

diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py
index 12c64c8fea2ac11fcd0a58c314ceee9eccfba4c1..4eb402ee278dc1bfeb84a5bbcb09f63e1203df2e 100644 (file)
--- a/regression-tests.api/test_Zones.py
+++ b/regression-tests.api/test_Zones.py
@@ -217,18 +217,57 @@ class AuthZones(ApiTestCase, AuthZonesHelperMixin):
 
     def test_create_zone_with_comments(self):
         name = unique_zone_name()
-        rrset = {
-            "name": name,
-            "type": "soa",  # test uppercasing of type, too.
-            "comments": [{
-                "account": "test1",
-                "content": "blah blah",
-                "modified_at": 11112,
-            }],
-        }
-        name, payload, data = self.create_zone(name=name, rrsets=[rrset])
+        rrsets = [
+              {
+                  "name": name,
+                  "type": "soa",  # test uppercasing of type, too.
+                  "comments": [{
+                      "account": "test1",
+                      "content": "blah blah",
+                      "modified_at": 11112,
+                  }],
+              },
+              {
+                  "name": name,
+                  "type": "AAAA",
+                  "ttl": 3600,
+                  "records": [{
+                      "content": "2001:DB8::1",
+                      "disabled": False,
+                  }],
+                  "comments": [{
+                      "account": "test AAAA",
+                      "content": "blah blah AAAA",
+                      "modified_at": 11112,
+                  }],
+              },
+              {
+                  "name": name,
+                  "type": "TXT",
+                  "ttl": 3600,
+                  "records": [{
+                      "content": "\"test TXT\"",
+                      "disabled": False,
+                  }],
+              },
+              {
+                  "name": name,
+                  "type": "A",
+                  "ttl": 3600,
+                  "records": [{
+                      "content": "192.0.2.1",
+                      "disabled": False,
+                  }],
+              },
+          ]
+        name, payload, data = self.create_zone(name=name, rrsets=rrsets)
+        # NS records have been created
+        self.assertEquals(len(data['rrsets']), len(rrsets) + 1)
         # check our comment has appeared
-        self.assertEquals(get_rrset(data, name, 'SOA')['comments'], rrset['comments'])
+        self.assertEquals(get_rrset(data, name, 'SOA')['comments'], rrsets[0]['comments'])
+        self.assertEquals(get_rrset(data, name, 'A')['comments'], [])
+        self.assertEquals(get_rrset(data, name, 'TXT')['comments'], [])
+        self.assertEquals(get_rrset(data, name, 'AAAA')['comments'], rrsets[1]['comments'])
 
     def test_create_zone_uncanonical_nameservers(self):
         name = unique_zone_name()
@@ -1510,15 +1549,15 @@ fred   IN  A      192.168.0.4
         print(r.json())
         self.assertEquals(r.json(), [
             {u'object_type': u'zone', u'name': name, u'zone_id': name},
-            {u'content': u'a.misconfigured.powerdns.server. hostmaster.'+name+' 22 10800 3600 604800 3600',
-             u'zone_id': name, u'zone': name, u'object_type': u'record', u'disabled': False,
-             u'ttl': 3600, u'type': u'SOA', u'name': name},
             {u'content': u'ns1.example.com.',
              u'zone_id': name, u'zone': name, u'object_type': u'record', u'disabled': False,
              u'ttl': 3600, u'type': u'NS', u'name': name},
             {u'content': u'ns2.example.com.',
              u'zone_id': name, u'zone': name, u'object_type': u'record', u'disabled': False,
              u'ttl': 3600, u'type': u'NS', u'name': name},
+            {u'content': u'a.misconfigured.powerdns.server. hostmaster.'+name+' 22 10800 3600 604800 3600',
+             u'zone_id': name, u'zone': name, u'object_type': u'record', u'disabled': False,
+             u'ttl': 3600, u'type': u'SOA', u'name': name},
         ])
 
     def test_search_rr_substring(self):