From 04ca3ad8f4f8b34b3425f4146cf28c89743eb697 Mon Sep 17 00:00:00 2001
From: Torsten Dreyer <torsten@t3r.de>
Date: Wed, 3 May 2017 10:31:44 +0200
Subject: [PATCH] Fix bug "dns timeout, no terrasync servers found"

The existance of a dns entry with a protocol defined caused the
naptr callback being terminated early and the request never completed.
This patch also adds a unit test for this particular case.
test_dns now also accepts up to two command line parameters:
first: dns DN to query for NAPTR records (default: terrasync.flightgear.org)
second: service to query (default: empty)
---
 simgear/io/DNSClient.cxx |  4 ++--
 simgear/io/test_DNS.cxx  | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/simgear/io/DNSClient.cxx b/simgear/io/DNSClient.cxx
index 3332dac5..07547bc3 100644
--- a/simgear/io/DNSClient.cxx
+++ b/simgear/io/DNSClient.cxx
@@ -191,11 +191,11 @@ static void dnscbNAPTR(struct dns_ctx *ctx, struct dns_rr_naptr *result, void *d
         r->ttl = result->dnsnaptr_ttl;
         for (int i = 0; i < result->dnsnaptr_nrr; i++) {
             if( !r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service )
-                return;
+                continue;
 
             //TODO: case ignore and result flags may have more than one flag
             if( !r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags )
-                return;
+                continue;
 
             NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR);
             r->entries.push_back(naptr);
diff --git a/simgear/io/test_DNS.cxx b/simgear/io/test_DNS.cxx
index c3b032f6..db99ae16 100644
--- a/simgear/io/test_DNS.cxx
+++ b/simgear/io/test_DNS.cxx
@@ -78,6 +78,9 @@ int main(int argc, char* argv[])
 {
     sglog().setLogLevels( SG_ALL, SG_DEBUG );
 
+    const char * EXISTING_RECORD = argc > 1 ? argv[1] : "terrasync.flightgear.org";
+    const char * QSERVICE = argc > 2 ? argv[2] : "https+ws20";
+
     Watchdog watchdog;
     watchdog.start(100);
 
@@ -98,8 +101,7 @@ int main(int argc, char* argv[])
         cout << "done" << endl;
     }
 
-#define EXISTING_RECORD "terrasync.flightgear.org"
-    cout << "test existing NAPTR: " EXISTING_RECORD << endl;
+    cout << "test existing NAPTR: " << EXISTING_RECORD << endl;
     {
         DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest(EXISTING_RECORD);
         DNS::Request_ptr r(naptrRequest);
@@ -110,17 +112,18 @@ int main(int argc, char* argv[])
         }
 
         if( r->isTimeout() ) {
-            cerr << "timeout testing existing record " EXISTING_RECORD << endl;
+            cerr << "timeout testing existing record " << EXISTING_RECORD << endl;
             return EXIT_FAILURE;
         }
         if(naptrRequest->entries.empty()) {
-            cerr << "no results for " EXISTING_RECORD << endl;
+            cerr << "no results for " << EXISTING_RECORD << endl;
             return EXIT_FAILURE;
         }
 
         cout << "test for ascending preference/order" << endl;
         int order = -1, preference = -1;
         for( DNS::NAPTRRequest::NAPTR_list::const_iterator it = naptrRequest->entries.begin(); it != naptrRequest->entries.end(); ++it ) {
+            cout << "NAPTR " << (*it)->order << " " << (*it)->preference << " '" << (*it)->service << "' '" << (*it)->regexp << "' '" << (*it)->replacement << "'" << endl;
             // currently only support "U" which implies empty replacement
             SG_CHECK_EQUAL((*it)->flags, "U" );
             SG_CHECK_EQUAL(naptrRequest->entries[0]->replacement, "" );
@@ -154,6 +157,29 @@ int main(int argc, char* argv[])
 
         }
     }
+    cout << "test existing NAPTR with explicit qservice: " << QSERVICE << endl;
+    {
+        DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest(EXISTING_RECORD);
+        naptrRequest->qservice = QSERVICE;
+        DNS::Request_ptr r(naptrRequest);
+        cl.makeRequest(r);
+        while( !r->isComplete() && !r->isTimeout()) {
+            SGTimeStamp::sleepForMSec(200);
+            cl.update(0);
+        }
+
+        if( r->isTimeout() ) {
+            cerr << "timeout testing existing record " << EXISTING_RECORD << endl;
+            return EXIT_FAILURE;
+        }
+        if(naptrRequest->entries.empty()) {
+            cerr << "no results for " << EXISTING_RECORD << endl;
+            //return EXIT_FAILURE; // not yet a failure - probably add this for 2017.4 and create DNS entries
+        }
+        for( DNS::NAPTRRequest::NAPTR_list::const_iterator it = naptrRequest->entries.begin(); it != naptrRequest->entries.end(); ++it ) {
+            cout << "NAPTR " << (*it)->order << " " << (*it)->preference << " '" << (*it)->service << "' '" << (*it)->regexp << "' '" << (*it)->replacement << "'" << endl;
+        }
+    }
 
     cout << "test non-existing NAPTR" << endl;
     {