From: bert hubert <bert.hubert@powerdns.com>
Date: Sun, 27 Sep 2015 11:54:47 +0000 (+0200)
Subject: with this commit, we remove the implicit DNSName string constructors, which hid many... 
X-Git-Tag: dnsdist-1.0.0-alpha1~248^2~28^2~27^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8171ab83930489ec60298bd82cefe864895fc546;p=thirdparty%2Fpdns.git

with this commit, we remove the implicit DNSName string constructors, which hid many many issues, including passing all DNS query packets as DNS domain names and converting them back again (WOW). To slightly compensate, we added an ostream operator<< to make printing of DNSNames easier.
This commit will start a world of pain and it isn't even done yet. Specifically forwarding to another nameserver is broken now (was broken already, we tried to push a :port number into a DNSName and parse it again). The webserver API was lovingly DNSNamed but will also likely have changes in case sensitivity, . termination and other things.
---

diff --git a/pdns/dns.cc b/pdns/dns.cc
index 2f66c5b10b..2e0639a94d 100644
--- a/pdns/dns.cc
+++ b/pdns/dns.cc
@@ -42,24 +42,6 @@ std::string RCode::to_s(unsigned short rcode) {
   return RCode::rcodes_s[rcode];
 }
 
-static void appendEscapedLabel(string& ret, const char* begin, unsigned char labellen)
-{
-  unsigned char n = 0;
-  for(n = 0 ; n < labellen; ++n)
-    if(begin[n] == '.' || begin[n] == '\\' || begin[n] == ' ')
-      break;
-  
-  if( n == labellen) {
-    ret.append(begin, labellen);
-    return;
-  }
-  string label(begin, labellen);
-  boost::replace_all(label, "\\",  "\\\\");
-  boost::replace_all(label, ".",  "\\.");
-  boost::replace_all(label, " ",  "\\032");
-  ret.append(label);
-}
-
 class BoundsCheckingPointer
 {
 public:
@@ -151,36 +133,6 @@ uint32_t hashQuestion(const char* packet, uint16_t len, uint32_t init)
   return ret;
 }
 
-string questionExpand(const char* packet, uint16_t len, uint16_t& type)
-{
-  type=0;
-  string ret;
-  if(len < 12) 
-    throw runtime_error("Error parsing question in incoming packet: packet too short");
-    
-  const unsigned char* end = (const unsigned char*)packet+len;
-  const unsigned char* pos = (const unsigned char*)packet+12;
-  unsigned char labellen;
-  
-  if(!*pos)
-    ret.assign(1, '.');
-  
-  while((labellen=*pos++) && pos < end) { // "scan and copy"
-    if(pos + labellen > end)
-      throw runtime_error("Error parsing question in incoming packet: label extends beyond packet");
-    
-    appendEscapedLabel(ret, (const char*) pos, labellen);
-    
-    ret.append(1, '.');
-    pos += labellen;
-  }
-
-  if(pos + labellen + 2 <= end)  
-    type=(*pos)*256 + *(pos+1);
-  // cerr << "returning: '"<<ret<<"'"<<endl;
-  return ret;
-}
-
 void fillSOAData(const string &content, SOAData &data)
 {
   // content consists of fields separated by spaces:
@@ -195,10 +147,10 @@ void fillSOAData(const string &content, SOAData &data)
   //  cout<<"'"<<content<<"'"<<endl;
 
   if(pleft)
-    data.nameserver=parts[0];
+    data.nameserver=DNSName(parts[0]);
 
   if(pleft>1) 
-    data.hostmaster=attodot(parts[1]); // ahu@ds9a.nl -> ahu.ds9a.nl, piet.puk@ds9a.nl -> piet\.puk.ds9a.nl
+    data.hostmaster=DNSName(attodot(parts[1])); // ahu@ds9a.nl -> ahu.ds9a.nl, piet.puk@ds9a.nl -> piet\.puk.ds9a.nl
 
   data.serial = pleft > 2 ? pdns_strtoui(parts[2].c_str(), NULL, 10) : 0;
   if (data.serial == UINT_MAX && errno == ERANGE) throw PDNSException("serial number too large in '"+parts[2]+"'");
diff --git a/pdns/dns.hh b/pdns/dns.hh
index 0821d05e0a..07c0cd98b2 100644
--- a/pdns/dns.hh
+++ b/pdns/dns.hh
@@ -220,7 +220,7 @@ struct dnsheader {
 
 #define L theL()
 extern time_t s_starttime;
-std::string questionExpand(const char* packet, uint16_t len, uint16_t& type);
+
 uint32_t hashQuestion(const char* packet, uint16_t len, uint32_t init);
 bool dnspacketLessThan(const std::string& a, const std::string& b);
 
diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index 68d51e478d..cdc6841559 100644
--- a/pdns/dnsname.cc
+++ b/pdns/dnsname.cc
@@ -15,8 +15,12 @@
    a primitive is nextLabel()
 */
 
-/* FIXME400: @nlyan suggests that we should only have a string constructor, and make sure
- * char* does not implicitly map to it, to avoid issues with embedded NULLs */
+std::ostream & operator<<(std::ostream &os, const DNSName& d)
+{
+  return os <<d.toString();
+}
+
+
 DNSName::DNSName(const char* p)
 {
   d_empty=false;
diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
index 8be308a242..7191710ede 100644
--- a/pdns/dnsname.hh
+++ b/pdns/dnsname.hh
@@ -27,9 +27,9 @@ class DNSName
 {
 public:
   DNSName() : d_empty(true) {}                 //!< Don't constructs the root name
-  DNSName(const char* p);      //!< Constructs from a human formatted, escaped presentation
-  DNSName(const std::string& str) : DNSName(str.c_str()) {}   //!< Constructs from a human formatted, escaped presentation
-  DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0); //!< Construct from a DNS Packet, taking the first question
+  explicit DNSName(const char* p);      //!< Constructs from a human formatted, escaped presentation
+  explicit DNSName(const std::string& str) : DNSName(str.c_str()) {}   //!< Constructs from a human formatted, escaped presentation
+  DNSName(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0); //!< Construct from a DNS Packet, taking the first question if offset=12
   
   bool isPartOf(const DNSName& rhs) const;   //!< Are we part of the rhs name?
   bool operator==(const DNSName& rhs) const; //!< DNS-native comparison (case insensitive)
@@ -172,3 +172,5 @@ struct SuffixMatchNode
   }
 
 };
+
+std::ostream & operator<<(std::ostream &os, const DNSName& d);
diff --git a/pdns/dnsparser.cc b/pdns/dnsparser.cc
index ade6ba93bb..fee0bc82a1 100644
--- a/pdns/dnsparser.cc
+++ b/pdns/dnsparser.cc
@@ -255,7 +255,7 @@ void MOADNSParser::init(const char *packet, unsigned int len)
       
       unsigned int recordStartPos=pr.d_pos;
 
-      string label=pr.getName();
+      DNSName label=pr.getName();
       
       pr.getDnsrecordheader(ah);
       dr.d_ttl=ah.d_ttl;
@@ -389,23 +389,23 @@ uint8_t PacketReader::get8BitInt()
   return d_content.at(d_pos++);
 }
 
-string PacketReader::getName()
+DNSName PacketReader::getName()
 {
   unsigned int consumed;
   vector<uint8_t> content(d_content);
   content.insert(content.begin(), sizeof(dnsheader), 0);
 
   try {
-    string ret = DNSName((const char*) content.data(), content.size(), d_pos + sizeof(dnsheader), true /* uncompress */, 0 /* qtype */, 0 /* qclass */, &consumed).toString();
+    DNSName dn((const char*) content.data(), content.size(), d_pos + sizeof(dnsheader), true /* uncompress */, 0 /* qtype */, 0 /* qclass */, &consumed);
     
     d_pos+=consumed;
-    return ret;
+    return dn;
   }
   catch(...)
     {
       throw std::out_of_range("dnsname issue");
     }
-
+  return DNSName(); // if this ever happens..
 }
 
 static string txtEscape(const string &name)
diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index f605b40552..1a67066bf5 100644
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -140,7 +140,7 @@ public:
   void copyRecord(vector<unsigned char>& dest, uint16_t len);
   void copyRecord(unsigned char* dest, uint16_t len);
 
-  string getName();
+  DNSName getName();
   string getText(bool multi);
 
   uint16_t d_pos;
@@ -169,7 +169,7 @@ public:
   virtual string serialize(const DNSName& qname, bool canonic=false, bool lowerCase=false) // it would rock if this were const, but it is too hard
   {
     vector<uint8_t> packet;
-    string empty;
+    DNSName empty;
     DNSPacketWriter pw(packet, empty, 1);
     if(canonic)
       pw.setCanonic(true);
diff --git a/pdns/dnsrecords.cc b/pdns/dnsrecords.cc
index 14a7d67fe3..479964fc15 100644
--- a/pdns/dnsrecords.cc
+++ b/pdns/dnsrecords.cc
@@ -425,7 +425,7 @@ TKEYRecordContent::TKEYRecordContent() : DNSRecordContent(QType::TKEY) { d_other
 
 uint16_t DNSKEYRecordContent::getTag()
 {
-  string data=this->serialize("");
+  string data=this->serialize(DNSName()); 
   const unsigned char* key=(const unsigned char*)data.c_str();
   unsigned int keysize=data.length();
 
diff --git a/pdns/dnswriter.cc b/pdns/dnswriter.cc
index 53803f5998..81f2f9bc71 100644
--- a/pdns/dnswriter.cc
+++ b/pdns/dnswriter.cc
@@ -99,7 +99,7 @@ void DNSPacketWriter::addOpt(int udpsize, int extRCode, int Z, const vector<pair
 
   ttl=ntohl(ttl); // will be reversed later on
 
-  startRecord("", QType::OPT, ttl, udpsize, ADDITIONAL, false);
+  startRecord(DNSName(), QType::OPT, ttl, udpsize, ADDITIONAL, false);
   for(optvect_t::const_iterator iter = options.begin(); iter != options.end(); ++iter) {
     xfr16BitInt(iter->first);
     xfr16BitInt(iter->second.length());
diff --git a/pdns/lua-recursor.cc b/pdns/lua-recursor.cc
index a955a796dc..7365881f00 100644
--- a/pdns/lua-recursor.cc
+++ b/pdns/lua-recursor.cc
@@ -67,7 +67,7 @@ extern "C" {
 
 static int getRegisteredNameLua(lua_State *L) {
   const char *name = luaL_checkstring(L, 1);
-  string regname=getRegisteredName(name).toString();
+  string regname=getRegisteredName(DNSName(name)).toString(); // hnnggg
   lua_pushstring(L, regname.c_str());
   return 1;
 }
@@ -126,13 +126,13 @@ int getFakeAAAARecords(const std::string& qname, const std::string& prefix, vect
   return rcode;
 }
 
-int getFakePTRRecords(const std::string& qname, const std::string& prefix, vector<DNSResourceRecord>& ret)
+int getFakePTRRecords(const DNSName& qname, const std::string& prefix, vector<DNSResourceRecord>& ret)
 {
   /* qname has a reverse ordered IPv6 address, need to extract the underlying IPv4 address from it
      and turn it into an IPv4 in-addr.arpa query */
   ret.clear();
-  vector<string> parts;
-  stringtok(parts, qname, ".");
+  vector<string> parts = qname.getRawLabels();
+
   if(parts.size() < 8)
     return -1;
 
@@ -289,7 +289,7 @@ bool RecursorLua::passthrough(const string& func, const ComboAddress& remote, co
       string luaprefix = lua_tostring(d_lua, 2);
       string luaqname = lua_tostring(d_lua,1);
       lua_pop(d_lua, 2);
-      res = getFakePTRRecords(luaqname, luaprefix, ret);
+      res = getFakePTRRecords(DNSName(luaqname), luaprefix, ret);
     }
     else if(tocall == "followCNAMERecords") {
       popResourceRecordsTable(d_lua, query, ret);
diff --git a/pdns/misc.cc b/pdns/misc.cc
index 57eec1975e..340ab1b73f 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -1076,13 +1076,13 @@ bool getTSIGHashEnum(const DNSName& algoName, TSIGHashEnum& algoEnum)
 DNSName getTSIGAlgoName(TSIGHashEnum& algoEnum)
 {
   switch(algoEnum) {
-  case TSIG_MD5: return "hmac-md5.sig-alg.reg.int.";
-  case TSIG_SHA1: return "hmac-sha1.";
-  case TSIG_SHA224: return "hmac-sha224.";
-  case TSIG_SHA256: return "hmac-sha256.";
-  case TSIG_SHA384: return "hmac-sha384.";
-  case TSIG_SHA512: return "hmac-sha512.";
-  case TSIG_GSS: return "gss-tsig.";
+  case TSIG_MD5: return DNSName("hmac-md5.sig-alg.reg.int.");
+  case TSIG_SHA1: return DNSName("hmac-sha1.");
+  case TSIG_SHA224: return DNSName("hmac-sha224.");
+  case TSIG_SHA256: return DNSName("hmac-sha256.");
+  case TSIG_SHA384: return DNSName("hmac-sha384.");
+  case TSIG_SHA512: return DNSName("hmac-sha512.");
+  case TSIG_GSS: return DNSName("gss-tsig.");
   }
   throw PDNSException("getTSIGAlgoName does not understand given algorithm, please fix!");
 }
diff --git a/pdns/misc.hh b/pdns/misc.hh
index 0c6c8cd8ef..acce976440 100644
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -507,9 +507,9 @@ inline bool isCanonical(const DNSName& qname)
 inline DNSName toCanonic(const DNSName& zone, const string& qname)
 {
   if(qname.size()==1 && qname[0]=='@')
-    return zone.toString();
+    return zone;
   if(isCanonical(qname))
-    return DNSName(qname).toString();
+    return DNSName(qname);
   return DNSName(qname) += zone;
 }
 
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 0f4857cda9..23fb9af73c 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -1347,7 +1347,7 @@ static void houseKeeping(void *)
       sr.setNoCache();
       int res=-1;
       try {
-	res=sr.beginResolve(".", QType(QType::NS), 1, ret);
+	res=sr.beginResolve(DNSName(), QType(QType::NS), 1, ret);
       }
       catch(PDNSException& e)
 	{
@@ -1449,10 +1449,9 @@ void broadcastFunction(const pipefunc_t& func, bool skipSelf)
 }
 
 uint32_t g_disthashseed;
-void distributeAsyncFunction(const DNSName& question, const pipefunc_t& func)
+void distributeAsyncFunction(const string& packet, const pipefunc_t& func)
 {
-  string squestion = question.toString();
-  unsigned int hash = hashQuestion(squestion.c_str(), squestion.length(), g_disthashseed);
+  unsigned int hash = hashQuestion(packet.c_str(), packet.length(), g_disthashseed);
   unsigned int target = 1 + (hash % (g_pipes.size()-1));
 
   if(target == t_id) {
@@ -1698,7 +1697,7 @@ void handleUDPServerResponse(int fd, FDMultiplexer::funcparam_t& var)
   }
   else {
     try {
-      pident.domain=questionExpand(data, len, pident.type); // don't copy this from above - we need to do the actual read
+      pident.domain=DNSName(data, len, 12, false, &pident.type); // don't copy this from above - we need to do the actual read
     }
     catch(std::exception& e) {
       g_stats.serverParseError++; // won't be fed to lwres.cc, so we have to increment
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 55ab3cfa05..f817e4a9b0 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -726,6 +726,7 @@ void sortPublicSuffixList()
   sort(g_pubs.begin(), g_pubs.end());
 }
 
+// XXX DNSName Pain - this function should benefit from native DNSName methods
 DNSName getRegisteredName(const DNSName& dom)
 {
   auto parts=dom.getRawLabels();
@@ -746,13 +747,13 @@ DNSName getRegisteredName(const DNSName& dom)
       BOOST_REVERSE_FOREACH(const std::string& p, parts) {
 	ret+=p+".";
       }
-      return ret;
+      return DNSName(ret);
     }
 
     last=parts[parts.size()-1];
     parts.resize(parts.size()-1);
   }
-  return "??";
+  return DNSName("??");
 }
 
 static DNSName nopFilter(const DNSName& name)
diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc
index 665ca32cde..8274a5aee8 100644
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -31,8 +31,9 @@ int RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype)
     if(packet->qdcount==0)
       break;
     uint16_t t;
-    string found=questionExpand(iter->d_packet.c_str(), iter->d_packet.length(), t);
-    if(!pdns_iequals(found, name)) {  
+
+    DNSName found(iter->d_packet.c_str(), iter->d_packet.size(), 12, false, &t);
+    if(found==name) {   // this is case insensitive
       break;
     }
     if(t==qtype || qtype==0xffff) {
diff --git a/pdns/reczones.cc b/pdns/reczones.cc
index f73eb8557a..949a50541f 100644
--- a/pdns/reczones.cc
+++ b/pdns/reczones.cc
@@ -60,17 +60,18 @@ void primeHints(void)
       static char templ[40];
       strncpy(templ,"a.root-servers.net.", sizeof(templ) - 1);
       *templ=c;
-      aaaarr.qname=arr.qname=nsrr.content=templ;
+      aaaarr.qname=arr.qname=DNSName(templ);
+      nsrr.content=templ; 
       arr.content=ips[c-'a'];
       set<DNSResourceRecord> aset;
       aset.insert(arr);
-      t_RC->replace(time(0), string(templ), QType(QType::A), aset, vector<std::shared_ptr<RRSIGRecordContent>>(), true); // auth, nuke it all
+      t_RC->replace(time(0), DNSName(templ), QType(QType::A), aset, vector<std::shared_ptr<RRSIGRecordContent>>(), true); // auth, nuke it all
       if (ip6s[c-'a'] != NULL) {
         aaaarr.content=ip6s[c-'a'];
 
         set<DNSResourceRecord> aaaaset;
         aaaaset.insert(aaaarr);
-        t_RC->replace(time(0), string(templ), QType(QType::AAAA), aaaaset, vector<std::shared_ptr<RRSIGRecordContent>>(), true);
+        t_RC->replace(time(0), DNSName(templ), QType(QType::AAAA), aaaaset, vector<std::shared_ptr<RRSIGRecordContent>>(), true);
       }
       
       nsset.insert(nsrr);
@@ -96,7 +97,7 @@ void primeHints(void)
       }
     }
   }
-  t_RC->replace(time(0),".", QType(QType::NS), nsset, vector<std::shared_ptr<RRSIGRecordContent>>(), true); // and stuff in the cache (auth)
+  t_RC->replace(time(0), DNSName(), QType(QType::NS), nsset, vector<std::shared_ptr<RRSIGRecordContent>>(), true); // and stuff in the cache (auth)
 }
 
 static void makeNameToIPZone(SyncRes::domainmap_t* newMap, const DNSName& hostname, const string& ip)
@@ -367,7 +368,7 @@ SyncRes::domainmap_t* parseAuthAndForwards()
         }
       }
       
-      (*newMap)[headers.first]=ad; 
+      (*newMap)[DNSName(headers.first)]=ad; 
     }
   }
   
@@ -413,7 +414,7 @@ SyncRes::domainmap_t* parseAuthAndForwards()
         throw PDNSException("Conversion error parsing line "+lexical_cast<string>(linenum)+" of " +::arg()["forward-zones-file"]);
       }
 
-      (*newMap)[domain]=ad;
+      (*newMap)[DNSName(domain)]=ad;
     }
     L<<Logger::Warning<<"Done parsing " << newMap->size() - before<<" forwarding instructions from file '"<<::arg()["forward-zones-file"]<<"'"<<endl;
   }
@@ -443,10 +444,10 @@ SyncRes::domainmap_t* parseAuthAndForwards()
         
         for(unsigned int n=1; n < parts.size(); ++n) {
           if(searchSuffix.empty() || parts[n].find('.') != string::npos)
-              makeNameToIPZone(newMap, parts[n], parts[0]);
+	    makeNameToIPZone(newMap, DNSName(parts[n]), parts[0]);
           else {
-              DNSName canonic=toCanonic(DNSName(searchSuffix), parts[n]);
-              if(canonic != parts[n]) {
+	    DNSName canonic=toCanonic(DNSName(searchSuffix), parts[n]); /// XXXX DNSName pain
+	    if(canonic != DNSName(parts[n])) {   // XXX further DNSName pain
               makeNameToIPZone(newMap, canonic, parts[0]);
             }
           }
diff --git a/pdns/secpoll-recursor.cc b/pdns/secpoll-recursor.cc
index 73e20fee28..ce187c3155 100644
--- a/pdns/secpoll-recursor.cc
+++ b/pdns/secpoll-recursor.cc
@@ -27,14 +27,15 @@ void doSecPoll(time_t* last_secpoll)
   vector<DNSResourceRecord> ret;
 
   string version = "recursor-" +string(PACKAGEVERSION);
-  string query = version.substr(0, 63)+ ".security-status."+::arg()["security-poll-suffix"];
+  string qstring(version.substr(0, 63)+ ".security-status."+::arg()["security-poll-suffix"]);
 
-  if(*query.rbegin()!='.')
-    query+='.';
+  if(*qstring.rbegin()!='.')
+    qstring+='.';
 
-  boost::replace_all(query, "+", "_");
-  boost::replace_all(query, "~", "_");
+  boost::replace_all(qstring, "+", "_");
+  boost::replace_all(qstring, "~", "_");
 
+  DNSName query(qstring);
   int res=sr.beginResolve(query, QType(QType::TXT), 1, ret);
   if(!res && !ret.empty()) {
     string content=ret.begin()->content;
@@ -52,7 +53,7 @@ void doSecPoll(time_t* last_secpoll)
   else {
     string pkgv(PACKAGEVERSION);
     if(pkgv.find("0.0."))
-      L<<Logger::Warning<<"Could not retrieve security status update for '" +pkgv+ "' on '"+query+"', RCODE = "<< RCode::to_s(res)<<endl;
+      L<<Logger::Warning<<"Could not retrieve security status update for '" +pkgv+ "' on '"<<query<<"', RCODE = "<< RCode::to_s(res)<<endl;
     else
       L<<Logger::Warning<<"Not validating response for security status update, this a non-release version."<<endl;
 
diff --git a/pdns/speedtest.cc b/pdns/speedtest.cc
index 4c0bc9c9fe..2f0c7c788b 100644
--- a/pdns/speedtest.cc
+++ b/pdns/speedtest.cc
@@ -489,9 +489,9 @@ struct TCacheComp
 
 struct NegCacheEntry
 {
-  string d_name;
+  DNSName d_name;
   QType d_qtype;
-  string d_qname;
+  DNSName d_qname;
   uint32_t d_ttd;
 };
 
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index ad0d152156..3070925d07 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -109,8 +109,11 @@ int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qcl
   if( (qtype.getCode() == QType::AXFR))
     return -1;
 
-  if( (qtype.getCode()==QType::PTR && pdns_iequals(qname, "1.0.0.127.in-addr.arpa.")) ||
-      (qtype.getCode()==QType::A && pdns_iequals(qname, "localhost."))) {
+  static const DNSName arpa("1.0.0.127.in-addr.arpa."), localhost("localhost."), 
+    versionbind("version.bind."), idserver("id.server."), versionpdns("version.pdns.");
+
+  if( (qtype.getCode()==QType::PTR && qname==arpa) ||
+      (qtype.getCode()==QType::A && qname==localhost)) {
     ret.clear();
     DNSResourceRecord rr;
     rr.qname=qname;
@@ -126,7 +129,7 @@ int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qcl
   }
 
   if(qclass==QClass::CHAOS && qtype.getCode()==QType::TXT &&
-        (pdns_iequals(qname, "version.bind.") || pdns_iequals(qname, "id.server.") || pdns_iequals(qname, "version.pdns.") )
+        (qname==versionbind || qname==idserver || qname==versionpdns )
      ) {
     ret.clear();
     DNSResourceRecord rr;
@@ -134,7 +137,7 @@ int SyncRes::beginResolve(const DNSName &qname, const QType &qtype, uint16_t qcl
     rr.qtype=qtype;
     rr.qclass=qclass;
     rr.ttl=86400;
-    if(pdns_iequals(qname,"version.bind.")  || pdns_iequals(qname,"version.pdns."))
+    if(qname==versionbind  || qname==versionpdns)
       rr.content="\""+::arg()["version-string"]+"\"";
     else
       rr.content="\""+s_serverID+"\"";
@@ -314,19 +317,19 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, const DNSName& domain,
     ret=asyncresolve(ip, domain, type, doTCP, sendRDQuery, EDNSLevel, now, res);
 
     if(ret == 0 || ret < 0) {
-      cerr<<"Transport error or timeout (ret="<<ret<<"), no change in mode"<<endl;
+      cerr<< (ret < 0 ? "Transport error" : "Timeout")<<" for query to "<<ip.toString()<<" for '"<<domain.toString()<<"' (ret="<<ret<<"), no change in mode"<<endl;
       return ret;
     }
     else if(mode==EDNSStatus::UNKNOWN || mode==EDNSStatus::EDNSOK || mode == EDNSStatus::EDNSIGNORANT ) {
       if(res->d_rcode == RCode::FormErr || res->d_rcode == RCode::NotImp)  {
-	cerr<<"Downgrading to NOEDNS because of FORMERR or NotImp!"<<endl;
+	cerr<<"Downgrading to NOEDNS because of "<<RCode::to_s(res->d_rcode)<<" for query to "<<ip.toString()<<" for '"<<domain.toString()<<"'"<<endl;
         mode = EDNSStatus::NOEDNS;
         continue;
       }
       else if(!res->d_haveEDNS) {
         if(mode != EDNSStatus::EDNSIGNORANT) {
           mode = EDNSStatus::EDNSIGNORANT;
-	  cerr<<"We find that "<<ip.toString()<<" is an EDNS-ignorer, moving to mode 3"<<endl;
+	  cerr<<"We find that "<<ip.toString()<<" is an EDNS-ignorer for '"<<domain.toString()<<"', moving to mode 3"<<endl;
 	}
       }
       else {
@@ -512,7 +515,7 @@ void SyncRes::getBestNSFromCache(const DNSName &qname, const QType& qtype, set<D
 
           DNSResourceRecord rr=*k;
           rr.content=k->content;
-          if(!DNSName(rr.content).isPartOf(subdomain) || t_RC->get(d_now.tv_sec, rr.content, s_doIPv6 ? QType(QType::ADDR) : QType(QType::A),
+          if(!DNSName(rr.content).isPartOf(subdomain) || t_RC->get(d_now.tv_sec, DNSName(rr.content), s_doIPv6 ? QType(QType::ADDR) : QType(QType::A),
                                                             doLog() ? &aset : 0) > 5) {
             bestns.insert(rr);
             LOG(prefix<<qname.toString()<<": NS (with ip, or non-glue) in cache for '"<<subdomain.toString()<<"' -> '"<<rr.content<<"'"<<endl);
@@ -585,7 +588,9 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
       nsset.insert(DNSName()); // this gets picked up in doResolveAt, if empty it means "we are auth", otherwise it denotes a forward
     else {
       for(vector<ComboAddress>::const_iterator server=iter->second.d_servers.begin(); server != iter->second.d_servers.end(); ++server)
-        nsset.insert((iter->second.d_rdForward ? "+" : "-") + server->toStringWithPort()); // add a '+' if the rd bit should be set
+	//        nsset.insert((iter->second.d_rdForward ? "+" : "-") + server->toStringWithPort()); // add a '+' if the rd bit should be set
+      // XXX this doesn't work, nsset can't contain a port number, or a plus etc!
+	abort();
     }
 
     return authdomain;
@@ -595,7 +600,7 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
   getBestNSFromCache(subdomain, qtype, bestns, flawedNSSet, depth, beenthere);
 
   for(set<DNSResourceRecord>::const_iterator k=bestns.begin();k!=bestns.end();++k) {
-    nsset.insert(k->content);
+    nsset.insert(DNSName(k->content));
     if(k==bestns.begin())
       subdomain=k->qname;
   }
@@ -628,7 +633,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
         ret.push_back(rr);
         if(!(qtype==QType(QType::CNAME))) { // perhaps they really wanted a CNAME!
           set<GetBestNSAnswer>beenthere;
-          res=doResolve(j->content, qtype, ret, depth+1, beenthere);
+          res=doResolve(DNSName(j->content), qtype, ret, depth+1, beenthere);
         }
         else
           res=0;
@@ -640,11 +645,11 @@ bool SyncRes::doCNAMECacheCheck(const DNSName &qname, const QType &qtype, vector
   return false;
 }
 
-// accepts . terminated names, www.powerdns.com. -> com.
-static const string getLastLabel(const DNSName& qname)
+static const DNSName getLastLabel(const DNSName& qname)
 {
-  auto parts = qname.getRawLabels();
-  return parts[parts.size()-1];
+  DNSName ret(qname);
+  ret.trimToLabels(1);
+  return ret;
 }
 
 bool SyncRes::doCacheCheck(const DNSName &qname, const QType &qtype, vector<DNSResourceRecord>&ret, int depth, int &res)
@@ -840,7 +845,7 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
 		for(vector<DNSName >::const_iterator tns=rnameservers.begin();;++tns) {
       if(tns==rnameservers.end()) {
         LOG(prefix<<qname.toString()<<": Failed to resolve via any of the "<<(unsigned int)rnameservers.size()<<" offered NS at level '"<<auth.toString()<<"'"<<endl);
-        if(auth!="." && flawedNSSet) {
+        if(auth!=DNSName() && flawedNSSet) {
           LOG(prefix<<qname.toString()<<": Ageing nameservers for level '"<<auth.toString()<<"', next query might succeed"<<endl);
           if(t_RC->doAgeCache(d_now.tv_sec, auth, QType::NS, 10))
             g_stats.nsSetInvalidations++;
@@ -1150,7 +1155,7 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
         }
         else if(i->d_place==DNSResourceRecord::ANSWER && pdns_iequals(i->qname, qname) && i->qtype.getCode()==QType::CNAME && (!(qtype==QType(QType::CNAME)))) {
           ret.push_back(*i);
-          newtarget=i->content;
+          newtarget=DNSName(i->content);
         }
 	else if(d_doDNSSEC && (i->qtype==QType::RRSIG || i->qtype==QType::NSEC || i->qtype==QType::NSEC3) && i->d_place==DNSResourceRecord::ANSWER){
 	  ret.push_back(*i); // enjoy your DNSSEC
@@ -1176,7 +1181,7 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
           }
           else
             LOG(prefix<<qname.toString()<<": got upwards/level NS record '"<<i->qname.toString()<<"' -> '"<<i->content<<"', had '"<<auth.toString()<<"'"<<endl);
-          nsset.insert(i->content);
+          nsset.insert(DNSName(i->content));
         }
         else if(i->d_place==DNSResourceRecord::AUTHORITY && dottedEndsOn(qname,i->qname) && i->qtype.getCode()==QType::DS) { 
 	  LOG(prefix<<qname.toString()<<": got DS record '"<<i->qname.toString()<<"' -> '"<<i->content<<"'"<<endl);
@@ -1260,6 +1265,6 @@ int directResolve(const std::string& qname, const QType& qtype, int qclass, vect
 
   SyncRes sr(now);
 
-  int res = sr.beginResolve(qname, QType(qtype), qclass, ret);
+  int res = sr.beginResolve(DNSName(qname), QType(qtype), qclass, ret); // DNSName conversion XXX
   return res;
 }
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 8614e62973..e9a1f54165 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -627,7 +627,7 @@ ComboAddress parseIPAndPort(const std::string& input, uint16_t port);
 ComboAddress getQueryLocalAddress(int family, uint16_t port);
 typedef boost::function<void*(void)> pipefunc_t;
 void broadcastFunction(const pipefunc_t& func, bool skipSelf = false);
-void distributeAsyncFunction(const DNSName& question, const pipefunc_t& func);
+void distributeAsyncFunction(const std::string& question, const pipefunc_t& func);
 
 int directResolve(const DNSName& qname, const QType& qtype, int qclass, vector<DNSResourceRecord>& ret);
 
diff --git a/pdns/test-packetcache_cc.cc b/pdns/test-packetcache_cc.cc
index a0b495be2b..19052c1de5 100644
--- a/pdns/test-packetcache_cc.cc
+++ b/pdns/test-packetcache_cc.cc
@@ -14,12 +14,6 @@
 #include <utility>
 extern StatBag S;
 
-
-std::ostream & operator<<(std::ostream &os, const DNSName& d)
-{
-    return os <<"DNSName("<<d.toString()<<")";
-}
-
 BOOST_AUTO_TEST_SUITE(packetcache_cc)
 
 BOOST_AUTO_TEST_CASE(test_PacketCacheSimple) {
diff --git a/pdns/ws-api.cc b/pdns/ws-api.cc
index de456aaf97..6f5026fbb7 100644
--- a/pdns/ws-api.cc
+++ b/pdns/ws-api.cc
@@ -221,7 +221,7 @@ void apiServerStatistics(HttpRequest* req, HttpResponse* resp) {
   resp->setBody(doc);
 }
 
-string apiZoneIdToName(const string& id) {
+DNSName apiZoneIdToName(const string& id) {
   string zonename;
   ostringstream ss;
 
@@ -261,14 +261,11 @@ string apiZoneIdToName(const string& id) {
 
   zonename = ss.str();
 
-  // strip trailing dot
-  if (zonename.size() > 0 && zonename.substr(zonename.size()-1) == ".") {
-    zonename.resize(zonename.size()-1);
-  }
-  return zonename;
+  return DNSName(zonename);
 }
 
-string apiZoneNameToId(const string& name) {
+string apiZoneNameToId(const DNSName& dname) {
+  string name=dname.toString();
   ostringstream ss;
 
   for(string::const_iterator iter = name.begin(); iter != name.end(); ++iter) {
diff --git a/pdns/ws-api.hh b/pdns/ws-api.hh
index e8b7431381..7c070636b6 100644
--- a/pdns/ws-api.hh
+++ b/pdns/ws-api.hh
@@ -35,8 +35,8 @@ void apiServerSearchLog(HttpRequest* req, HttpResponse* resp);
 void apiServerStatistics(HttpRequest* req, HttpResponse* resp);
 
 // helpers
-string apiZoneIdToName(const string& id);
-string apiZoneNameToId(const string& name);
+DNSName apiZoneIdToName(const string& id);
+string apiZoneNameToId(const DNSName& name);
 
 // To be provided by product code.
 void productServerStatisticsFetch(std::map<string,string>& out);
diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index 7c06b5dad2..b9082ed925 100644
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -131,11 +131,11 @@ static void apiServerConfigAllowFrom(HttpRequest* req, HttpResponse* resp)
   resp->setBody(document);
 }
 
-static void fillZone(const string& zonename, HttpResponse* resp)
+static void fillZone(const DNSName& zonename, HttpResponse* resp)
 {
-  SyncRes::domainmap_t::const_iterator iter = t_sstorage->domainmap->find(zonename);
+  auto iter = t_sstorage->domainmap->find(zonename);
   if (iter == t_sstorage->domainmap->end())
-    throw ApiException("Could not find domain '"+zonename+"'");
+    throw ApiException("Could not find domain '"+zonename.toString()+"'");
 
   Document doc;
   doc.SetObject();
@@ -143,7 +143,7 @@ static void fillZone(const string& zonename, HttpResponse* resp)
   const SyncRes::AuthDomain& zone = iter->second;
 
   // id is the canonical lookup key, which doesn't actually match the name (in some cases)
-  string zoneId = apiZoneNameToId(iter->first.toString());
+  string zoneId = apiZoneNameToId(iter->first);
   Value jzoneid(zoneId.c_str(), doc.GetAllocator()); // copy
   doc.AddMember("id", jzoneid, doc.GetAllocator());
   string url = "/servers/localhost/zones/" + zoneId;
@@ -187,14 +187,10 @@ static void doCreateZone(const Value& document)
     throw ApiException("Config Option \"experimental-api-config-dir\" must be set");
   }
 
-  string zonename = stringFromJson(document, "name");
-  // TODO: better validation of zonename - apiZoneNameToId takes care of escaping / however
-  if(zonename.empty())
+  if(stringFromJson(document, "name").empty())
     throw ApiException("Zone name empty");
-
-  if (zonename[zonename.size()-1] != '.') {
-    zonename += ".";
-  }
+  
+  DNSName zonename(stringFromJson(document, "name"));
 
   string singleIPTarget = stringFromJson(document, "single_target_ip", "");
   string kind = toUpper(stringFromJson(document, "kind"));
@@ -228,7 +224,7 @@ static void doCreateZone(const Value& document)
     }
     ofzone.close();
 
-    apiWriteConfigFile(confbasename, "auth-zones+=" + zonename + "=" + zonefilename);
+    apiWriteConfigFile(confbasename, "auth-zones+=" + zonename.toString() + "=" + zonefilename);
   } else if (kind == "FORWARDED") {
     const Value &servers = document["servers"];
     if (kind == "FORWARDED" && (!servers.IsArray() || servers.Size() == 0))
@@ -245,16 +241,16 @@ static void doCreateZone(const Value& document)
     }
 
     if (rd) {
-      apiWriteConfigFile(confbasename, "forward-zones-recurse+=" + zonename + "=" + serverlist);
+      apiWriteConfigFile(confbasename, "forward-zones-recurse+=" + zonename.toString() + "=" + serverlist);
     } else {
-      apiWriteConfigFile(confbasename, "forward-zones+=" + zonename + "=" + serverlist);
+      apiWriteConfigFile(confbasename, "forward-zones+=" + zonename.toString() + "=" + serverlist);
     }
   } else {
     throw ApiException("invalid kind");
   }
 }
 
-static bool doDeleteZone(const string& zonename)
+static bool doDeleteZone(const DNSName& zonename)
 {
   if (::arg()["experimental-api-config-dir"].empty()) {
     throw ApiException("Config Option \"experimental-api-config-dir\" must be set");
@@ -285,12 +281,9 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp)
     Document document;
     req->json(document);
 
-    string zonename = stringFromJson(document, "name");
-    if (zonename[zonename.size()-1] != '.') {
-      zonename += ".";
-    }
+    DNSName zonename(stringFromJson(document, "name"));
 
-    SyncRes::domainmap_t::const_iterator iter = t_sstorage->domainmap->find(zonename);
+    auto iter = t_sstorage->domainmap->find(zonename);
     if (iter != t_sstorage->domainmap->end())
       throw ApiException("Zone already exists");
 
@@ -312,7 +305,7 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp)
     Value jdi;
     jdi.SetObject();
     // id is the canonical lookup key, which doesn't actually match the name (in some cases)
-    string zoneId = apiZoneNameToId(val.first.toString());
+    string zoneId = apiZoneNameToId(val.first);
     Value jzoneid(zoneId.c_str(), doc.GetAllocator()); // copy
     jdi.AddMember("id", jzoneid, doc.GetAllocator());
     string url = "/servers/localhost/zones/" + zoneId;
@@ -336,12 +329,11 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp)
 
 static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp)
 {
-  string zonename = apiZoneIdToName(req->parameters["id"]);
-  zonename += ".";
+  DNSName zonename = apiZoneIdToName(req->parameters["id"]);
 
   SyncRes::domainmap_t::const_iterator iter = t_sstorage->domainmap->find(zonename);
   if (iter == t_sstorage->domainmap->end())
-    throw ApiException("Could not find domain '"+zonename+"'");
+    throw ApiException("Could not find domain '"+zonename.toString()+"'");
 
   if(req->method == "PUT" && !::arg().mustDo("experimental-api-readonly")) {
     Document document;
@@ -350,7 +342,7 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp)
     doDeleteZone(zonename);
     doCreateZone(document);
     reloadAuthAndForwards();
-    fillZone(stringFromJson(document, "name"), resp);
+    fillZone(DNSName(stringFromJson(document, "name")), resp);
   }
   else if(req->method == "DELETE" && !::arg().mustDo("experimental-api-readonly")) {
     if (!doDeleteZone(zonename)) {
@@ -380,7 +372,7 @@ static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) {
   doc.SetArray();
 
   BOOST_FOREACH(const SyncRes::domainmap_t::value_type& val, *t_sstorage->domainmap) {
-    string zoneId = apiZoneNameToId(val.first.toString());
+    string zoneId = apiZoneNameToId(val.first);
     if (pdns_ci_find(val.first.toString(), q) != string::npos) {
       Value object;
       object.SetObject();
@@ -393,7 +385,7 @@ static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) {
     }
 
     // if zone name is an exact match, don't bother with returning all records/comments in it
-    if (val.first == q) {
+    if (val.first == DNSName(q)) {
       continue;
     }
 
@@ -425,7 +417,7 @@ static void apiServerFlushCache(HttpRequest* req, HttpResponse* resp) {
   if(req->method != "PUT")
     throw HttpMethodNotAllowedException();
 
-  DNSName canon = req->getvars["domain"];
+  DNSName canon(req->getvars["domain"]);
   int count = broadcastAccFunction<uint64_t>(boost::bind(pleaseWipeCache, canon));
   count += broadcastAccFunction<uint64_t>(boost::bind(pleaseWipeAndCountNegCache, canon));
   map<string, string> object;
diff --git a/pdns/zoneparser-tng.cc b/pdns/zoneparser-tng.cc
index 5f9d6702da..e5cd013736 100644
--- a/pdns/zoneparser-tng.cc
+++ b/pdns/zoneparser-tng.cc
@@ -304,7 +304,7 @@ bool ZoneParserTNG::get(DNSResourceRecord& rr, std::string* comment)
     rr.qname=d_prevqname;
     prevqname=true;
   }else {
-    rr.qname=qname;
+    rr.qname=DNSName(qname); 
     parts.pop_front();
     if(qname.empty() || qname[0]==';')
       goto retry;
diff --git a/pdns/zoneparser-tng.hh b/pdns/zoneparser-tng.hh
index a743acc2ca..0691634259 100644
--- a/pdns/zoneparser-tng.hh
+++ b/pdns/zoneparser-tng.hh
@@ -32,7 +32,7 @@
 class ZoneParserTNG
 {
 public:
-  ZoneParserTNG(const string& fname, const DNSName& zname="", const string& reldir="");
+  ZoneParserTNG(const string& fname, const DNSName& zname=DNSName(), const string& reldir="");
   ZoneParserTNG(const vector<string> zonedata, const DNSName& zname);
 
   ~ZoneParserTNG();