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

(cherry picked from commit f2d6dcc017a05fb5e4f9ba1349c568ff43ce9bb9)

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 663607ee9620cace019836a60b20c625709a4f7c..29f0b01e3daa07952c5294e71beb1ec78a796917 100644 (file)
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -369,6 +369,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;
               }
@@ -384,6 +387,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;
               }
@@ -1206,9 +1212,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 b27b2e808b5114ed06849da65800aaeb538c9df9..db971e0a1661261eaebfe9302aa6243ad5dceb10 100644 (file)
--- a/regression-tests.api/test_Zones.py
+++ b/regression-tests.api/test_Zones.py
@@ -172,18 +172,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()
@@ -1430,15 +1469,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):