From: Otto Date: Mon, 8 Mar 2021 10:51:40 +0000 (+0100) Subject: Add one more unit test and use the proper counter. X-Git-Tag: rec-4.5.0-alpha3^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F10152%2Fhead;p=thirdparty%2Fpdns.git Add one more unit test and use the proper counter. nretrieveAddressesForNS is only incremented on empty results, so the clearing did not work as expected. --- diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc index 8f2da0e7b9..34921b0ecd 100644 --- a/pdns/recursordist/test-syncres_cc2.cc +++ b/pdns/recursordist/test-syncres_cc2.cc @@ -233,6 +233,102 @@ BOOST_AUTO_TEST_CASE(test_glueless_referral_loop_with_nonresolving) BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 4U); } +BOOST_AUTO_TEST_CASE(test_glueless_referral_with_non_resolving) +{ + std::unique_ptr sr; + initSR(sr); + + primeHints(); + + const DNSName target("powerdns.com."); + + size_t queryCount = 0; + + sr->setAsyncCallback([target, &queryCount](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional& srcmask, boost::optional context, LWResult* res, bool* chained) { + if (isRootServer(ip)) { + setLWResult(res, 0, false, false, true); + + if (domain.isPartOf(DNSName("com."))) { + addRecordToLW(res, "com.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + } + else if (domain.isPartOf(DNSName("org."))) { + addRecordToLW(res, "org.", QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800); + } + else { + setLWResult(res, RCode::NXDomain, false, false, true); + return LWResult::Result::Success; + } + + addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600); + addRecordToLW(res, "a.gtld-servers.net.", QType::AAAA, "2001:DB8::1", DNSResourceRecord::ADDITIONAL, 3600); + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.1:53") || ip == ComboAddress("[2001:DB8::1]:53")) { + if (domain == target) { + setLWResult(res, 0, false, false, true); + addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns1.powerdns.org.", DNSResourceRecord::AUTHORITY, 172800); + addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns2.powerdns.org.", DNSResourceRecord::AUTHORITY, 172800); + return LWResult::Result::Success; + } + else if (domain == DNSName("pdns-public-ns1.powerdns.org.")) { + queryCount++; + setLWResult(res, 0, true, false, true); + if (queryCount > 8) { + addRecordToLW(res, "pdns-public-ns1.powerdns.org.", QType::A, "192.0.2.2"); + addRecordToLW(res, "pdns-public-ns1.powerdns.org.", QType::AAAA, "2001:DB8::2"); + } + return LWResult::Result::Success; + } + else if (domain == DNSName("pdns-public-ns2.powerdns.org.")) { + queryCount++; + setLWResult(res, 0, true, false, true); + if (queryCount > 8) { + addRecordToLW(res, "pdns-public-ns2.powerdns.org.", QType::A, "192.0.2.3"); + addRecordToLW(res, "pdns-public-ns2.powerdns.org.", QType::AAAA, "2001:DB8::3"); + } + return LWResult::Result::Success; + } + + setLWResult(res, RCode::NXDomain, false, false, true); + return LWResult::Result::Success; + } + else if (ip == ComboAddress("192.0.2.2:53") || ip == ComboAddress("192.0.2.3:53") || ip == ComboAddress("[2001:DB8::2]:53") || ip == ComboAddress("[2001:DB8::3]:53")) { + setLWResult(res, 0, true, false, true); + addRecordToLW(res, target, QType::A, "192.0.2.4"); + return LWResult::Result::Success; + } + else + return LWResult::Result::Timeout; + }); + + SyncRes::s_nonresolvingnsmaxfails = 10; + SyncRes::s_nonresolvingnsthrottletime = 60; + + vector ret; + int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::ServFail); + BOOST_REQUIRE_EQUAL(ret.size(), 0U); + BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 2U); + + // Again, should not change anything + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::ServFail); + BOOST_REQUIRE_EQUAL(ret.size(), 0U); + //BOOST_CHECK(ret[0].d_type == QType::A); + //BOOST_CHECK_EQUAL(ret[0].d_name, target); + + BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 2U); + + // Again, but now things should start working because of the queryCounter getting high enough + // and one entry remains in the non-resolving cache + res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); + BOOST_CHECK_EQUAL(res, RCode::NoError); + BOOST_REQUIRE_EQUAL(ret.size(), 1U); + BOOST_CHECK(ret[0].d_type == QType::A); + BOOST_CHECK_EQUAL(ret[0].d_name, target); + BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 1U); +} + BOOST_AUTO_TEST_CASE(test_cname_qperq) { std::unique_ptr sr; diff --git a/pdns/syncres.cc b/pdns/syncres.cc index d0d72c6e0b..5e4e5db830 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -2299,13 +2299,13 @@ vector SyncRes::retrieveAddressesForNS(const std::string& prefix, } LOG(prefix<first<< "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<first, depth, beenthere, cacheOnly, nretrieveAddressesForNS); } // Other exceptions should likely not throttle... catch (const ImmediateServFailException& ex) { - if (s_nonresolvingnsmaxfails > 0 && nretrieveAddressesForNS > oldnretrieveAddressesForNS) { + if (s_nonresolvingnsmaxfails > 0 && d_outqueries > oldOutQueries) { auto dontThrottleNames = g_dontThrottleNames.getLocal(); if (!dontThrottleNames->check(tns->first)) { t_sstorage.nonresolving.incr(tns->first, d_now); @@ -2313,7 +2313,7 @@ vector SyncRes::retrieveAddressesForNS(const std::string& prefix, } throw ex; } - if (s_nonresolvingnsmaxfails > 0 && nretrieveAddressesForNS > oldnretrieveAddressesForNS) { + if (s_nonresolvingnsmaxfails > 0 && d_outqueries > oldOutQueries) { if (result.empty()) { auto dontThrottleNames = g_dontThrottleNames.getLocal(); if (!dontThrottleNames->check(tns->first)) {