From: bert hubert <bert.hubert@powerdns.com>
Date: Wed, 1 Nov 2017 17:40:01 +0000 (+0100)
Subject: fix issue where we would submit nameserver performance stats for an empty DNSName... 
X-Git-Tag: auth-4.1.0-rc2~5^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a2d65f83bd3ce5171790835f85d6408c03f8f4cf;p=pdns

fix issue where we would submit nameserver performance stats for an empty DNSName for authoritative zones, which would trip up dump-nsstats. Fixed it in depth.
Also added some error messages in case dump-nsspeeds ever throws an exception again.
---

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 2f47b60be..7daaa998a 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -215,7 +215,16 @@ string doDumpNSSpeeds(T begin, T end)
   try {
     total = broadcastAccFunction<uint64_t>(boost::bind(pleaseDumpNSSpeeds, fd));
   }
-  catch(...){}
+  catch(std::exception& e)
+  {
+    close(fd);
+    return "error dumping NS speeds: "+string(e.what())+"\n";
+  }
+  catch(PDNSException& e)
+  {
+    close(fd);
+    return "error dumping NS speeds: "+e.reason+"\n";
+  }
 
   close(fd);
   return "dumped "+std::to_string(total)+" records\n";
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 40e273b6d..504aa61ce 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -368,7 +368,9 @@ uint64_t SyncRes::doDumpNSSpeeds(int fd)
   for(const auto& i : t_sstorage.nsSpeeds)
   {
     count++;
-    fprintf(fp, "%s -> ", i.first.toString().c_str());
+
+    // an <empty> can appear hear in case of authoritative (hosted) zones
+    fprintf(fp, "%s -> ", i.first.toLogString().c_str());
     for(const auto& j : i.second.d_collection)
     {
       // typedef vector<pair<ComboAddress, DecayingEwma> > collection_t;
@@ -685,7 +687,6 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,
 
   if(ret.size() > 1) {
     map<ComboAddress, double> speeds;
-
     auto& collection = t_sstorage.nsSpeeds[qname].d_collection;
     for(const auto& val: ret) {
       double speed;
@@ -1202,8 +1203,10 @@ inline vector<DNSName> SyncRes::shuffleInSpeedOrder(NsSet &tnameservers, const s
 {
   vector<DNSName> rnameservers;
   rnameservers.reserve(tnameservers.size());
-  for(const auto& tns:tnameservers) {
+  for(const auto& tns: tnameservers) {
     rnameservers.push_back(tns.first);
+    if(tns.first.empty()) // this was an authoritative OOB zone, don't pollute the nsSpeeds with that
+      return rnameservers;
   }
   map<DNSName, double> speeds;