rec: Revert 'Keep the EDNS status of a server on FormErr with EDNS'

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index e95262e540b7b1e74e83a071574cd2fdeb907d9d..f5270aaba86a59b5a130886b3e6f64f6d107084d 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -652,8 +652,9 @@ BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled) {
   initSR(sr);
 
   /* in this test, the auth answers with FormErr to an EDNS-enabled
-     query, but the response does contain EDNS so we should not mark
-     it as EDNS ignorant or intolerant.
+     query, but the response does contain EDNS so we might consider
+     that the server knows a bit about EDNS. It turns out that we can't
+     because there are too many awful servers out there.
   */
   size_t queriesWithEDNS = 0;
   size_t queriesWithoutEDNS = 0;
@@ -669,11 +670,15 @@ BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled) {
       }
       usedServers.insert(ip);
 
-      if (type == QType::DNAME) {
-        setLWResult(res, RCode::FormErr);
+      if (type == QType::A) {
         if (EDNS0Level > 0) {
+          setLWResult(res, RCode::FormErr);
           res->d_haveEDNS = true;
         }
+        else {
+          setLWResult(res, 0, true, false, false);
+          addRecordToLW(res, domain, QType::A, "192.0.2.1");
+        }
         return 1;
       }
 
@@ -683,15 +688,15 @@ BOOST_AUTO_TEST_CASE(test_edns_formerr_but_edns_enabled) {
   primeHints();
 
   vector<DNSRecord> ret;
-  int res = sr->beginResolve(DNSName("powerdns.com."), QType(QType::DNAME), QClass::IN, ret);
-  BOOST_CHECK_EQUAL(res, RCode::ServFail);
-  BOOST_CHECK_EQUAL(ret.size(), 0);
-  BOOST_CHECK_EQUAL(queriesWithEDNS, 26);
-  BOOST_CHECK_EQUAL(queriesWithoutEDNS, 0);
-  BOOST_CHECK_EQUAL(SyncRes::getEDNSStatusesSize(), 26);
-  BOOST_CHECK_EQUAL(usedServers.size(), 26);
+  int res = sr->beginResolve(DNSName("powerdns.com."), QType(QType::A), QClass::IN, ret);
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(ret.size(), 1);
+  BOOST_CHECK_EQUAL(queriesWithEDNS, 1);
+  BOOST_CHECK_EQUAL(queriesWithoutEDNS, 1);
+  BOOST_CHECK_EQUAL(SyncRes::getEDNSStatusesSize(), 1);
+  BOOST_CHECK_EQUAL(usedServers.size(), 1);
   for (const auto& server : usedServers) {
-    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::EDNSOK);
+    BOOST_CHECK_EQUAL(SyncRes::getEDNSStatus(server), SyncRes::EDNSStatus::NOEDNS);
   }
 }
 

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index b33775f75cec50ebcf906de37bd7d7edc368272a..2d0e4000f9c47c8644ea3919756a5ac1161d4a8e 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -479,7 +479,12 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
       return ret;
     }
     else if(mode==EDNSStatus::UNKNOWN || mode==EDNSStatus::EDNSOK || mode == EDNSStatus::EDNSIGNORANT ) {
-      if(!res->d_haveEDNS && (res->d_rcode == RCode::FormErr || res->d_rcode == RCode::NotImp)) {
+      /* So, you might be tempted to treat the presence of EDNS in a response as meaning that the
+         server does understand EDNS, and thus prevent a downgrade to no EDNS.
+         It turns out that you can't because there are a lot of crappy servers out there,
+         so you have to treat a FormErr as 'I have no idea what this EDNS thing is' no matter what.
+      */
+      if(res->d_rcode == RCode::FormErr || res->d_rcode == RCode::NotImp) {
        //      cerr<<"Downgrading to NOEDNS because of "<<RCode::to_s(res->d_rcode)<<" for query to "<<ip.toString()<<" for '"<<domain<<"'"<<endl;
         mode = EDNSStatus::NOEDNS;
         continue;