From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 8 Aug 2023 10:15:00 +0000 (+0200)
Subject: rec: replace data in the aggressive cache if it becomes available
X-Git-Tag: rec-4.9.1~1^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=63406ef9f8ebb0873bb7cb5baa5f8cbbb209c54f;p=thirdparty%2Fpdns.git

rec: replace data in the aggressive cache if it becomes available

Currently, new data does not get recorded into the aggressive cache
if there's an existing entry that matches. Together with the fact
that in some cases pruning can be unfair (it scans the zones
always in the same order and stops clearing when it has reached the
goal) and/or not very active (when the recursor is lighlty loaded)
this has the consequence that old expired records can remain in
the cache that prevent new data to be recorded and used.

(cherry picked from commit 93b25e9613f252bc1798975dc1f7a475400f2996)
---

diff --git a/pdns/recursordist/aggressive_nsec.cc b/pdns/recursordist/aggressive_nsec.cc
index 96c1c20359..e17bf2dd3a 100644
--- a/pdns/recursordist/aggressive_nsec.cc
+++ b/pdns/recursordist/aggressive_nsec.cc
@@ -344,16 +344,22 @@ void AggressiveNSECCache::insertNSEC(const DNSName& zone, const DNSName& owner,
     /* the TTL is already a TTD by now */
     if (!nsec3 && isWildcardExpanded(owner.countLabels(), *signatures.at(0))) {
       DNSName realOwner = getNSECOwnerName(owner, signatures);
-      auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, std::move(realOwner), std::move(next), record.d_ttl});
+      auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, realOwner, next, record.d_ttl});
       if (pair.second) {
         ++d_entriesCount;
       }
+      else {
+        zoneEntry->d_entries.replace(pair.first, {record.getContent(), signatures, realOwner, next, record.d_ttl});
+      }
     }
     else {
-      auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, owner, std::move(next), record.d_ttl});
+      auto pair = zoneEntry->d_entries.insert({record.getContent(), signatures, owner, next, record.d_ttl});
       if (pair.second) {
         ++d_entriesCount;
       }
+      else {
+        zoneEntry->d_entries.replace(pair.first, {record.getContent(), signatures, owner, next, record.d_ttl});
+      }
     }
   }
 }
diff --git a/pdns/recursordist/test-aggressive_nsec_cc.cc b/pdns/recursordist/test-aggressive_nsec_cc.cc
index f5424e608a..1596ba3332 100644
--- a/pdns/recursordist/test-aggressive_nsec_cc.cc
+++ b/pdns/recursordist/test-aggressive_nsec_cc.cc
@@ -1234,6 +1234,36 @@ BOOST_AUTO_TEST_CASE(test_aggressive_nsec_dump)
     BOOST_CHECK_EQUAL(line, str);
   }
 
+  expected.clear();
+  expected.push_back("; Zone powerdns.com.\n");
+  expected.push_back("www.powerdns.com. 10 IN NSEC z.powerdns.com. A RRSIG NSEC\n");
+  expected.push_back("- RRSIG NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data\n");
+  expected.push_back("z.powerdns.com. 30 IN NSEC zz.powerdns.com. AAAA RRSIG NSEC\n");
+  expected.push_back("- RRSIG NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data\n");
+  expected.push_back("; Zone powerdns.org.\n");
+  expected.push_back("www.powerdns.org. 10 IN NSEC3 1 0 50 ab HASG==== A RRSIG NSEC3\n");
+  expected.push_back("- RRSIG NSEC3 5 3 10 20370101000000 20370101000000 24567 dummy. data\n");
+
+  rec.d_name = DNSName("z.powerdns.com");
+  rec.d_type = QType::NSEC;
+  rec.d_ttl = now.tv_sec + 30;
+  rec.setContent(getRecordContent(QType::NSEC, "zz.powerdns.com. AAAA RRSIG NSEC"));
+  rrsig = std::make_shared<RRSIGRecordContent>("NSEC 5 3 10 20370101000000 20370101000000 24567 dummy. data");
+  cache->insertNSEC(DNSName("powerdns.com"), rec.d_name, rec, {rrsig}, false);
+
+  fp = std::unique_ptr<FILE, int (*)(FILE*)>(tmpfile(), fclose);
+  BOOST_CHECK_EQUAL(cache->dumpToFile(fp, now), 3U);
+
+  rewind(fp.get());
+
+  for (auto str : expected) {
+    read = getline(&line, &len, fp.get());
+    if (read == -1) {
+      BOOST_FAIL("Unable to read a line from the temp file");
+    }
+    BOOST_CHECK_EQUAL(line, str);
+  }
+
   /* getline() allocates a buffer when called with a nullptr,
      then reallocates it when needed, but we need to free the
      last allocation if any. */