]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Do cache negcache results, even when wasVariable() is true 11010/head
authorOtto <otto.moerbeek@open-xchange.com>
Fri, 19 Nov 2021 10:57:28 +0000 (11:57 +0100)
committerOtto <otto.moerbeek@open-xchange.com>
Fri, 19 Nov 2021 10:57:28 +0000 (11:57 +0100)
See https://datatracker.ietf.org/doc/html/rfc7871#section-7.4
Fixes #10994

pdns/recursordist/test-syncres_cc2.cc
pdns/syncres.cc

index 03d52c1071896b9a798206ba111284aa18420c38..8e46deb05fd6c6d4c1b36953f0886710fbe8c6f2 100644 (file)
@@ -1107,7 +1107,7 @@ BOOST_AUTO_TEST_CASE(test_rfc8020_nodata_bis)
   BOOST_CHECK_EQUAL(g_negCache->size(), 2U);
 }
 
-BOOST_AUTO_TEST_CASE(test_skip_negcache_for_variable_response)
+BOOST_AUTO_TEST_CASE(test_dont_skip_negcache_for_variable_response)
 {
   std::unique_ptr<SyncRes> sr;
   initSR(sr);
@@ -1160,8 +1160,7 @@ BOOST_AUTO_TEST_CASE(test_skip_negcache_for_variable_response)
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NXDomain);
   BOOST_CHECK_EQUAL(ret.size(), 2U);
-  /* no negative cache entry because the response was variable */
-  BOOST_CHECK_EQUAL(g_negCache->size(), 0U);
+  BOOST_CHECK_EQUAL(g_negCache->size(), 1U);
 }
 
 BOOST_AUTO_TEST_CASE(test_ecs_cache_limit_allowed)
index 2740d452f417a38c3233e84aa974bbb323e17f33..92344dd9314b59a116e162310a7ec24b0afc0b24 100644 (file)
@@ -3637,7 +3637,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
          and do an additional query for the CNAME target.
          We have a regression test making sure we do exactly that.
       */
-      if (!wasVariable() && newtarget.empty() && putInNegCache) {
+      if (newtarget.empty() && putInNegCache) {
         g_negCache->add(ne);
         if (s_rootNXTrust && ne.d_auth.isRoot() && auth.isRoot() && lwr.d_aabit) {
           ne.d_name = ne.d_name.getLastLabel();
@@ -3811,9 +3811,7 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
             }
             LOG(prefix<<qname<<": got negative indication of DS record for '"<<newauth<<"'"<<endl);
 
-            if (!wasVariable()) {
-              g_negCache->add(ne);
-            }
+            g_negCache->add(ne);
 
             /* Careful! If the client is asking for a DS that does not exist, we need to provide the SOA along with the NSEC(3) proof
                and we might not have it if we picked up the proof from a delegation, in which case we need to keep on to do the actual DS
@@ -3865,10 +3863,8 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co
         }
         ne.d_ttd = d_now.tv_sec + lowestTTL;
 
-        if (!wasVariable()) {
-          if (qtype.getCode()) {  // prevents us from NXDOMAIN'ing a whole domain
-            g_negCache->add(ne);
-          }
+        if (qtype.getCode()) {  // prevents us from NXDOMAIN'ing a whole domain
+          g_negCache->add(ne);
         }
 
         ret.push_back(rec);