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 b84cafde99ae9a2fd992964221f61541950c3d1b..885f8ac7ad4ac91185888d76eef4260f22332829 100644 (file)
--- 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;