From: Christian Hofstaedtler <christian.hofstaedtler@deduktiva.com>
Date: Fri, 15 Jul 2016 22:36:15 +0000 (+0200)
Subject: API: prevent duplicate records in single RRset
X-Git-Tag: auth-4.1.0-rc1~17^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3675a8ad4093a49319de5f4bc7ff01277917825;p=thirdparty%2Fpdns.git

API: prevent duplicate records in single RRset

If a zone already had duplicates, we do nothing to them
(for now).
---

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 2140e5edf5..0c377d3eee 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -991,6 +991,26 @@ static void gatherRecordsFromZone(const std::string& zonestring, vector<DNSResou
   }
 }
 
+/** Throws ApiException if records with duplicate name/type/content are present.
+ *  NOTE: sorts records in-place.
+ */
+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;
+    }
+  );
+  DNSResourceRecord previous;
+  for(const auto& rec : records) {
+    if (previous.qtype == rec.qtype && previous.qname == rec.qname && previous.content == rec.content) {
+      throw ApiException("Duplicate record in RRset " + rec.qname.toString() + " IN " + rec.qtype.getName() + " with content \"" + rec.content + "\"");
+    }
+    previous = rec;
+  }
+}
+
 static void apiServerZones(HttpRequest* req, HttpResponse* resp) {
   UeberBackend B;
   DNSSECKeeper dk(&B);
@@ -999,6 +1019,7 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) {
     auto document = req->json();
     DNSName zonename = apiNameToDNSName(stringFromJson(document, "name"));
     apiCheckNameAllowedCharacters(zonename.toString());
+    zonename.makeUsLowerCase();
 
     bool exists = B.getDomainInfo(zonename, di);
     if(exists)
@@ -1054,6 +1075,7 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) {
     }
 
     for(auto& rr : new_records) {
+      rr.qname.makeUsLowerCase();
       if (!rr.qname.isPartOf(zonename) && rr.qname != zonename)
         throw ApiException("RRset "+rr.qname.toString()+" IN "+rr.qtype.getName()+": Name is out of zone");
       apiCheckQNameAllowedCharacters(rr.qname.toString());
@@ -1112,6 +1134,8 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) {
       }
     }
 
+    checkDuplicateRecords(new_records);
+
     // no going back after this
     if(!B.createDomain(zonename))
       throw ApiException("Creating domain '"+zonename.toString()+"' failed");
@@ -1419,6 +1443,7 @@ static void patchZone(HttpRequest* req, HttpResponse* resp) {
               rr.content = makeBackendRecordContent(rr.qtype, rr.content);
             }
           }
+          checkDuplicateRecords(new_records);
         }
 
         if (replace_comments) {
diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py
index 24d5117972..95581072f4 100644
--- a/regression-tests.api/test_Zones.py
+++ b/regression-tests.api/test_Zones.py
@@ -566,9 +566,10 @@ powerdns.com.           86400   IN      SOA     powerdnssec1.ds9a.nl. ahu.ds9a.n
 
         eq_zone_rrsets(data['rrsets'], expected)
 
-        # noDot check
+        # check content in DB is stored WITHOUT trailing dot.
         dbrecs = get_db_records(payload['name'], 'NS')
-        self.assertEqual(dbrecs[0]['content'], 'powerdnssec2.ds9a.nl')
+        dbrec = next((dbrec for dbrec in dbrecs if dbrec['content'].startswith('powerdnssec1')))
+        self.assertEqual(dbrec['content'], 'powerdnssec1.ds9a.nl')
 
     def test_import_zone_bind(self):
         payload = {}
@@ -788,6 +789,29 @@ fred   IN  A      192.168.0.4
         self.assertEquals(get_rrset(data, name, 'NS')['records'], rrset1['records'])
         self.assertEquals(get_rrset(data, name, 'MX')['records'], rrset2['records'])
 
+    def test_zone_rr_update_duplicate_record(self):
+        name, payload, zone = self.create_zone()
+        rrset = {
+            'changetype': 'replace',
+            'name': name,
+            'type': 'NS',
+            'ttl': 3600,
+            'records': [
+                {"content": "ns9999.example.com.", "disabled": False},
+                {"content": "ns9996.example.com.", "disabled": False},
+                {"content": "ns9987.example.com.", "disabled": False},
+                {"content": "ns9988.example.com.", "disabled": False},
+                {"content": "ns9999.example.com.", "disabled": False},
+            ]
+        }
+        payload = {'rrsets': [rrset]}
+        r = self.session.patch(
+            self.url("/api/v1/servers/localhost/zones/" + name),
+            data=json.dumps(payload),
+            headers={'content-type': 'application/json'})
+        self.assertEquals(r.status_code, 422)
+        self.assertIn('Duplicate record in RRset', r.json()['error'])
+
     def test_zone_rr_delete(self):
         name, payload, zone = self.create_zone()
         # do a delete of all NS records (these are created with the zone)