From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 19 Nov 2024 11:33:29 +0000 (+0100)
Subject: Dedup only in specific places
X-Git-Tag: dnsdist-2.0.0-alpha1~182^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5962f66f00e60730bc06b2d540685f6521fb057;p=thirdparty%2Fpdns.git

Dedup only in specific places
---

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 4055aafa73..8b2a049dec 100644
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -779,6 +779,9 @@ int getFakeAAAARecords(const DNSName& qname, ComboAddress prefix, vector<DNSReco
                 }),
               ret.end());
   }
+  else {
+    pdns::dedupRecords(ret);
+  }
   t_Counters.at(rec::Counter::dns64prefixanswers)++;
   return rcode;
 }
@@ -1509,7 +1512,9 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi
       }
 
       if (!ret.empty()) {
+#ifdef notyet
         pdns::dedupRecords(ret);
+#endif
         pdns::orderAndShuffle(ret, false);
         if (auto listToSort = luaconfsLocal->sortlist.getOrderCmp(comboWriter->d_source)) {
           stable_sort(ret.begin(), ret.end(), *listToSort);
diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 9202e3a103..b8d951aeb5 100644
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -2727,6 +2727,7 @@ bool SyncRes::doCNAMECacheCheck(const DNSName& qname, const QType qtype, vector<
       // so you can't trust that a real lookup will have been made.
       res = doResolve(newTarget, qtype, ret, depth + 1, beenthere, cnameContext);
       LOG(prefix << qname << ": Updating validation state for response to " << qname << " from " << context.state << " with the state from the DNAME/CNAME quest: " << cnameContext.state << endl);
+      pdns::dedupRecords(ret); // multiple NSECS could have been added, #14120
       updateValidationState(qname, context.state, cnameContext.state, prefix);
 
       return true;
@@ -4446,9 +4447,11 @@ void SyncRes::sanitizeRecordsPass2(const std::string& prefix, LWResult& lwr, con
     }
     lwr.d_records = std::move(vec);
   }
+#ifdef notyet
   if (auto count = pdns::dedupRecords(lwr.d_records); count > 0) {
     LOG(prefix << qname << ": Removed " << count << " duplicate records from response received from " << auth << endl);
   }
+#endif
 }
 
 void SyncRes::rememberParentSetIfNeeded(const DNSName& domain, const vector<DNSRecord>& newRecords, unsigned int depth, const string& prefix)
diff --git a/pdns/recursordist/test-syncres_cc5.cc b/pdns/recursordist/test-syncres_cc5.cc
index e0eb6a4d1a..a1e873e02e 100644
--- a/pdns/recursordist/test-syncres_cc5.cc
+++ b/pdns/recursordist/test-syncres_cc5.cc
@@ -1575,6 +1575,7 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_nodata_nowildcard_duplicated_n
         addRecordToLW(res, "a.gtld-servers.com.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
         return LWResult::Result::Success;
       }
+      // The code below introduces duplicate NSEC3 records
       if (address == ComboAddress("192.0.2.1:53")) {
         setLWResult(res, 0, true, false, true);
         /* no data */
@@ -1603,8 +1604,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_nodata_nowildcard_duplicated_n
   int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure);
-  /* the duplicated NSEC3 should have been dedupped */
-  BOOST_REQUIRE_EQUAL(ret.size(), 8U);
+  /* the duplicated NSEC3 have not been dedupped */
+  BOOST_REQUIRE_EQUAL(ret.size(), 9U);
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 
   /* again, to test the cache */
@@ -1612,8 +1613,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_validation_nsec3_nodata_nowildcard_duplicated_n
   res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
   BOOST_CHECK_EQUAL(res, RCode::NoError);
   BOOST_CHECK_EQUAL(sr->getValidationState(), vState::Secure);
-  /* the duplicated NSEC3 should have been dedupped */
-  BOOST_REQUIRE_EQUAL(ret.size(), 8U);
+  /* the duplicated NSEC3 have not been dedupped */
+  BOOST_REQUIRE_EQUAL(ret.size(), 9U);
   BOOST_CHECK_EQUAL(queriesCount, 4U);
 }