rec: Don't copy entries when retrieving from the negative cache

diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index 13fbf619fe6026bc32b2aae50359633e63c38a7f..6552486b7f11a073885f13f65e335e0d78e2c58d 100644 (file)
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -35,7 +35,7 @@
  * \param ne       A NegCacheEntry that is filled when there is a cache entry
  * \return         true if ne was filled out, false otherwise
  */
-bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne) {
+bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne) {
   // Never deny the root.
   if (qname.isRoot())
     return false;
@@ -51,7 +51,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
          ni->d_qtype == qtnull) {
     // We have something
     if ((uint32_t)now.tv_sec < ni->d_ttd) {
-      ne = *ni;
+      *ne = &(*ni);
       moveCacheItemToBack(d_negcache, ni);
       return true;
     }
@@ -70,7 +70,7 @@ bool NegCache::getRootNXTrust(const DNSName& qname, const struct timeval& now, N
  * \param ne       A NegCacheEntry that is filled when there is a cache entry
  * \return         true if ne was filled out, false otherwise
  */
-bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch) {
+bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch) {
   const auto& idx = d_negcache.get<2>();
   auto range = idx.equal_range(qname);
   auto ni = range.first;
@@ -83,7 +83,7 @@ bool NegCache::get(const DNSName& qname, const QType& qtype, const struct timeva
 
       if((uint32_t) now.tv_sec < ni->d_ttd) {
         // Not expired
-        ne = *ni;
+        *ne = &(*ni);
         moveCacheItemToBack(d_negcache, firstIndexIterator);
         return true;
       }

diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 2bc41da620796d3ec2f0d8a924a91cd4146eb178..b78951e21d68fd2af5b3748eaaacbcc477442905 100644 (file)
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -60,8 +60,8 @@ class NegCache : public boost::noncopyable {
 
     void add(const NegCacheEntry& ne);
     void updateValidationStatus(const DNSName& qname, const QType& qtype, const vState newState);
-    bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, NegCacheEntry& ne, bool typeMustMatch=false);
-    bool getRootNXTrust(const DNSName& qname, const struct timeval& now, NegCacheEntry& ne);
+    bool get(const DNSName& qname, const QType& qtype, const struct timeval& now, const NegCacheEntry** ne, bool typeMustMatch=false);
+    bool getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne);
     uint64_t count(const DNSName& qname) const;
     uint64_t count(const DNSName& qname, const QType qtype) const;
     void prune(unsigned int maxEntries);

diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc
index 77418c9a50a2bb0f73e24dcc5bd2607a7b62bbdd..aba53fa26368f83b84c94552362127c8cd4d1d3c 100644 (file)
--- a/pdns/recursordist/test-negcache_cc.cc
+++ b/pdns/recursordist/test-negcache_cc.cc
@@ -57,13 +57,13 @@ BOOST_AUTO_TEST_CASE(test_get_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.get(qname, QType(1), now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.get(qname, QType(1), now, &ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne.d_name, qname);
-  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(0).getName());
-  BOOST_CHECK_EQUAL(ne.d_auth, auth);
+  BOOST_CHECK_EQUAL(ne->d_name, qname);
+  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(0).getName());
+  BOOST_CHECK_EQUAL(ne->d_auth, auth);
 }
 
 BOOST_AUTO_TEST_CASE(test_get_entry_exact_type) {
@@ -81,10 +81,11 @@ BOOST_AUTO_TEST_CASE(test_get_entry_exact_type) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.get(qname, QType(1), now, ne, true);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.get(qname, QType(1), now, &ne, true);
 
   BOOST_CHECK_EQUAL(ret, false);
+  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_get_NODATA_entry) {
@@ -99,17 +100,18 @@ BOOST_AUTO_TEST_CASE(test_get_NODATA_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.get(qname, QType(1), now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.get(qname, QType(1), now, &ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne.d_name, qname);
-  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(1).getName());
-  BOOST_CHECK_EQUAL(ne.d_auth, auth);
+  BOOST_CHECK_EQUAL(ne->d_name, qname);
+  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(1).getName());
+  BOOST_CHECK_EQUAL(ne->d_auth, auth);
 
-  NegCache::NegCacheEntry ne2;
-  ret = cache.get(qname, QType(16), now, ne2);
+  const NegCache::NegCacheEntry* ne2 = nullptr;
+  ret = cache.get(qname, QType(16), now, &ne2);
   BOOST_CHECK_EQUAL(ret, false);
+  BOOST_CHECK(ne2 == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_entry) {
@@ -124,13 +126,13 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.getRootNXTrust(qname, now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.getRootNXTrust(qname, now, &ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne.d_name, qname);
-  BOOST_CHECK_EQUAL(ne.d_qtype.getName(), QType(0).getName());
-  BOOST_CHECK_EQUAL(ne.d_auth, auth);
+  BOOST_CHECK_EQUAL(ne->d_name, qname);
+  BOOST_CHECK_EQUAL(ne->d_qtype.getName(), QType(0).getName());
+  BOOST_CHECK_EQUAL(ne->d_auth, auth);
 }
 
 BOOST_AUTO_TEST_CASE(test_add_and_get_expired_entry) {
@@ -146,15 +148,13 @@ BOOST_AUTO_TEST_CASE(test_add_and_get_expired_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
 
   now.tv_sec += 1000;
-  bool ret = cache.get(qname, QType(1), now, ne);
+  bool ret = cache.get(qname, QType(1), now, &ne);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK_EQUAL(ne.d_name, DNSName());
-  BOOST_CHECK_EQUAL(ne.d_auth, DNSName());
-  BOOST_CHECK(ne.authoritySOA.records.empty());
+  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_expired_entry) {
@@ -170,15 +170,13 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_expired_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
 
   now.tv_sec += 1000;
-  bool ret = cache.getRootNXTrust(qname, now, ne);
+  bool ret = cache.getRootNXTrust(qname, now, &ne);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK_EQUAL(ne.d_name, DNSName());
-  BOOST_CHECK_EQUAL(ne.d_auth, DNSName());
-  BOOST_CHECK(ne.authoritySOA.records.empty());
+  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_add_updated_entry) {
@@ -196,12 +194,12 @@ BOOST_AUTO_TEST_CASE(test_add_updated_entry) {
 
   BOOST_CHECK_EQUAL(cache.size(), 1);
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.get(qname, QType(1), now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.get(qname, QType(1), now, &ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne.d_name, qname);
-  BOOST_CHECK_EQUAL(ne.d_auth, auth2);
+  BOOST_CHECK_EQUAL(ne->d_name, qname);
+  BOOST_CHECK_EQUAL(ne->d_auth, auth2);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust) {
@@ -217,12 +215,12 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust) {
   cache.add(genNegCacheEntry(qname, auth, now));
   cache.add(genNegCacheEntry(qname2, auth2, now));
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.getRootNXTrust(qname, now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.getRootNXTrust(qname, now, &ne);
 
   BOOST_CHECK(ret);
-  BOOST_CHECK_EQUAL(ne.d_name, qname2);
-  BOOST_CHECK_EQUAL(ne.d_auth, auth2);
+  BOOST_CHECK_EQUAL(ne->d_name, qname2);
+  BOOST_CHECK_EQUAL(ne->d_auth, auth2);
 }
 
 BOOST_AUTO_TEST_CASE(test_getRootNXTrust_full_domain_only) {
@@ -238,10 +236,11 @@ BOOST_AUTO_TEST_CASE(test_getRootNXTrust_full_domain_only) {
   cache.add(genNegCacheEntry(qname, auth, now));
   cache.add(genNegCacheEntry(qname2, auth2, now, 1)); // Add the denial for COM|A
 
-  NegCache::NegCacheEntry ne;
-  bool ret = cache.getRootNXTrust(qname, now, ne);
+  const NegCache::NegCacheEntry* ne = nullptr;
+  bool ret = cache.getRootNXTrust(qname, now, &ne);
 
   BOOST_CHECK_EQUAL(ret, false);
+  BOOST_CHECK(ne == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_prune) {
@@ -288,22 +287,20 @@ BOOST_AUTO_TEST_CASE(test_wipe_single) {
   cache.wipe(auth);
   BOOST_CHECK_EQUAL(cache.size(), 400);
 
-  NegCache::NegCacheEntry ne2;
-  bool ret = cache.get(auth, QType(1), now, ne2);
+  const NegCache::NegCacheEntry* ne2 = nullptr;
+  bool ret = cache.get(auth, QType(1), now, &ne2);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK_EQUAL(ne2.d_auth, DNSName());
-  BOOST_CHECK_EQUAL(ne2.d_name, DNSName());
+  BOOST_CHECK(ne2 == nullptr);
 
   cache.wipe(DNSName("1.powerdns.com"));
   BOOST_CHECK_EQUAL(cache.size(), 399);
 
-  NegCache::NegCacheEntry ne3;
-  ret = cache.get(auth, QType(1), now, ne3);
+  const NegCache::NegCacheEntry* ne3 = nullptr;
+  ret = cache.get(auth, QType(1), now, &ne3);
 
   BOOST_CHECK_EQUAL(ret, false);
-  BOOST_CHECK_EQUAL(ne3.d_auth, DNSName());
-  BOOST_CHECK_EQUAL(ne3.d_name, DNSName());
+  BOOST_CHECK(ne3 == nullptr);
 }
 
 BOOST_AUTO_TEST_CASE(test_wipe_subtree) {
@@ -369,8 +366,7 @@ BOOST_AUTO_TEST_CASE(test_dumpToFile) {
   cache.add(genNegCacheEntry(DNSName("www1.powerdns.com"), DNSName("powerdns.com"), now));
   cache.add(genNegCacheEntry(DNSName("www2.powerdns.com"), DNSName("powerdns.com"), now));
 
-  FILE* fp;
-  fp = tmpfile();
+  FILE* fp = tmpfile();
   if (!fp)
     BOOST_FAIL("Temporary file could not be opened");
 

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 3a157279e29b19e6c0832c32e72207668f6d3b93..a9293f25204240dcb3b5c2b9858095d8171e36dd 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -9003,15 +9003,15 @@ BOOST_AUTO_TEST_CASE(test_dnssec_rrsig_negcache_validity) {
   BOOST_CHECK_EQUAL(queriesCount, 4);
 
   /* check that the entry has not been negatively cached for longer than the RRSIG validity */
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_ttd, now + 1);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_ttd, now + 1);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Secure);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1);
 
   /* again, to test the cache */
   ret.clear();
@@ -9639,14 +9639,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) {
   BOOST_REQUIRE_EQUAL(ret.size(), 4);
   BOOST_CHECK_EQUAL(queriesCount, 1);
   /* check that the entry has not been negatively cached */
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -9657,12 +9657,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure) {
   BOOST_REQUIRE_EQUAL(ret.size(), 4);
   BOOST_CHECK_EQUAL(queriesCount, 4);
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Secure);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 1);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Secure);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 1);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_secure_ds) {
@@ -9775,14 +9775,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   BOOST_CHECK_EQUAL(queriesCount, 1);
   /* check that the entry has not been negatively cached */
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -9792,12 +9792,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_insecure) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Insecure);
   BOOST_REQUIRE_EQUAL(ret.size(), 1);
   BOOST_CHECK_EQUAL(queriesCount, 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Insecure);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Insecure);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
@@ -9852,14 +9852,14 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Indeterminate);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
   BOOST_CHECK_EQUAL(queriesCount, 1);
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
   BOOST_CHECK_EQUAL(SyncRes::t_sstorage.negcache.size(), 1);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Indeterminate);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Indeterminate);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 
   ret.clear();
   /* second one _does_ require validation */
@@ -9869,12 +9869,12 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_from_negcache_bogus) {
   BOOST_CHECK_EQUAL(sr->getValidationState(), Bogus);
   BOOST_REQUIRE_EQUAL(ret.size(), 2);
   BOOST_CHECK_EQUAL(queriesCount, 4);
-  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), ne), true);
-  BOOST_CHECK_EQUAL(ne.d_validationState, Bogus);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.records.size(), 1);
-  BOOST_CHECK_EQUAL(ne.authoritySOA.signatures.size(), 1);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.records.size(), 0);
-  BOOST_CHECK_EQUAL(ne.DNSSECRecords.signatures.size(), 0);
+  BOOST_REQUIRE_EQUAL(SyncRes::t_sstorage.negcache.get(target, QType(QType::A), sr->getNow(), &ne), true);
+  BOOST_CHECK_EQUAL(ne->d_validationState, Bogus);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.records.size(), 1);
+  BOOST_CHECK_EQUAL(ne->authoritySOA.signatures.size(), 1);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.records.size(), 0);
+  BOOST_CHECK_EQUAL(ne->DNSSECRecords.signatures.size(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(test_lowercase_outgoing) {

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index b4d9824158e0aa2952890bc77ea72145779528b2..431b5be5135d2b7740717b57a3cddcce4b1221c9 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1024,7 +1024,7 @@ static void addTTLModifiedRecords(const vector<DNSRecord>& records, const uint32
   }
 }
 
-void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const DNSName& qname, const QType& qtype, const int res, vState& state, unsigned int depth)
+void SyncRes::computeNegCacheValidationStatus(const NegCache::NegCacheEntry* ne, const DNSName& qname, const QType& qtype, const int res, vState& state, unsigned int depth)
 {
   DNSName subdomain(qname);
   /* if we are retrieving a DS, we only care about the state of the parent zone */
@@ -1034,10 +1034,10 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const
   computeZoneCuts(subdomain, g_rootdnsname, depth);
 
   tcache_t tcache;
-  reapRecordsFromNegCacheEntryForValidation(tcache, ne.authoritySOA.records);
-  reapRecordsFromNegCacheEntryForValidation(tcache, ne.authoritySOA.signatures);
-  reapRecordsFromNegCacheEntryForValidation(tcache, ne.DNSSECRecords.records);
-  reapRecordsFromNegCacheEntryForValidation(tcache, ne.DNSSECRecords.signatures);
+  reapRecordsFromNegCacheEntryForValidation(tcache, ne->authoritySOA.records);
+  reapRecordsFromNegCacheEntryForValidation(tcache, ne->authoritySOA.signatures);
+  reapRecordsFromNegCacheEntryForValidation(tcache, ne->DNSSECRecords.records);
+  reapRecordsFromNegCacheEntryForValidation(tcache, ne->DNSSECRecords.signatures);
 
   for (const auto& entry : tcache) {
     // this happens when we did store signatures, but passed on the records themselves
@@ -1065,13 +1065,14 @@ void SyncRes::computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const
   }
 
   if (state == Secure) {
+    vState neValidationState = ne->d_validationState;
     dState expectedState = res == RCode::NXDomain ? NXDOMAIN : NXQTYPE;
-    dState denialState = getDenialValidationState(ne, state, expectedState, false);
-    updateDenialValidationState(ne, state, denialState, expectedState, qtype == QType::DS);
+    dState denialState = getDenialValidationState(*ne, state, expectedState, false);
+    updateDenialValidationState(neValidationState, ne->d_name, state, denialState, expectedState, qtype == QType::DS);
   }
   if (state != Indeterminate) {
     /* validation succeeded, let's update the cache entry so we don't have to validate again */
-    t_sstorage.negcache.updateValidationStatus(ne.d_name, ne.d_qtype, state);
+    t_sstorage.negcache.updateValidationStatus(ne->d_name, ne->d_qtype, state);
   }
 }
 
@@ -1091,36 +1092,36 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
   uint32_t sttl=0;
   //  cout<<"Lookup for '"<<qname<<"|"<<qtype.getName()<<"' -> "<<getLastLabel(qname)<<endl;
   vState cachedState;
-  NegCache::NegCacheEntry ne;
+  const NegCache::NegCacheEntry* ne = nullptr;
 
   if(s_rootNXTrust &&
-     t_sstorage.negcache.getRootNXTrust(qname, d_now, ne) &&
-      ne.d_auth.isRoot() &&
+     t_sstorage.negcache.getRootNXTrust(qname, d_now, &ne) &&
+      ne->d_auth.isRoot() &&
       !(wasForwardedOrAuthZone && !authname.isRoot())) { // when forwarding, the root may only neg-cache if it was forwarded to.
-    sttl = ne.d_ttd - d_now.tv_sec;
-    LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne.d_auth<<"' & '"<<ne.d_name<<"' for another "<<sttl<<" seconds"<<endl);
+    sttl = ne->d_ttd - d_now.tv_sec;
+    LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne->d_auth<<"' & '"<<ne->d_name<<"' for another "<<sttl<<" seconds"<<endl);
     res = RCode::NXDomain;
     giveNegative = true;
-    cachedState = ne.d_validationState;
+    cachedState = ne->d_validationState;
   }
-  else if (t_sstorage.negcache.get(qname, qtype, d_now, ne) &&
-           !(wasForwardedOrAuthZone && ne.d_auth != authname)) { // Only the authname nameserver can neg cache entries
+  else if (t_sstorage.negcache.get(qname, qtype, d_now, &ne) &&
+           !(wasForwardedOrAuthZone && ne->d_auth != authname)) { // Only the authname nameserver can neg cache entries
 
     /* If we are looking for a DS, discard NXD if auth == qname
        and ask for a specific denial instead */
-    if (qtype != QType::DS || ne.d_qtype.getCode() || ne.d_auth != qname ||
-        t_sstorage.negcache.get(qname, qtype, d_now, ne, true))
+    if (qtype != QType::DS || ne->d_qtype.getCode() || ne->d_auth != qname ||
+        t_sstorage.negcache.get(qname, qtype, d_now, &ne, true))
     {
       res = 0;
-      sttl = ne.d_ttd - d_now.tv_sec;
+      sttl = ne->d_ttd - d_now.tv_sec;
       giveNegative = true;
-      cachedState = ne.d_validationState;
-      if(ne.d_qtype.getCode()) {
-        LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+      cachedState = ne->d_validationState;
+      if(ne->d_qtype.getCode()) {
+        LOG(prefix<<qname<<": "<<qtype.getName()<<" is negatively cached via '"<<ne->d_auth<<"' for another "<<sttl<<" seconds"<<endl);
         res = RCode::NoError;
       }
       else {
-        LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne.d_auth<<"' for another "<<sttl<<" seconds"<<endl);
+        LOG(prefix<<qname<<": Entire name '"<<qname<<"', is negatively cached via '"<<ne->d_auth<<"' for another "<<sttl<<" seconds"<<endl);
         res = RCode::NXDomain;
       }
     }
@@ -1136,11 +1137,11 @@ bool SyncRes::doCacheCheck(const DNSName &qname, const DNSName& authname, bool w
     }
 
     // Transplant SOA to the returned packet
-    addTTLModifiedRecords(ne.authoritySOA.records, sttl, ret);
+    addTTLModifiedRecords(ne->authoritySOA.records, sttl, ret);
     if(d_doDNSSEC) {
-      addTTLModifiedRecords(ne.authoritySOA.signatures, sttl, ret);
-      addTTLModifiedRecords(ne.DNSSECRecords.records, sttl, ret);
-      addTTLModifiedRecords(ne.DNSSECRecords.signatures, sttl, ret);
+      addTTLModifiedRecords(ne->authoritySOA.signatures, sttl, ret);
+      addTTLModifiedRecords(ne->DNSSECRecords.records, sttl, ret);
+      addTTLModifiedRecords(ne->DNSSECRecords.signatures, sttl, ret);
     }
 
     LOG(prefix<<qname<<": updating validation state with negative cache content for "<<qname<<" to "<<vStates[state]<<endl);
@@ -2163,30 +2164,30 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
   return RCode::NoError;
 }
 
-void SyncRes::updateDenialValidationState(NegCache::NegCacheEntry& ne, vState& state, const dState denialState, const dState expectedState, bool allowOptOut)
+void SyncRes::updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState, bool allowOptOut)
 {
   if (denialState == expectedState) {
-    ne.d_validationState = Secure;
+    neValidationState = Secure;
   }
   else {
     if (denialState == OPTOUT && allowOptOut) {
-      LOG(d_prefix<<"OPT-out denial found for "<<ne.d_name<<endl);
-      ne.d_validationState = Secure;
+      LOG(d_prefix<<"OPT-out denial found for "<<neName<<endl);
+      neValidationState = Secure;
       return;
     }
     else if (denialState == INSECURE) {
-      LOG(d_prefix<<"Insecure denial found for "<<ne.d_name<<", returning Insecure"<<endl);
-      ne.d_validationState = Insecure;
+      LOG(d_prefix<<"Insecure denial found for "<<neName<<", returning Insecure"<<endl);
+      neValidationState = Insecure;
     }
     else {
-      LOG(d_prefix<<"Invalid denial found for "<<ne.d_name<<", returning Bogus, res="<<denialState<<", expectedState="<<expectedState<<endl);
-      ne.d_validationState = Bogus;
+      LOG(d_prefix<<"Invalid denial found for "<<neName<<", returning Bogus, res="<<denialState<<", expectedState="<<expectedState<<endl);
+      neValidationState = Bogus;
     }
-    updateValidationState(state, ne.d_validationState);
+    updateValidationState(state, neValidationState);
   }
 }
 
-dState SyncRes::getDenialValidationState(NegCache::NegCacheEntry& ne, const vState state, const dState expectedState, bool referralToUnsigned)
+dState SyncRes::getDenialValidationState(const NegCache::NegCacheEntry& ne, const vState state, const dState expectedState, bool referralToUnsigned)
 {
   cspmap_t csp = harvestCSPFromNE(ne);
   return getDenial(csp, ne.d_name, ne.d_qtype.getCode(), referralToUnsigned, expectedState == NXQTYPE);
@@ -2221,7 +2222,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
 
       if (state == Secure) {
         dState denialState = getDenialValidationState(ne, state, NXDOMAIN, false);
-        updateDenialValidationState(ne, state, denialState, NXDOMAIN, false);
+        updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, NXDOMAIN, false);
       }
       else {
         ne.d_validationState = state;
@@ -2369,7 +2370,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
 
         if (state == Secure) {
           dState denialState = getDenialValidationState(ne, state, NXQTYPE, false);
-          updateDenialValidationState(ne, state, denialState, NXQTYPE, qtype == QType::DS);
+          updateDenialValidationState(ne.d_validationState, ne.d_name, state, denialState, NXQTYPE, qtype == QType::DS);
         } else {
           ne.d_validationState = state;
         }

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index b10b440761a73177ba733b89821f9f0f116c0a7c..aa8300d17b66bad390f3ec83906578cbc6337875 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -776,9 +776,9 @@ private:
   vState validateRecordsWithSigs(unsigned int depth, const DNSName& qname, const QType& qtype, const DNSName& name, const std::vector<DNSRecord>& records, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures);
   vState validateDNSKeys(const DNSName& zone, const std::vector<DNSRecord>& dnskeys, const std::vector<std::shared_ptr<RRSIGRecordContent> >& signatures, unsigned int depth);
   vState getDNSKeys(const DNSName& signer, skeyset_t& keys, unsigned int depth);
-  dState getDenialValidationState(NegCache::NegCacheEntry& ne, const vState state, const dState expectedState, bool referralToUnsigned);
-  void updateDenialValidationState(NegCache::NegCacheEntry& ne, vState& state, const dState denialState, const dState expectedState, bool allowOptOut);
-  void computeNegCacheValidationStatus(NegCache::NegCacheEntry& ne, const DNSName& qname, const QType& qtype, const int res, vState& state, unsigned int depth);
+  dState getDenialValidationState(const NegCache::NegCacheEntry& ne, const vState state, const dState expectedState, bool referralToUnsigned);
+  void updateDenialValidationState(vState& neValidationState, const DNSName& neName, vState& state, const dState denialState, const dState expectedState, bool allowOptOut);
+  void computeNegCacheValidationStatus(const NegCache::NegCacheEntry* ne, const DNSName& qname, const QType& qtype, const int res, vState& state, unsigned int depth);
   vState getTA(const DNSName& zone, dsmap_t& ds);
   bool haveExactValidationStatus(const DNSName& domain);
   vState getValidationStatus(const DNSName& subdomain, bool allowIndeterminate=true);