From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 8 Apr 2020 13:26:08 +0000 (+0200)
Subject: Avoid self-assignment of DNSName, some version of boost::container::string
X-Git-Tag: dnsdist-1.5.0-rc2~17^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d720eb8add5ebda11867e8b404125e0b68ed2911;p=thirdparty%2Fpdns.git

Avoid self-assignment of DNSName, some version of boost::container::string
do not like that and throw an assertion.
---

diff --git a/modules/remotebackend/Makefile.am b/modules/remotebackend/Makefile.am
index e106047624..762df16080 100644
--- a/modules/remotebackend/Makefile.am
+++ b/modules/remotebackend/Makefile.am
@@ -125,6 +125,7 @@ libtestremotebackend_la_SOURCES = \
 	../../pdns/unix_utility.cc \
 	../../pdns/gss_context.cc ../../pdns/gss_context.hh \
 	../../pdns/json.hh ../../pdns/json.cc \
+	../../pdns/shuffle.hh ../../pdns/shuffle.cc \
 	httpconnector.cc \
 	pipeconnector.cc \
 	unixconnector.cc \
diff --git a/pdns/Makefile.am b/pdns/Makefile.am
index 3aa748fd6f..2436ee0550 100644
--- a/pdns/Makefile.am
+++ b/pdns/Makefile.am
@@ -920,7 +920,9 @@ dnsbulktest_SOURCES = \
 	rcpgenerator.cc \
 	sillyrecords.cc \
 	statbag.cc \
-	unix_utility.cc
+	unix_utility.cc \
+	arguments.cc arguments.hh \
+	dns_random.cc dns_random.hh
 
 dnsbulktest_LDFLAGS = \
 	$(AM_LDFLAGS) \
@@ -1083,6 +1085,7 @@ pdns_notify_LDADD = \
 
 if LIBSODIUM
 pdns_notify_LDADD += $(LIBSODIUM_LIBS)
+dnsbulktest_LDADD += $(LIBSODIUM_LIBS)
 endif
 
 dnsscope_SOURCES = \
diff --git a/pdns/calidns.cc b/pdns/calidns.cc
index 39796e5f71..d07ac19f4a 100644
--- a/pdns/calidns.cc
+++ b/pdns/calidns.cc
@@ -408,7 +408,8 @@ try
     }
     unknown.emplace_back(std::make_shared<vector<uint8_t>>(packet));
   }
-  random_shuffle(unknown.begin(), unknown.end());
+
+  shuffle(unknown.begin(), unknown.end(), pdns::dns_random_engine());
   if (!g_quiet) {
     cout<<"Generated "<<unknown.size()<<" ready to use queries"<<endl;
   }
@@ -473,7 +474,8 @@ try
     for(;n < total; ++n) {
       toSend.push_back(known[dns_random(known.size())].get());
     }
-    random_shuffle(toSend.begin(), toSend.end());
+
+    shuffle(toSend.begin(), toSend.end(), pdns::dns_random_engine());
     g_recvcounter.store(0);
     g_recvbytes=0;
     DTime dt;
diff --git a/pdns/dnsbulktest.cc b/pdns/dnsbulktest.cc
index 18bfeee6cf..a93866e99d 100644
--- a/pdns/dnsbulktest.cc
+++ b/pdns/dnsbulktest.cc
@@ -34,6 +34,8 @@
 #include "misc.hh"
 #include "dnswriter.hh"
 #include "dnsrecords.hh"
+#include "dns_random.hh"
+#include "arguments.hh"
 
 using namespace boost::accumulators;
 namespace po = boost::program_options;
@@ -42,6 +44,12 @@ po::variables_map g_vm;
 
 StatBag S;
 
+ArgvMap &arg()
+{
+  static ArgvMap theArg;
+  return theArg;
+}
+
 bool g_quiet=false;
 bool g_envoutput=false;
 
@@ -307,7 +315,7 @@ try
       domains.push_back(TypedQuery("www."+split.second, qtype));
   }
   cerr<<"Read "<<domains.size()<<" domains!"<<endl;
-  random_shuffle(domains.begin(), domains.end());
+  shuffle(domains.begin(), domains.end(), pdns::dns_random_engine());
 
   boost::format datafmt("%s %|20t|%+15s  %|40t|%s %|60t|%+15s\n");
 
diff --git a/pdns/dnsname.hh b/pdns/dnsname.hh
index f933a62252..d23be23d90 100644
--- a/pdns/dnsname.hh
+++ b/pdns/dnsname.hh
@@ -63,6 +63,23 @@ class DNSName
 {
 public:
   DNSName()  {}          //!< Constructs an *empty* DNSName, NOT the root!
+  // Work around assertion in some boost versions that do not like self-assignment of boost::container::string
+  DNSName& operator=(const DNSName& rhs)
+  {
+    if (this != &rhs) {
+      d_storage = rhs.d_storage;
+    }
+    return *this;
+  }
+  DNSName& operator=(const DNSName&& rhs)
+  {
+    if (this != &rhs) {
+      d_storage = std::move(rhs.d_storage);
+    }
+    return *this;
+  }
+  DNSName(const DNSName& a) = default;
+  DNSName(DNSName&& a) = default;
   explicit DNSName(const char* p): DNSName(p, std::strlen(p)) {} //!< Constructs from a human formatted, escaped presentation
   explicit DNSName(const char* p, size_t len);      //!< Constructs from a human formatted, escaped presentation
   explicit DNSName(const std::string& str) : DNSName(str.c_str(), str.length()) {}; //!< Constructs from a human formatted, escaped presentation
diff --git a/pdns/dynhandler.cc b/pdns/dynhandler.cc
index db1615909d..4aef4a2f32 100644
--- a/pdns/dynhandler.cc
+++ b/pdns/dynhandler.cc
@@ -254,7 +254,7 @@ string DLNotifyRetrieveHandler(const vector<string>&parts, Utility::pid_t ppid)
   if(di.kind != DomainInfo::Slave || di.masters.empty())
     return "Domain '"+domain.toString()+"' is not a slave domain (or has no master defined)";
 
-  random_shuffle(di.masters.begin(), di.masters.end());
+  shuffle(di.masters.begin(), di.masters.end(), pdns::dns_random_engine());
   Communicator.addSuckRequest(domain, di.masters.front()); 
   return "Added retrieval request for '"+domain.toString()+"' from master "+di.masters.front().toLogString();
 }
diff --git a/pdns/shuffle.cc b/pdns/shuffle.cc
index 6440d77c66..64c4bba7dd 100644
--- a/pdns/shuffle.cc
+++ b/pdns/shuffle.cc
@@ -33,58 +33,81 @@
 void pdns::shuffle(std::vector<DNSZoneRecord>& rrs)
 {
   std::vector<DNSZoneRecord>::iterator first, second;
-  for(first=rrs.begin();first!=rrs.end();++first)
-    if(first->dr.d_place==DNSResourceRecord::ANSWER && first->dr.d_type != QType::CNAME) // CNAME must come first
+
+  // We assume the CNAMES are listed firts in the ANSWWER section and the the other records
+  // and we want to shuffle the other records only
+
+  // First we scan for the first non-CNAME ANSWER record
+  for (first = rrs.begin(); first != rrs.end(); ++first) {
+    if (first->dr.d_place == DNSResourceRecord::ANSWER && first->dr.d_type != QType::CNAME) {
       break;
-  for(second=first;second!=rrs.end();++second)
-    if(second->dr.d_place!=DNSResourceRecord::ANSWER)
+    }
+  }
+  // And then for one past the last ANSWER recordd
+  for (second = first; second != rrs.end(); ++second)
+    if (second->dr.d_place != DNSResourceRecord::ANSWER)
       break;
 
+  // Now shuffle the non-CNAME ANSWER records
   dns_random_engine r;
-  if(second-first > 1)
+  if (second - first > 1) {
     shuffle(first, second, r);
+  }
 
-  // now shuffle the additional records
-  for(first=second;first!=rrs.end();++first)
-    if(first->dr.d_place==DNSResourceRecord::ADDITIONAL && first->dr.d_type != QType::CNAME) // CNAME must come first
+  // now shuffle the ADDITIONAL records in the same manner as the ANSWER records
+  for (first = second; first != rrs.end(); ++first) {
+    if (first->dr.d_place == DNSResourceRecord::ADDITIONAL && first->dr.d_type != QType::CNAME) {
       break;
-  for(second=first;second!=rrs.end();++second)
-    if(second->dr.d_place!=DNSResourceRecord::ADDITIONAL)
+    }
+  }
+  for (second = first; second != rrs.end(); ++second) {
+    if (second->dr.d_place != DNSResourceRecord::ADDITIONAL) {
       break;
+    }
+  }
 
-  if(second-first>1)
+  if (second - first > 1) {
     shuffle(first, second, r);
-
+  }
   // we don't shuffle the rest
 }
 
-
 // shuffle, maintaining some semblance of order
-void pdns::shuffle(std::vector<DNSRecord>& rrs)
+static void shuffle(std::vector<DNSRecord>& rrs)
 {
+  // This shuffles in the same style as the above method, keeping CNAME in the front and RRSIGs at the end
   std::vector<DNSRecord>::iterator first, second;
-  for(first=rrs.begin();first!=rrs.end();++first)
-    if(first->d_place==DNSResourceRecord::ANSWER && first->d_type != QType::CNAME) // CNAME must come first
+  for (first = rrs.begin(); first != rrs.end(); ++first) {
+    if (first->d_place == DNSResourceRecord::ANSWER && first->d_type != QType::CNAME) {
       break;
-  for(second=first;second!=rrs.end();++second)
-    if(second->d_place!=DNSResourceRecord::ANSWER || second->d_type == QType::RRSIG) // leave RRSIGs at the end
+    }
+  }
+  for (second = first; second != rrs.end(); ++second) {
+    if (second->d_place != DNSResourceRecord::ANSWER || second->d_type == QType::RRSIG) {
       break;
+    }
+  }
 
-  dns_random_engine r;
-  if(second-first>1)
+  pdns::dns_random_engine r;
+  if (second - first > 1) {
     shuffle(first, second, r);
+  }
 
   // now shuffle the additional records
-  for(first=second;first!=rrs.end();++first)
-    if(first->d_place==DNSResourceRecord::ADDITIONAL && first->d_type != QType::CNAME) // CNAME must come first
+  for (first = second; first != rrs.end(); ++first) {
+    if (first->d_place == DNSResourceRecord::ADDITIONAL && first->d_type != QType::CNAME) {
       break;
-  for(second=first; second!=rrs.end(); ++second)
-    if(second->d_place!=DNSResourceRecord::ADDITIONAL)
+    }
+  }
+  for (second = first; second != rrs.end(); ++second) {
+    if (second->d_place != DNSResourceRecord::ADDITIONAL) {
       break;
+    }
+  }
 
-  if(second-first>1)
+  if (second - first > 1) {
     shuffle(first, second, r);
-
+  }
   // we don't shuffle the rest
 }
 
@@ -102,8 +125,8 @@ static uint16_t mapTypesToOrder(uint16_t type)
 // then shuffle the parts that desire shuffling
 void pdns::orderAndShuffle(vector<DNSRecord>& rrs)
 {
-  std::stable_sort(rrs.begin(), rrs.end(), [](const DNSRecord&a, const DNSRecord& b) { 
-      return std::make_tuple(a.d_place, mapTypesToOrder(a.d_type)) < std::make_tuple(b.d_place, mapTypesToOrder(b.d_type));
-    });
+  std::stable_sort(rrs.begin(), rrs.end(), [](const DNSRecord& a, const DNSRecord& b) {
+    return std::make_tuple(a.d_place, mapTypesToOrder(a.d_type)) < std::make_tuple(b.d_place, mapTypesToOrder(b.d_type));
+  });
   shuffle(rrs);
 }
diff --git a/pdns/shuffle.hh b/pdns/shuffle.hh
index c42e400720..8917204904 100644
--- a/pdns/shuffle.hh
+++ b/pdns/shuffle.hh
@@ -25,9 +25,8 @@
 struct DNSRecord;
 struct DNSZoneRecord;
 
-namespace pdns {
-  void shuffle(std::vector<DNSRecord>& rrs);
-  void shuffle(std::vector<DNSZoneRecord>& rrs);
-  void orderAndShuffle(std::vector<DNSRecord>& rrs);
+namespace pdns
+{
+void shuffle(std::vector<DNSZoneRecord>& rrs);
+void orderAndShuffle(std::vector<DNSRecord>& rrs);
 }
-
diff --git a/pdns/signingpipe.cc b/pdns/signingpipe.cc
index 83beaa8fd6..c129a4193a 100644
--- a/pdns/signingpipe.cc
+++ b/pdns/signingpipe.cc
@@ -3,6 +3,7 @@
 #endif
 #include "signingpipe.hh"
 #include "misc.hh"
+#include "dns_random.hh"
 #include <poll.h>
 
 #include <sys/socket.h>
@@ -196,7 +197,7 @@ void ChunkedSigningPipe::sendRRSetToWorker() // it sounds so socialist!
   rwVect = waitForRW(wantRead, wantWrite, -1); // wait for something to happen
   
   if(wantWrite && !rwVect.second.empty()) {
-    random_shuffle(rwVect.second.begin(), rwVect.second.end()); // pick random available worker
+    shuffle(rwVect.second.begin(), rwVect.second.end(), pdns::dns_random_engine()); // pick random available worker
     auto ptr = d_rrsetToSign.release();
     writen2(*rwVect.second.begin(), &ptr, sizeof(ptr));
     d_rrsetToSign = make_unique<rrset_t>();
@@ -245,7 +246,7 @@ void ChunkedSigningPipe::sendRRSetToWorker() // it sounds so socialist!
   
   if(wantWrite) {  // our optimization above failed, we now wait synchronously
     rwVect = waitForRW(false, wantWrite, -1); // wait for something to happen
-    random_shuffle(rwVect.second.begin(), rwVect.second.end()); // pick random available worker
+    shuffle(rwVect.second.begin(), rwVect.second.end(), pdns::dns_random_engine()); // pick random available worker
     auto ptr = d_rrsetToSign.release();
     writen2(*rwVect.second.begin(), &ptr, sizeof(ptr));
     d_rrsetToSign = make_unique<rrset_t>();
diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc
index e88fc24791..f6775de310 100644
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -694,7 +694,7 @@ struct SlaveSenderReceiver
 
   Identifier send(DomainNotificationInfo& dni)
   {
-    random_shuffle(dni.di.masters.begin(), dni.di.masters.end());
+    shuffle(dni.di.masters.begin(), dni.di.masters.end(), pdns::dns_random_engine());
     try {
       return std::make_tuple(dni.di.zone,
                              *dni.di.masters.begin(),
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 50ac112d10..d63952b282 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1657,9 +1657,7 @@ inline std::vector<std::pair<DNSName, float>> SyncRes::shuffleInSpeedOrder(NsSet
       return rnameservers;
   }
 
-  // Using  shuffle(rnameservers.begin(), rnameservers.end(), pdsn:dns_ramndom_engine()) causes a boost assert,
-  // to be investigated
-  random_shuffle(rnameservers.begin(),rnameservers.end());
+  shuffle(rnameservers.begin(),rnameservers.end(), pdns::dns_random_engine());
   speedOrder so;
   stable_sort(rnameservers.begin(),rnameservers.end(), so);
 
diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 8a8c4336e9..7b9143f59a 100644
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -1836,7 +1836,7 @@ static void apiServerZoneAxfrRetrieve(HttpRequest* req, HttpResponse* resp) {
   if(di.masters.empty())
     throw ApiException("Domain '"+zonename.toString()+"' is not a slave domain (or has no master defined)");
 
-  random_shuffle(di.masters.begin(), di.masters.end());
+  shuffle(di.masters.begin(), di.masters.end(), pdns::dns_random_engine());
   Communicator.addSuckRequest(zonename, di.masters.front());
   resp->setSuccessResult("Added retrieval request for '"+zonename.toString()+"' from master "+di.masters.front().toLogString());
 }