apple dns: resolve IP addresses without calling Apple APIs

source/common/network/apple_dns_impl.cc

@@ -113,22 +113,44 @@ ActiveDnsQuery* AppleDnsResolverImpl::resolve(const std::string& dns_name,
                                               DnsLookupFamily dns_lookup_family,
                                               ResolveCb callback) {
   ENVOY_LOG(debug, "DNS resolver resolve={}", dns_name);
-  std::unique_ptr<PendingResolution> pending_resolution(
-      new PendingResolution(*this, callback, dispatcher_, main_sd_ref_, dns_name));
 
-  DNSServiceErrorType error = pending_resolution->dnsServiceGetAddrInfo(dns_lookup_family);
-  if (error != kDNSServiceErr_NoError) {
-    ENVOY_LOG(warn, "DNS resolver error ({}) in dnsServiceGetAddrInfo for {}", error, dns_name);
-    return nullptr;
-  }
+  Address::InstanceConstSharedPtr address{};
+  try {
+    // When an IP address is submitted to c-ares in DnsResolverImpl, c-ares synchronously returns
+    // the IP without submitting a DNS query. Because Envoy has come to rely on this behavior, this
+    // resolver implements a similar resolution path to avoid making improper DNS queries for
+    // resolved IPs.
+    address = Utility::parseInternetAddress(dns_name);
+    ENVOY_LOG(debug, "DNS resolver resolved ({}) to ({}) without issuing call to Apple API",
+              dns_name, address->asString());
+  } catch (const EnvoyException& e) {
+    // Resolution via Apple APIs
+    ENVOY_LOG(debug, "DNS resolver local resolution failed with: {}", e.what());
+    std::unique_ptr<PendingResolution> pending_resolution(
+        new PendingResolution(*this, callback, dispatcher_, main_sd_ref_, dns_name));
+
+    DNSServiceErrorType error = pending_resolution->dnsServiceGetAddrInfo(dns_lookup_family);
+    if (error != kDNSServiceErr_NoError) {
+      ENVOY_LOG(warn, "DNS resolver error ({}) in dnsServiceGetAddrInfo for {}", error, dns_name);
+      return nullptr;
+    }
 
-  // If the query was synchronously resolved, there is no need to return the query.
-  if (pending_resolution->synchronously_completed_) {
-    return nullptr;
+    // If the query was synchronously resolved in the Apple API call, there is no need to return the
+    // query.
+    if (pending_resolution->synchronously_completed_) {
+      return nullptr;
+    }
+
+    pending_resolution->owned_ = true;
+    return pending_resolution.release();
   }
 
-  pending_resolution->owned_ = true;
-  return pending_resolution.release();
+  ASSERT(address != nullptr);
+  // Finish local, synchronous resolution. This needs to happen outside of the exception block above
+  // as the callback itself can throw.
+  callback(DnsResolver::ResolutionStatus::Success,
+           {DnsResponse(address, std::chrono::seconds(60))});
+  return nullptr;
 }
 
 void AppleDnsResolverImpl::addPendingQuery(PendingResolution* query) {
@@ -146,16 +168,9 @@ void AppleDnsResolverImpl::flushPendingQueries(const bool with_error) {
   for (std::set<PendingResolution*>::iterator it = queries_with_pending_cb_.begin();
        it != queries_with_pending_cb_.end(); ++it) {
     auto query = *it;
-    try {
-      ASSERT(query->pending_cb_);
-      query->callback_(query->pending_cb_->status_, std::move(query->pending_cb_->responses_));
-    } catch (const std::exception& e) {
-      ENVOY_LOG(warn, "std::exception in DNSService callback: {}", e.what());
-      throw EnvoyException(e.what());
-    } catch (...) {
-      ENVOY_LOG(warn, "Unknown exception in DNSService callback");
-      throw EnvoyException("unknown");
-    }
+
+    ASSERT(query->pending_cb_);
+    query->callback_(query->pending_cb_->status_, std::move(query->pending_cb_->responses_));
 
     if (query->owned_) {
       ENVOY_LOG(debug, "Resolution for {} completed (async)", query->dns_name_);

source/common/network/utility.h

@@ -72,6 +72,7 @@ class Utility {
    * Resolve a URL.
    * @param url supplies the url to resolve.
    * @return Address::InstanceConstSharedPtr the resolved address.
+   * @throw EnvoyException if url is invalid.
    */
   static Address::InstanceConstSharedPtr resolveUrl(const std::string& url);
 

test/common/network/apple_dns_impl_test.cc

@@ -97,16 +97,15 @@ class AppleDnsImplTest : public testing::Test {
                               });
   }
 
-  template <typename T>
   ActiveDnsQuery* resolveWithException(const std::string& address,
-                                       const DnsLookupFamily lookup_family, T exception_object) {
-    return resolver_->resolve(address, lookup_family,
-                              [exception_object](DnsResolver::ResolutionStatus status,
-                                                 std::list<DnsResponse>&& results) -> void {
-                                UNREFERENCED_PARAMETER(status);
-                                UNREFERENCED_PARAMETER(results);
-                                throw exception_object;
-                              });
+                                       const DnsLookupFamily lookup_family) {
+    return resolver_->resolve(
+        address, lookup_family,
+        [](DnsResolver::ResolutionStatus status, std::list<DnsResponse>&& results) -> void {
+          UNREFERENCED_PARAMETER(status);
+          UNREFERENCED_PARAMETER(results);
+          throw EnvoyException("Envoy exception");
+        });
   }
 
 protected:
@@ -154,24 +153,14 @@ TEST_F(AppleDnsImplTest, DnsIpAddressVersion) {
 }
 
 TEST_F(AppleDnsImplTest, CallbackException) {
-  EXPECT_NE(nullptr, resolveWithException<EnvoyException>("1.2.3.4", DnsLookupFamily::V4Only,
-                                                          EnvoyException("Envoy exception")));
+  EXPECT_NE(nullptr, resolveWithException("google.com", DnsLookupFamily::V4Only));
   EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
                             "Envoy exception");
 }
 
-TEST_F(AppleDnsImplTest, CallbackException2) {
-  EXPECT_NE(nullptr, resolveWithException<std::runtime_error>("1.2.3.4", DnsLookupFamily::V4Only,
-                                                              std::runtime_error("runtime error")));
-  EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
-                            "runtime error");
-}
-
-TEST_F(AppleDnsImplTest, CallbackException3) {
-  EXPECT_NE(nullptr,
-            resolveWithException<std::string>("1.2.3.4", DnsLookupFamily::V4Only, std::string()));
-  EXPECT_THROW_WITH_MESSAGE(dispatcher_->run(Event::Dispatcher::RunType::Block), EnvoyException,
-                            "unknown");
+TEST_F(AppleDnsImplTest, CallbackExceptionLocalResolution) {
+  EXPECT_THROW_WITH_MESSAGE(resolveWithException("1.2.3.4", DnsLookupFamily::V4Only),
+                            EnvoyException, "Envoy exception");
 }
 
 // Validate working of cancellation provided by ActiveDnsQuery return.
@@ -194,6 +183,19 @@ TEST_F(AppleDnsImplTest, Timeout) {
   dispatcher_->run(Event::Dispatcher::RunType::Block);
 }
 
+TEST_F(AppleDnsImplTest, LocalResolution) {
+  auto pending_resolution = resolver_->resolve(
+      "0.0.0.0", DnsLookupFamily::Auto,
+      [](DnsResolver::ResolutionStatus status, std::list<DnsResponse>&& results) -> void {
+        EXPECT_EQ(DnsResolver::ResolutionStatus::Success, status);
+        EXPECT_EQ(1, results.size());
+        EXPECT_EQ("0.0.0.0:0", results.front().address_->asString());
+        EXPECT_EQ(std::chrono::seconds(60), results.front().ttl_);
+      });
+  EXPECT_EQ(nullptr, pending_resolution);
+  // Note that the dispatcher does NOT have to run because resolution is synchronous.
+}
+
 // This class compliments the tests above by using a mocked Apple API that allows finer control over
 // error conditions, and callback firing.
 class AppleDnsImplFakeApiTest : public testing::Test {