From 97c8ea811b30fec50ca825cb7d2748ade0547c0c Mon Sep 17 00:00:00 2001
From: Charles-Henri Bruyand <charles-henri.bruyand@open-xchange.com>
Date: Wed, 31 Oct 2018 10:13:34 +0100
Subject: [PATCH] auth: api - wrap hostname check in a single function

---
 pdns/dnsrecords.cc                 | 30 +++++++++++++++++++++++++++++-
 pdns/dnsrecords.hh                 |  1 +
 pdns/pdnsutil.cc                   | 30 +++++-------------------------
 pdns/ws-auth.cc                    | 26 ++++----------------------
 regression-tests.api/test_Zones.py |  2 +-
 5 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/pdns/dnsrecords.cc b/pdns/dnsrecords.cc
index 0eaeed74b..222fa01d9 100644
--- a/pdns/dnsrecords.cc
+++ b/pdns/dnsrecords.cc
@@ -587,7 +587,6 @@ DNSRecord makeOpt(const uint16_t udpsize, const uint16_t extRCode, const uint16_
   return dr;
 }
 
-
 void reportBasicTypes()
 {
   ARecordContent::report();
@@ -665,6 +664,35 @@ ComboAddress getAddr(const DNSRecord& dr, uint16_t defport)
     return getRR<AAAARecordContent>(dr)->getCA(defport);
 }
 
+/**
+ * Check if the DNSNames that should be hostnames, are hostnames
+ */
+void checkHostnameCorrectness(const DNSResourceRecord& rr)
+{
+  if (rr.qtype.getCode() == QType::NS || rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) {
+    DNSName toCheck;
+    if (rr.qtype.getCode() == QType::SRV) {
+      vector<string> parts;
+      stringtok(parts, rr.getZoneRepresentation());
+      if (parts.size() == 4) toCheck = DNSName(parts[3]);
+    } else if (rr.qtype.getCode() == QType::MX) {
+      vector<string> parts;
+      stringtok(parts, rr.getZoneRepresentation());
+      if (parts.size() == 2) toCheck = DNSName(parts[1]);
+    } else {
+      toCheck = DNSName(rr.content);
+    }
+
+    if (toCheck.empty()) {
+      throw std::runtime_error("unable to extract hostname from content");
+    }
+    else if ((rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) && toCheck == g_rootdnsname) {
+      // allow null MX/SRV
+    } else if(!toCheck.isHostname()) {
+      throw std::runtime_error(boost::str(boost::format("non-hostname content %s") % toCheck.toString()));
+    }
+  }
+}
 
 #if 0
 static struct Reporter
diff --git a/pdns/dnsrecords.hh b/pdns/dnsrecords.hh
index 52cb01975..c7397b9b2 100644
--- a/pdns/dnsrecords.hh
+++ b/pdns/dnsrecords.hh
@@ -805,4 +805,5 @@ void reportBasicTypes();
 void reportOtherTypes();
 void reportAllTypes();
 ComboAddress getAddr(const DNSRecord& dr, uint16_t defport=0);
+void checkHostnameCorrectness(const DNSResourceRecord& rr);
 #endif 
diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index 7b237e915..f9226d276 100644
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -428,35 +428,15 @@ int checkZone(DNSSECKeeper &dk, UeberBackend &B, const DNSName& zone, const vect
         checkOcclusion.insert({rr.qname, rr.qtype});
       }
     }
-
     if((rr.qtype.getCode() == QType::A || rr.qtype.getCode() == QType::AAAA) && !rr.qname.isWildcard() && !rr.qname.isHostname())
       cout<<"[Info] "<<rr.qname.toString()<<" record for '"<<rr.qtype.getName()<<"' is not a valid hostname."<<endl;
 
     // Check if the DNSNames that should be hostnames, are hostnames
-    if (rr.qtype.getCode() == QType::NS || rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) {
-      DNSName toCheck;
-      if (rr.qtype.getCode() == QType::SRV) {
-        vector<string> parts;
-        stringtok(parts, rr.getZoneRepresentation());
-        if (parts.size() == 4) toCheck = DNSName(parts[3]);
-      } else if (rr.qtype.getCode() == QType::MX) {
-        vector<string> parts;
-        stringtok(parts, rr.getZoneRepresentation());
-        if (parts.size() == 2) toCheck = DNSName(parts[1]);
-      } else {
-        toCheck = DNSName(rr.content);
-      }
-
-      if (toCheck.empty()) {
-        cout<<"[Warning] "<<rr.qtype.getName()<<" record in zone '"<<zone<<"': unable to extract hostname from content."<<endl;
-        numwarnings++;
-      }
-      else if ((rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) && toCheck == g_rootdnsname) {
-        // allow null MX/SRV
-      } else if(!toCheck.isHostname()) {
-        cout<<"[Warning] "<<rr.qtype.getName()<<" record in zone '"<<zone<<"' has non-hostname content '"<<toCheck.toString()<<"'."<<endl;
-        numwarnings++;
-      }
+    try {
+      checkHostnameCorrectness(rr);
+    } catch (const std::exception& e) {
+      cout << "[Warning] " << rr.qtype.getName() << " record in zone '" << zone << ": " << e.what() << endl;
+      numwarnings++;
     }
 
     if (rr.qtype.getCode() == QType::CNAME) {
diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 6d16b591a..5d6e387fd 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -1725,28 +1725,10 @@ static void patchZone(HttpRequest* req, HttpResponse* resp) {
             }
 
             // Check if the DNSNames that should be hostnames, are hostnames
-            if (rr.qtype.getCode() == QType::NS || rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) {
-              DNSName toCheck;
-              if (rr.qtype.getCode() == QType::SRV) {
-                vector<string> parts;
-                stringtok(parts, rr.getZoneRepresentation());
-                if (parts.size() == 4) toCheck = DNSName(parts[3]);
-              } else if (rr.qtype.getCode() == QType::MX) {
-                vector<string> parts;
-                stringtok(parts, rr.getZoneRepresentation());
-                if (parts.size() == 2) toCheck = DNSName(parts[1]);
-              } else {
-                toCheck = DNSName(rr.content);
-              }
-
-              if (toCheck.empty()) {
-                throw ApiException("RRset "+qname.toString()+" IN "+qtype.getName() + " unable to extract hostname from content.");
-              }
-              else if ((rr.qtype.getCode() == QType::MX || rr.qtype.getCode() == QType::SRV) && toCheck == g_rootdnsname) {
-                // allow null MX/SRV
-              } else if(!toCheck.isHostname()) {
-                throw ApiException("RRset "+qname.toString()+" IN "+qtype.getName() + " record has non-hostname content '" + toCheck.toString() + "'.");
-              }
+            try {
+              checkHostnameCorrectness(rr);
+            } catch (const std::exception& e) {
+              throw ApiException("RRset "+qname.toString()+" IN "+qtype.getName() + " " + e.what());
             }
           }
           checkDuplicateRecords(new_records);
diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py
index 6081cc52f..eb7081888 100644
--- a/regression-tests.api/test_Zones.py
+++ b/regression-tests.api/test_Zones.py
@@ -1006,7 +1006,7 @@ fred   IN  A      192.168.0.4
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
         self.assertEquals(r.status_code, 422)
-        self.assertIn('record has non-hostname content', r.json()['error'])
+        self.assertIn('non-hostname content', r.json()['error'])
         data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         self.assertIsNone(get_rrset(data, name, 'MX'))
 
-- 
2.40.0