If we encounter a loop in QM, continue with the next iteration.

We would not make progres anyway, and end up repeatedly asking the full name
from the roots as the "Trying less specific NS" code path is hit.

Noted by and fix from  @phonedph1

Fixes #12090

As this is an optimzation for broken domains, this is not an immediate candidate
for backporting, though it should be considered for that after more testing.

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 443b94fd63e9d0056dc602ab8fe918cb55af6a19..5dc97741cebe145a7a4aaa4a8da12251abd10eb7 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -1737,6 +1737,19 @@ int SyncRes::doResolve(const DNSName &qname, const QType qtype, vector<DNSRecord
         return res;
       }
 
+      // If we have seen this child during resolution already; just skip it. We tried to QM it already or otherwise broken.
+      bool skipStep4 = false;
+      for (const auto& visitedNS : beenthere) {
+        if (visitedNS.qname == child) {
+          skipStep4 = true;
+          break;
+        }
+      }
+      if (skipStep4) {
+        QLOG("Step4 Being skipped as visited this child name already");
+        continue;
+      }
+
       // Step 4
       QLOG("Step4 Resolve A for child");
       bool oldFollowCNAME = d_followCNAME;