Better error reporting interface for checkRRSet().

diff --git a/pdns/check-zone.cc b/pdns/check-zone.cc
index a8c1f839a0caafb79883e00c102f2ed19177d26b..beccefbfce5f855fa3239491ef38b20effc559ce 100644 (file)
--- a/pdns/check-zone.cc
+++ b/pdns/check-zone.cc
@@ -28,12 +28,7 @@
 namespace Check
 {
 
-static void rejectRecord(const DNSResourceRecord& rec, const std::string& why)
-{
-  throw CheckException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + why);
-}
-
-void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone)
+void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone, vector<pair<DNSResourceRecord, string>>& errors, bool stopAtFirstError)
 {
   // QTypes that MUST NOT have multiple records of the same type in a given RRset.
   static const std::set<uint16_t> onlyOneEntryTypes = {QType::CNAME, QType::DNAME, QType::SOA};
@@ -42,6 +37,10 @@ void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone)
   // QTypes that are NOT allowed at apex.
   static const std::set<uint16_t> nonApexTypes = {QType::DS};
 
+  if (stopAtFirstError) {
+    errors.reserve(1);
+  }
+
   sort(records.begin(), records.end(),
        [](const DNSResourceRecord& rec_a, const DNSResourceRecord& rec_b) -> bool {
          /* we need _strict_ weak ordering */
@@ -53,27 +52,42 @@ void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone)
     if (previous.qname == rec.qname) {
       if (previous.qtype == rec.qtype) {
         if (onlyOneEntryTypes.count(rec.qtype.getCode()) != 0) {
-          rejectRecord(rec, ": only one such record allowed");
+          errors.emplace_back(std::make_pair(rec, ": only one such record allowed"));
+          if (stopAtFirstError) {
+            break;
+          }
         }
         if (previous.content == rec.content) {
-          rejectRecord(rec, std::string{": duplicate record with content \""} + rec.content + "\"");
+          errors.emplace_back(std::make_pair(rec, std::string{": duplicate record with content \""} + rec.content + "\""));
+          if (stopAtFirstError) {
+            break;
+          }
         }
       }
       else {
         if (QType::exclusiveEntryTypes.count(rec.qtype.getCode()) != 0
             || QType::exclusiveEntryTypes.count(previous.qtype.getCode()) != 0) {
-          rejectRecord(rec, std::string{": conflicts with existing "} + previous.qtype.toString() + " RRset of the same name");
+          errors.emplace_back(std::make_pair(rec, std::string{": conflicts with existing "} + previous.qtype.toString() + " RRset of the same name"));
+          if (stopAtFirstError) {
+            break;
+          }
         }
       }
     }
 
     if (rec.qname == zone.operator const DNSName&()) {
       if (nonApexTypes.count(rec.qtype.getCode()) != 0) {
-        rejectRecord(rec, ": is not allowed at apex");
+        errors.emplace_back(std::make_pair(rec, ": is not allowed at apex"));
+        if (stopAtFirstError) {
+          break;
+        }
       }
     }
     else if (atApexTypes.count(rec.qtype.getCode()) != 0) {
-      rejectRecord(rec, ": is only allowed at apex");
+      errors.emplace_back(std::make_pair(rec, ": is only allowed at apex"));
+      if (stopAtFirstError) {
+        break;
+      }
     }
 
     // Check if the DNSNames that should be hostnames, are hostnames
@@ -81,7 +95,10 @@ void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone)
       checkHostnameCorrectness(rec);
     }
     catch (const std::exception& e) {
-      rejectRecord(rec, std::string{": "} + e.what());
+      errors.emplace_back(std::make_pair(rec, std::string{": "} + e.what()));
+      if (stopAtFirstError) {
+        break;
+      }
     }
 
     previous = rec;

diff --git a/pdns/check-zone.hh b/pdns/check-zone.hh
index 4bbcac4d2abf1d0a75d3d3554b844fc76841440c..d71b367dde18e9e2795b5f5cb647d0ca2c02d80f 100644 (file)
--- a/pdns/check-zone.hh
+++ b/pdns/check-zone.hh
@@ -20,13 +20,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-class CheckException : public runtime_error
-{
-public:
-  CheckException(const string& what_arg) :
-    runtime_error(what_arg) {}
-};
-
 namespace Check
 {
 
@@ -38,6 +31,6 @@ namespace Check
  *   *) no duplicates for QTypes that can only be present once per RRset
  *   *) hostnames are hostnames
  */
-void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone);
+void checkRRSet(vector<DNSResourceRecord>& records, const ZoneName& zone, vector<pair<DNSResourceRecord, string>>& errors, bool stopAtFirstError);
 
 } // namespace Check

diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc
index 54cae5e4bbacc8dcf47f036cd38256b4306825bc..47c08c3e8bc8f21b59b97dc3da4e184351c3249e 100644 (file)
--- a/pdns/pdnsutil.cc
+++ b/pdns/pdnsutil.cc
@@ -2558,13 +2558,16 @@ static int addOrReplaceRecord(bool isAdd, const vector<string>& cmds)
     newrrs = std::move(oldrrs);
   }
 
-  try {
-    Check::checkRRSet(newrrs, zone);
-  }
-  catch (CheckException& e) {
-    cerr << e.what() << endl;
+  std::vector<std::pair<DNSResourceRecord, string>> errors;
+  Check::checkRRSet(newrrs, zone, errors, false);
+  if (!errors.empty()) {
+    for (const auto& error : errors) {
+      const auto [rec, why] = error;
+      cerr << "RRset " << rec.qname.toString() << " IN " << rec.qtype.toString() << why << endl;
+    }
     return EXIT_FAILURE;
   }
+
   if (isAdd) {
     // We had collected all record types earlier in order to be able to
     // perform the proper checks. Trim the list to only keep those of the

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index d83ccc33fea47bae656945d08d985a91ca95bf2c..d02bbbd2d5eeb7c709c0c544531673b2a1db6a79 100644 (file)
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -1665,11 +1665,12 @@ static void gatherRecordsFromZone(const std::string& zonestring, vector<DNSResou
  */
 static void checkNewRecords(vector<DNSResourceRecord>& records, const ZoneName& zone)
 {
-  try {
-    Check::checkRRSet(records, zone);
-  }
-  catch (CheckException& e) {
-    throw ApiException(e.what());
+  std::vector<std::pair<DNSResourceRecord, string>> errors;
+
+  Check::checkRRSet(records, zone, errors, true);
+  if (!errors.empty()) {
+    const auto [rec, why] = errors.front();
+    throw ApiException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + why);
   }
 }