From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 10 Dec 2024 08:20:04 +0000 (+0100)
Subject: also test sigs
X-Git-Tag: dnsdist-2.0.0-alpha1~195^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4a2ec313f7fdec129d249db795b0bbeb1232c484;p=thirdparty%2Fpdns.git

also test sigs
---

diff --git a/pdns/recursordist/recursor_cache.hh b/pdns/recursordist/recursor_cache.hh
index 3ce9186e64..f0f05d6feb 100644
--- a/pdns/recursordist/recursor_cache.hh
+++ b/pdns/recursordist/recursor_cache.hh
@@ -78,12 +78,22 @@ public:
   // are optimizations: an empty vector will be stored as a nullptr, but get() will return a pointer
   // to an already existing empty vector in that case, this is more convenient for the caller, since
   // it avoid checking for nullptr, just iterate as for the non-empty case.
+  //
+  // get() will return a shared vector to a const vector of shared pointers. Only a single shared
+  // pointer gets copied, while earlier code would copy all shared pointer in the vector.
+  //
+  // In the current SyncRes code, AuthRecs never get appended to a non-empty vector while SigRecs do
+  // get appended in some cases; the handleHit() code will take measures. In the futrue we might
+  // want a more specialized datastructure than a vector, it would require another level of
+  // indirection though, so for now we construct a new shared vector if appending is needed. See
+  // handleHit() for details.
   using AuthRecsVec = std::vector<DNSRecord>;
   using AuthRecs = std::shared_ptr<const AuthRecsVec>; // const to avoid modifying the vector, which would be bad for shared data
   const static AuthRecs s_emptyAuthRecs;
 
+  // Use same setup as AuthRecs.
   using SigRecsVec = std::vector<std::shared_ptr<const RRSIGRecordContent>>;
-  using SigRecs = std::shared_ptr<const SigRecsVec>;
+  using SigRecs = std::shared_ptr<const SigRecsVec>; // Also const as it is shared
   const static SigRecs s_emptySigRecs;
 
   [[nodiscard]] time_t get(time_t, const DNSName& qname, QType qtype, Flags flags, vector<DNSRecord>* res, const ComboAddress& who, const OptTag& routingTag = boost::none, SigRecs* signatures = nullptr, AuthRecs* authorityRecs = nullptr, bool* variable = nullptr, vState* state = nullptr, bool* wasAuth = nullptr, DNSName* fromAuthZone = nullptr, ComboAddress* fromAuthIP = nullptr);
diff --git a/pdns/recursordist/test-recursorcache_cc.cc b/pdns/recursordist/test-recursorcache_cc.cc
index b0c98e1432..3c4ebf0fad 100644
--- a/pdns/recursordist/test-recursorcache_cc.cc
+++ b/pdns/recursordist/test-recursorcache_cc.cc
@@ -1432,11 +1432,11 @@ struct NOPTest
   }
 };
 
-struct AuthRecordsTest
+struct RecordsSpeedTest
 {
   [[nodiscard]] static string getName()
   {
-    return "AuthRecordsTest";
+    return "RecordsSpeedTest";
   }
 
   void operator()() const
@@ -1456,6 +1456,9 @@ struct AuthRecordsTest
     time_t now = time(nullptr);
     time_t ttd = now + 30;
 
+    MemRecursorCache::SigRecsVec signatures;
+    signatures.emplace_back(std::dynamic_pointer_cast<RRSIGRecordContent>(RRSIGRecordContent::make("DNSKEY 8 0 172800 20241111000000 20241021000000 20326 . alCFgDZS+0l5zcpQ/7R+5OFeCrk9KGkNP2F9ynXIXG6QigPj/9qjm0xx ItRJUUim+SrJywAmLKe+48oTUeSRyDKVVg3LGDekLKcIVz0EBqTL2y44 usDlUlxqx5O0LQVHy4h/hm9+dCXFiSBWoV0LcAplV9OYWhxi+CxmxZU5 8vK6eVAde8E2JHdeDuy23WF5lxYEg1q7ehEt5EdRvZ7hZzfawEFR3Qv3 WMootO2eBAAneIe94daJP/i1iwQJ4p+bGVCZ4sJk+Pk9J7lwEQq6Ghkd SpLsRxArUhvoVgtnh0LkAV7TsajYk8K2JRt7wHNDbBV6+Vdq2bh7ZPGv LiGkIQ==")));
+
     DNSName power("powerdns.com.");
     DNSRecord dr0;
     ComboAddress dr0Content("192.0.2.40");
@@ -1477,7 +1480,7 @@ struct AuthRecordsTest
       DNSName a = DNSName("hello ") + DNSName(std::to_string(counter));
       BOOST_CHECK_EQUAL(DNSName(a.toString()), a);
 
-      MRC.replace(now, a, QType(QType::A), rset0, {}, authRecords, true, authZone, boost::none, boost::none, vState::Insecure, somebody, false, ttl_time);
+      MRC.replace(now, a, QType(QType::A), rset0, signatures, authRecords, true, authZone, boost::none, boost::none, vState::Insecure, somebody, false, ttl_time);
     }
 
     BOOST_CHECK_EQUAL(MRC.size(), expected);
@@ -1489,7 +1492,7 @@ struct AuthRecordsTest
       for (size_t counter = 0; counter < expected; ++counter) {
         std::vector<DNSRecord> retrieved;
         MemRecursorCache::AuthRecs authRecs;
-        std::vector<std::shared_ptr<const RRSIGRecordContent>> sigs;
+        MemRecursorCache::SigRecs sigs;
         bool variable = false;
         vState state = vState::Indeterminate;
         bool wasAuth = false;
@@ -1507,7 +1510,7 @@ struct AuthRecordsTest
 BOOST_AUTO_TEST_CASE(test_speed)
 {
   doRun(NOPTest());
-  doRun(AuthRecordsTest());
+  doRun(RecordsSpeedTest());
 }
 #endif