Detect domain-specific resolvers on OS X and disable DnsClient in such cases.

net/dns/dns_config_service_posix.cc

Index: net/dns/dns_config_service_posix.cc
diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc
index ff2295e704a6aa3b1b6a80ac49d2d31185f9717f..a3b75f495358eb676b7522b9186e8819eaecbe11 100644
--- a/net/dns/dns_config_service_posix.cc
+++ b/net/dns/dns_config_service_posix.cc
@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_path_watcher.h"
+#include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
@@ -20,6 +21,55 @@
 #include "net/dns/notify_watcher_mac.h"
 #include "net/dns/serial_worker.h"
 
+#if defined(OS_MACOSX)
+#include <dlfcn.h>
+#include "third_party/apple_dnsinfo/dnsinfo.h"
+namespace {
+
+// dnsinfo symbols are available via System.framework, but can

[Mark Mentovai, 2013/08/06 15:41:00]

System.framework is just a symbolic link to libSystem.dylib. Nobody knows it by
that name. Replace System.framework with libSystem.dylib on this line and on
line 31…

[szym, 2013/08/06 17:50:25]

Done.

+// also be present in SystemConfiguration. To avoid confusion,

[Mark Mentovai, 2013/08/06 15:41:00]

…and say SystemConfiguration.framework on this line to eliminate ambiguity.

[szym, 2013/08/06 17:50:25]

Done.

+// load them explicitly from System.
+class DnsInfoApi {
+ public:
+  typedef const char* (*dns_configuration_notify_key_t)();
+  typedef dns_config_t* (*dns_configuration_copy_t)();
+  typedef void (*dns_configuration_free_t)(dns_config_t*);
+
+  DnsInfoApi() {
+    handle_ = dlopen("/usr/lib/libSystem.dylib",
+                     RTLD_LAZY | RTLD_NOLOAD);
+    if (!handle_) return;
+    dns_configuration_notify_key =
+      reinterpret_cast<dns_configuration_notify_key_t>(
+        dlsym(handle_, "dns_configuration_notify_key"));
+    dns_configuration_copy =
+      reinterpret_cast<dns_configuration_copy_t>(
+        dlsym(handle_, "dns_configuration_copy"));
+    dns_configuration_free =
+      reinterpret_cast<dns_configuration_free_t>(
+        dlsym(handle_, "dns_configuration_free"));
+  }
+  ~DnsInfoApi() {
+    if (handle_) dlclose(handle_);
+  }
+  bool ready() const { return handle_; }

[Mark Mentovai, 2013/08/06 15:41:00]

ready() isn’t a great interface. It checks that the library is open, but not
that the symbols were looked up successfully. When working with private
interfaces, you really need to be more careful.

I recommend eliminating ready() and instead checking for the actual functions
you need at the points of use. On line 87, instead of calling ready(), you could
say

  if (!dns_info_api().dns_configuration_notify_key)
    return false;

Alternatively, you can keep ready() and make it check for all of the symbols
(perhaps as an in-line check, or perhaps by dlclosing and NULLing out handle_ in
the constructor if any dlsym fails).

[szym, 2013/08/06 17:50:25]

Done.

+
+  dns_configuration_notify_key_t dns_configuration_notify_key;

[Mark Mentovai, 2013/08/06 15:41:00]

Someone outside of this class can accidentally say

  dns_info_api().dns_configuration_notify_key = NULL;

That’s kinda bad. You can make the fields const, which will require you to
rewrite this constructor in initializer list format, but that’d be better…

  DnsInfoApi()
      : handle_(dlopen("/usr/lib/libSystem.dylib", RTLD_LAZY | RTLD_NOLOAD),
        dns_configuration_notify_key(handle_ ? reinterpret_cast<…>(dlsym(…)) :
NULL),
        // and so on

This will almost certainly require you to declare

 private:
  void* handle_;

before

 public:
  dns_configuration_notify_key_t dns_configuration_notify_key;

[szym, 2013/08/06 15:48:58]

That's exactly why dns_info_api() returns a _const_ DnsInfoApi&.

+  dns_configuration_copy_t dns_configuration_copy;
+  dns_configuration_free_t dns_configuration_free;
+
+ private:
+  void* handle_;
+};
+
+const DnsInfoApi& dns_info_api() {
+  base::LazyInstance<DnsInfoApi>::Leaky api = LAZY_INSTANCE_INITIALIZER;
+  return api.Get();
+}
+
+}  // namespace
+#endif
+
 namespace net {
 
 #if !defined(OS_ANDROID)
@@ -31,14 +81,12 @@ const base::FilePath::CharType* kFilePathHosts =
     FILE_PATH_LITERAL("/etc/hosts");
 
 #if defined(OS_MACOSX)
-// From 10.7.3 configd-395.10/dnsinfo/dnsinfo.h
-static const char* kDnsNotifyKey =
-    "com.apple.system.SystemConfiguration.dns_configuration";
-
 class ConfigWatcher {
  public:
   bool Watch(const base::Callback<void(bool succeeded)>& callback) {
-    return watcher_.Watch(kDnsNotifyKey, callback);
+    if (!dns_info_api().ready()) return false;
+    return watcher_.Watch(dns_info_api().dns_configuration_notify_key(),
+                          callback);
   }
 
  private:
@@ -100,6 +148,30 @@ ConfigParsePosixResult ReadDnsConfig(DnsConfig* config) {
   res_nclose(&res);
 #endif
 #endif
+
+#if defined(OS_MACOSX)
+  if (!dns_info_api().ready()) return CONFIG_PARSE_POSIX_NO_DNSINFO;
+  dns_config_t* dns_config = dns_info_api().dns_configuration_copy();
+  if (!dns_config)
+    return CONFIG_PARSE_POSIX_NO_DNSINFO;
+
+  // TODO(szym): Parse dns_config_t for resolvers rather than res_state.
+  // DnsClient can't handle domain-specific unscoped resolvers.
+  unsigned num_resolvers = 0;
+  for (int i = 0; i < dns_config->n_resolver; ++i) {
+    dns_resolver_t* resolver = dns_config->resolver[i];
+    if (!resolver->n_nameserver)
+      continue;
+    if (resolver->options && !strcmp(resolver->options, "mdns"))
+      continue;
+    ++num_resolvers;
+  }
+  dns_info_api().dns_configuration_free(dns_config);

[Mark Mentovai, 2013/08/06 15:41:00]

Can you add a scoper to free this instead of calling free explicitly? That would
guard against early returns.

[szym, 2013/08/06 17:50:25]

Done.

+  if (num_resolvers > 1) {
+    LOG(WARNING) << "dns_config has unhandled options!";
+    return CONFIG_PARSE_POSIX_UNHANDLED_OPTIONS;
+  }
+#endif
   // Override timeout value to match default setting on Windows.
   config->timeout = base::TimeDelta::FromSeconds(kDnsTimeoutSeconds);
   return result;
@@ -171,8 +243,20 @@ class DnsConfigServicePosix::ConfigReader : public SerialWorker {
 
   virtual void DoWork() OVERRIDE {
     base::TimeTicks start_time = base::TimeTicks::Now();
+    dns_config_.unhandled_options = false;
     ConfigParsePosixResult result = ReadDnsConfig(&dns_config_);
-    success_ = (result == CONFIG_PARSE_POSIX_OK);
+    switch (result) {
+      case CONFIG_PARSE_POSIX_MISSING_OPTIONS:
+      case CONFIG_PARSE_POSIX_UNHANDLED_OPTIONS:
+        dns_config_.unhandled_options = true;
+        // Fall through.
+      case CONFIG_PARSE_POSIX_OK:
+        success_ = true;
+        break;
+      default:
+        success_ = false;
+        break;
+    }
     UMA_HISTOGRAM_ENUMERATION("AsyncDNS.ConfigParsePosix",
                               result, CONFIG_PARSE_POSIX_MAX);
     UMA_HISTOGRAM_BOOLEAN("AsyncDNS.ConfigParseResult", success_);