remove d_empty, length()

diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index b039de17948644a8ae6deb3c090af7ae3154253b..8388391d47a4d90b584d09f35601a6a74e6660a1 100644 (file)
--- a/pdns/dnsname.cc
+++ b/pdns/dnsname.cc
@@ -8,11 +8,8 @@
 #include <boost/functional/hash.hpp>
 
 /* raw storage
-   in DNS label format, without trailing 0. So the root is of length 0.
-
-   www.powerdns.com = 3www8powerdns3com
-
-   a primitive is nextLabel()
+   in DNS label format, with trailing 0. W/o trailing 0, we are 'empty'
+   www.powerdns.com = 3www8powerdns3com0
 */
 
 std::ostream & operator<<(std::ostream &os, const DNSName& d)
@@ -23,16 +20,18 @@ std::ostream & operator<<(std::ostream &os, const DNSName& d)
 
 DNSName::DNSName(const char* p)
 {
-  d_empty=false;
-  d_storage.reserve(strlen(p)+1);
-  auto labels = segmentDNSName(p);
-  for(const auto& e : labels)
-    appendRawLabel(e);
+  if(p[0]=='.' && p[1]==0) {
+    d_storage.assign(1, (char)0);
+  } else {
+    d_storage.reserve(strlen(p)+1);
+    auto labels = segmentDNSName(p);
+    for(const auto& e : labels)
+      appendRawLabel(e);
+  }
 }
 
 DNSName::DNSName(const char* pos, int len, int offset, bool uncompress, uint16_t* qtype, uint16_t* qclass, unsigned int* consumed)
 {
-  d_empty=false;
   packetParser(pos, len, offset, uncompress, qtype, qclass, consumed);
 }
 
@@ -81,22 +80,24 @@ void DNSName::packetParser(const char* pos, int len, int offset, bool uncompress
 std::string DNSName::toString(const std::string& separator, const bool trailing) const
 {
   if (empty())
-    return "";
-  if(empty() && trailing)  // I keep wondering if there is some deeper meaning to the need to do this
-    return separator;
+    throw std::out_of_range("Attempt to print an unset dnsname");
+
   std::string ret;
   for(const auto& s : getRawLabels()) {
     ret+= escapeLabel(s) + separator;
   }
+  if(ret.empty())
+    return trailing ? separator : "";
+
   return ret.substr(0, ret.size()-!trailing);
 }
 
 std::string DNSName::toDNSString() const
 {
-  //  if (empty())
-  //  return "";
+  if (empty())
+    throw std::out_of_range("Attempt to DNSString an unset dnsname");
+
   string ret(d_storage.c_str(), d_storage.length());
-  ret.append(1,(char)0);
   return toLower(ret); // toLower or not toLower, that is the question
   // return ret;
 }
@@ -107,16 +108,15 @@ std::string DNSName::toDNSString() const
  * @return the total wirelength of the DNSName
  */
 size_t DNSName::wirelength() const {
-  return d_storage.length() + 1;
+  return d_storage.length();
 }
 
-// are WE part of parent
+// Are WE part of parent
 bool DNSName::isPartOf(const DNSName& parent) const
 {
   if(parent.empty() || empty())
-    return false;
-  if(parent.d_storage.empty())
-    return true;
+    throw std::out_of_range("empty dnsnames aren't part of anything");
+
   if(parent.d_storage.size() > d_storage.size())
     return false;
 
@@ -144,6 +144,7 @@ void DNSName::makeUsRelative(const DNSName& zone)
 {
   if (isPartOf(zone)) {
     d_storage.erase(d_storage.size()-zone.d_storage.size());
+    d_storage.append(1, (char)0); // put back the trailing 0
   } 
   else
     clear();
@@ -168,12 +169,17 @@ void DNSName::appendRawLabel(const std::string& label)
     throw std::range_error("no such thing as an empty label to append");
   if(label.size() > 63)
     throw std::range_error("label too long to append");
-  if(d_storage.size() + label.size() > 253) // reserve two bytes, one for length and one for the root label
+  if(d_storage.size() + label.size() > 254) // reserve two bytes, one for length and one for the root label
     throw std::range_error("name too long to append");
 
-  d_empty=false;
-  d_storage.append(1, (char)label.size());
+  if(d_storage.empty()) {
+    d_storage.append(1, (char)label.size());
+  }
+  else {
+    *d_storage.rbegin()=(char)label.size();
+  }
   d_storage.append(label.c_str(), label.length());
+  d_storage.append(1, (char)0);
 }
 
 void DNSName::prependRawLabel(const std::string& label)
@@ -182,10 +188,12 @@ void DNSName::prependRawLabel(const std::string& label)
     throw std::range_error("no such thing as an empty label to prepend");
   if(label.size() > 63)
     throw std::range_error("label too long to prepend");
-  if(d_storage.size() + label.size() > 253) // reserve two bytes, one for length and one for the root label
+  if(d_storage.size() + label.size() > 254) // reserve two bytes, one for length and one for the root label
     throw std::range_error("name too long to prepend");
 
-  d_empty=false;
+  if(d_storage.empty())
+    d_storage.append(1, (char)0);
+
   string_t prep(1, (char)label.size());
   prep.append(label.c_str(), label.size());
   d_storage = prep+d_storage;
@@ -201,8 +209,8 @@ vector<string> DNSName::getRawLabels() const
 {
   vector<string> ret;
 
-  // 3www4ds9a2nl
-  for(const char* p = d_storage.c_str(); p < d_storage.c_str() + d_storage.size(); p+=*p+1)
+  // 3www4ds9a2nl0
+  for(const char* p = d_storage.c_str(); p < d_storage.c_str() + d_storage.size() && *p; p+=*p+1)
     ret.push_back({p+1, (unsigned int)*p}); // XXX FIXME
   return ret;
 }
@@ -210,7 +218,7 @@ vector<string> DNSName::getRawLabels() const
 
 bool DNSName::chopOff()
 {
-  if(d_storage.empty())
+  if(d_storage.empty() || d_storage[0]==0)
     return false;
   d_storage = d_storage.substr((unsigned int)d_storage[0]+1);
   return true;
@@ -227,7 +235,7 @@ bool DNSName::isWildcard() const
 unsigned int DNSName::countLabels() const
 {
   unsigned int count=0;
-  for(const char* p = d_storage.c_str(); p < d_storage.c_str() + d_storage.size(); p+=*p+1)
+  for(const char* p = d_storage.c_str(); p < d_storage.c_str() + d_storage.size() && *p; p+=*p+1)
     ++count;
   return count;
 }
@@ -245,7 +253,7 @@ bool DNSName::operator==(const DNSName& rhs) const
 
   auto us = d_storage.crbegin();
   auto p = rhs.d_storage.crbegin();
-  for(; us != d_storage.crend() && p != rhs.d_storage.crend(); ++us, ++p) {
+  for(; us != d_storage.crend() && p != rhs.d_storage.crend(); ++us, ++p) {   // why does this go backward? 
     if(tolower(*p) != tolower(*us))
       return false;
   }

diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
index 6bc80a9a50af6a3157f6fb4fbd612908f9dcb005..0822497971ebcd423758a7e82e4412365b9e46bc 100644 (file)
--- a/pdns/dnsname.hh
+++ b/pdns/dnsname.hh
@@ -27,13 +27,13 @@
 class DNSName
 {
 public:
-  DNSName() : d_empty(true) {}          //!< Constructs an *empty* DNSName, NOT the root!
+  DNSName()  {}          //!< Constructs an *empty* DNSName, NOT the root!
   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)
+  bool operator==(const DNSName& rhs) const; //!< DNS-native comparison (case insensitive) - empty compares to empty
   bool operator!=(const DNSName& other) const { return !(*this == other); }
 
   std::string toString(const std::string& separator=".", const bool trailing=true) const;              //!< Our human-friendly, escaped, representation
@@ -49,17 +49,22 @@ public:
   bool isWildcard() const;
   unsigned int countLabels() const;
   size_t wirelength() const; //!< Number of total bytes in the name
-  bool empty() const { return d_empty; }
-  bool isRoot() const { return !d_empty && d_storage.empty(); }
-  void clear() { d_storage.clear(); d_empty=true; }
+  bool empty() const { return d_storage.empty(); }
+  bool isRoot() const { return d_storage.size()==1 && d_storage[0]==0; }
+  void clear() { d_storage.clear(); }
   void trimToLabels(unsigned int);
   DNSName& operator+=(const DNSName& rhs)
   {
-    if(d_storage.size() + rhs.d_storage.size() > 254) // reserve one byte for the root label
+    if(d_storage.size() + rhs.d_storage.size() > 256) // reserve one byte for the root label
       throw std::range_error("name too long");
+    if(rhs.empty())
+      return *this;
+
+    if(d_storage.empty())
+      d_storage+=rhs.d_storage;
+    else
+      d_storage.replace(d_storage.length()-1, rhs.d_storage.length(), rhs.d_storage);
 
-    d_storage+=rhs.d_storage;
-    d_empty&=rhs.d_empty;
     return *this;
   }
 
@@ -76,7 +81,6 @@ public:
   void serialize(Archive &ar, const unsigned int version)
   {
     ar & d_storage;
-    ar & d_empty;
   }
 
   inline bool canonCompare(const DNSName& rhs) const;
@@ -86,7 +90,6 @@ private:
   typedef std::string string_t;
 
   string_t d_storage;
-  bool d_empty;
 
   void packetParser(const char* p, int len, int offset, bool uncompress, uint16_t* qtype=0, uint16_t* qclass=0, unsigned int* consumed=0, int depth=0);
   static std::string escapeLabel(const std::string& orig);
@@ -116,9 +119,9 @@ inline bool DNSName::canonCompare(const DNSName& rhs) const
   uint8_t ourpos[64], rhspos[64];
   uint8_t ourcount=0, rhscount=0;
   //cout<<"Asked to compare "<<toString()<<" to "<<rhs.toString()<<endl;
-  for(const unsigned char* p = (const unsigned char*)d_storage.c_str(); p < (const unsigned char*)d_storage.c_str() + d_storage.size() && ourcount < sizeof(ourpos); p+=*p+1)
+  for(const unsigned char* p = (const unsigned char*)d_storage.c_str(); p < (const unsigned char*)d_storage.c_str() + d_storage.size() && *p && ourcount < sizeof(ourpos); p+=*p+1)
     ourpos[ourcount++]=(p-(const unsigned char*)d_storage.c_str());
-  for(const unsigned char* p = (const unsigned char*)rhs.d_storage.c_str(); p < (const unsigned char*)rhs.d_storage.c_str() + rhs.d_storage.size() && rhscount < sizeof(rhspos); p+=*p+1)
+  for(const unsigned char* p = (const unsigned char*)rhs.d_storage.c_str(); p < (const unsigned char*)rhs.d_storage.c_str() + rhs.d_storage.size() && *p && rhscount < sizeof(rhspos); p+=*p+1)
     rhspos[rhscount++]=(p-(const unsigned char*)rhs.d_storage.c_str());
 
   if(ourcount == sizeof(ourpos) || rhscount==sizeof(rhspos)) {

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index e963baca4d7b779c9814b4a577a265d704179eec..64c515ede9666d48280dcd70125c3ef0bbaef390 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1184,7 +1184,7 @@ int SyncRes::doResolveAt(set<DNSName> nameservers, DNSName auth, bool flawedNSSe
           LOG(prefix<<qname.toString()<<": got negative caching indication for name '"<<qname.toString()+"' (accept="<<dottedEndsOn(rec.d_name, auth)<<"), newtarget='"<<newtarget.toString()<<"'"<<endl);
 
           rec.d_ttl = min(rec.d_ttl, s_maxnegttl);
-          if(!newtarget.length()) // only add a SOA if we're not going anywhere after this
+          if(newtarget.empty()) // only add a SOA if we're not going anywhere after this
             ret.push_back(rec);
 
           NegCacheEntry ne;

diff --git a/pdns/test-dnsname_cc.cc b/pdns/test-dnsname_cc.cc
index f0b640da2618845c9669121ee83d6e799aa96daf..f102b45a26030a813913db7257c5cfaae648fd76 100644 (file)
--- a/pdns/test-dnsname_cc.cc
+++ b/pdns/test-dnsname_cc.cc
@@ -14,7 +14,6 @@ BOOST_AUTO_TEST_SUITE(dnsname_cc)
 BOOST_AUTO_TEST_CASE(test_basic) {
   string before("www.ds9a.nl.");
   DNSName b(before);
-
   BOOST_CHECK_EQUAL(b.getRawLabels().size(), 3);
   string after(b.toString());
   BOOST_CHECK_EQUAL(before, after);
@@ -46,12 +45,6 @@ BOOST_AUTO_TEST_CASE(test_basic) {
     BOOST_CHECK(name != parent);
   }
 
-  { // Check root part of root
-    DNSName name;
-    DNSName parent;
-    BOOST_CHECK(!name.isPartOf(parent));
-  }
-
   { // Check name part of root
     DNSName name("a.");
     DNSName parent(".");
@@ -91,7 +84,7 @@ BOOST_AUTO_TEST_CASE(test_basic) {
   { // Make relative
     DNSName name("aaaa.bbb.cc.d.");
     DNSName parent("cc.d.");
-    BOOST_CHECK( name.makeRelative(parent) == DNSName("aaaa.bbb."));
+    BOOST_CHECK_EQUAL( name.makeRelative(parent), DNSName("aaaa.bbb."));
   }
 
   { // Labelreverse
@@ -117,14 +110,13 @@ BOOST_AUTO_TEST_CASE(test_basic) {
 
   BOOST_CHECK( left == DNSName("WwW.Ds9A.Nl.com."));
   
-  DNSName root;
-  BOOST_CHECK(root.toString() != ".");
+  DNSName unset;
 
-  root.appendRawLabel("www");
-  root.appendRawLabel("powerdns.com");
-  root.appendRawLabel("com");
+  unset.appendRawLabel("www");
+  unset.appendRawLabel("powerdns.com");
+  unset.appendRawLabel("com");
 
-  BOOST_CHECK_EQUAL(root.toString(), "www.powerdns\\.com.com.");
+  BOOST_CHECK_EQUAL(unset.toString(), "www.powerdns\\.com.com.");
 
   DNSName rfc4343_2_2(R"(Donald\032E\.\032Eastlake\0323rd.example.)");
   DNSName example("example.");
@@ -149,7 +141,7 @@ BOOST_AUTO_TEST_CASE(test_basic) {
 
   BOOST_CHECK_EQUAL(n.toString(), "powerdns\\.dnsmaster.powerdns.com.");
 
-  BOOST_CHECK(DNSName().toString() != ".");
+  //  BOOST_CHECK(DNSName().toString() != ".");
 
   DNSName p;
   string label("power");
@@ -179,6 +171,35 @@ BOOST_AUTO_TEST_CASE(test_dnsstrings) {
   BOOST_CHECK_EQUAL(w.toDNSString(), string("\003www\010powerdns\003com\000", 18));
 }
 
+BOOST_AUTO_TEST_CASE(test_empty) {
+  DNSName empty;
+  BOOST_CHECK_THROW(empty.toString(), std::out_of_range);
+  BOOST_CHECK_THROW(empty.toStringNoDot(), std::out_of_range);
+  BOOST_CHECK_THROW(empty.toDNSString(), std::out_of_range);
+  BOOST_CHECK(empty.empty());
+  BOOST_CHECK(!empty.isRoot());
+  BOOST_CHECK(!empty.isWildcard());
+  BOOST_CHECK_EQUAL(empty, empty);
+  BOOST_CHECK(!(empty < empty));
+  
+  DNSName root(".");
+  BOOST_CHECK(empty < root);
+
+  BOOST_CHECK_THROW(empty.isPartOf(root), std::out_of_range);
+  BOOST_CHECK_THROW(root.isPartOf(empty), std::out_of_range);
+}
+
+BOOST_AUTO_TEST_CASE(test_specials) {
+  DNSName root(".");
+  
+  BOOST_CHECK(root.isRoot());
+  BOOST_CHECK(root != DNSName());
+
+  DNSName wcard("*.powerdns.com");
+  BOOST_CHECK(wcard.isWildcard());
+}
+
+
 BOOST_AUTO_TEST_CASE(test_chopping) {
   DNSName w("www.powerdns.com.");
   BOOST_CHECK_EQUAL(w.toString(), "www.powerdns.com.");
@@ -274,7 +295,6 @@ BOOST_AUTO_TEST_CASE(test_packetParse) {
 
   DNSName dn3((char*)&packet[0], packet.size(), 12+13+4+2 + 4 + 4 + 2, true);
   BOOST_CHECK_EQUAL(dn3.toString(), "ns1.powerdns.com."); 
-
   try {
     DNSName dn4((char*)&packet[0], packet.size(), 12+13+4, false); // compressed, should fail
     BOOST_CHECK(0); 
@@ -285,6 +305,7 @@ BOOST_AUTO_TEST_CASE(test_packetParse) {
 BOOST_AUTO_TEST_CASE(test_escaping) {
   DNSName n;
   string label;
+
   for(int i = 0; i < 250; ++i) {
     if(!((i+1)%63)) {
       n.appendRawLabel(label);
@@ -322,7 +343,7 @@ BOOST_AUTO_TEST_CASE(test_suffixmatch) {
 
   BOOST_CHECK(!smn.check(DNSName("www.news.gov.uk.")));
 
-  smn.add(DNSName()); // block the root
+  smn.add(DNSName(".")); // block the root
   BOOST_CHECK(smn.check(DNSName("a.root-servers.net.")));
 }
 
@@ -443,6 +464,7 @@ BOOST_AUTO_TEST_CASE(test_name_length_max) { // 255 char name
 
   { // concat
     DNSName dn(name);
+
     dn += DNSName(label + ".");
     BOOST_CHECK_EQUAL(dn.toString().size(), 254);
   }