From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 10 Jun 2020 12:55:18 +0000 (+0200)
Subject: For the forwarder case, look in the cache first and only use forwarder
X-Git-Tag: rec-4.4.0-beta1~48^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3424d2fef36295a0210c927133802142166661fb;p=thirdparty%2Fpdns.git

For the forwarder case, look in the cache first and only use forwarder
if a cache entry is found that is less specific.
---

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index b84cafde99..885f8ac7ad 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1132,31 +1132,45 @@ DNSName SyncRes::getBestNSNamesFromCache(const DNSName &qname, const QType& qtyp
   DNSName authdomain(qname);
 
   domainmap_t::const_iterator iter=getBestAuthZone(&authdomain);
-  if(iter!=t_sstorage.domainmap->end()) {
-    if( iter->second.isAuth() )
+  // We have an auth, forwarder of forwarder-recurse
+  if (iter != t_sstorage.domainmap->end()) {
+    if (iter->second.isAuth()) {
       // this gets picked up in doResolveAt, the empty DNSName, combined with the
       // empty vector means 'we are auth for this zone'
       nsset.insert({DNSName(), {{}, false}});
+      return authdomain;
+    }
     else {
-      // Again, picked up in doResolveAt. An empty DNSName, combined with a
-      // non-empty vector of ComboAddresses means 'this is a forwarded domain'
-      // This is actually picked up in retrieveAddressesForNS called from doResolveAt.
-      nsset.insert({DNSName(), {iter->second.d_servers, iter->second.shouldRecurse() }});
+      if (iter->second.shouldRecurse()) {
+        // Again, picked up in doResolveAt. An empty DNSName, combined with a
+        // non-empty vector of ComboAddresses means 'this is a forwarded domain'
+        // This is actually picked up in retrieveAddressesForNS called from doResolveAt.
+        nsset.insert({DNSName(), {iter->second.d_servers, true }});
+        return authdomain;
+      }
     }
-    return authdomain;
   }
 
+  // We might have a (non-recursive) forwarder, but maybe the cache already contains
+  // a better NS
   DNSName subdomain(qname);
   vector<DNSRecord> bestns;
   getBestNSFromCache(subdomain, qtype, bestns, flawedNSSet, depth, beenthere);
 
-  for(auto k=bestns.cbegin() ; k != bestns.cend(); ++k) {
+  // If the forwarder is better or equal to what's found in the cache, use forwarder
+  if (iter != t_sstorage.domainmap->end() && authdomain.isPartOf(subdomain)) {
+    nsset.insert({DNSName(), {iter->second.d_servers, false }});
+    return authdomain;
+  }
+
+  for (auto k=bestns.cbegin(); k != bestns.cend(); ++k) {
     // The actual resolver code will not even look at the ComboAddress or bool
     const auto nsContent = getRR<NSRecordContent>(*k);
     if (nsContent) {
       nsset.insert({nsContent->getNS(), {{}, false}});
-      if(k==bestns.cbegin())
+      if (k == bestns.cbegin()) {
         subdomain=k->d_name;
+      }
     }
   }
   return subdomain;