[NaCl SDK] nacl_io: implement getaddrinfo()

native_client_sdk/src/libraries/nacl_io/host_resolver.cc

Index: native_client_sdk/src/libraries/nacl_io/host_resolver.cc
diff --git a/native_client_sdk/src/libraries/nacl_io/host_resolver.cc b/native_client_sdk/src/libraries/nacl_io/host_resolver.cc
index 4fbf3c2150dc7355c0ca0c6bd179217c2ab56809..549b1da5e714667eb43052efa97454071a9e52cb 100644
--- a/native_client_sdk/src/libraries/nacl_io/host_resolver.cc
+++ b/native_client_sdk/src/libraries/nacl_io/host_resolver.cc
@@ -4,6 +4,7 @@
 
 #include "nacl_io/host_resolver.h"
 
+#include <assert.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -13,6 +14,65 @@
 
 #ifdef PROVIDES_SOCKET_API
 
+namespace {
+
+void HintsToPPHints(const addrinfo* hints, PP_HostResolver_Hint* pp_hints) {
+  memset(pp_hints, 0, sizeof(*pp_hints));
+
+  if (hints->ai_family == AF_INET)
+    pp_hints->family = PP_NETADDRESS_FAMILY_IPV4;
+  else if (hints->ai_family == AF_INET6)
+    pp_hints->family = PP_NETADDRESS_FAMILY_IPV6;

[binji, 2014/01/29 19:18:52]

default value for else?

[Sam Clegg, 2014/01/29 22:06:13]

Default value is UNSPECIFIED which is 0.

+
+  if (hints->ai_flags & AI_CANONNAME)
+    pp_hints->flags = PP_HOSTRESOLVER_FLAG_CANONNAME;
+}
+
+void CreateAddrInfo(const addrinfo* hints,
+                    struct sockaddr* addr,
+                    const char* name,
+                    addrinfo** list_start,
+                    addrinfo** list_end) {
+  addrinfo* ai = new addrinfo();
+  memset(ai, 0, sizeof(*ai));

[binji, 2014/01/29 19:18:52]

Technically this is unnecessary because addrinfo is POD and you called new
addrinfo with parentheses above: see http://stackoverflow.com/a/620402

Probably obscure enough of a rule to leave as is (or comment) however.

[Sam Clegg, 2014/01/29 22:06:13]

Very interesting.  I removed the parens and kept the memset

+
+  if (hints && hints->ai_socktype)
+    ai->ai_socktype = hints->ai_socktype;
+  else
+    ai->ai_socktype = SOCK_STREAM;
+
+  if (hints && hints->ai_protocol)
+    ai->ai_protocol = hints->ai_protocol;
+
+  if (name)
+    ai->ai_canonname = strdup(name);
+
+  if (addr->sa_family == AF_INET6) {
+    sockaddr_in6* in = new sockaddr_in6();
+    *in = *(sockaddr_in6*)addr;
+    ai->ai_family = AF_INET6;
+    ai->ai_addr = reinterpret_cast<sockaddr*>(in);
+    ai->ai_addrlen = sizeof(*in);
+  } else if (addr->sa_family == AF_INET) {
+    sockaddr_in* in = new sockaddr_in();
+    *in = *(sockaddr_in*)addr;
+    ai->ai_family = AF_INET;
+    ai->ai_addr = reinterpret_cast<sockaddr*>(in);
+    ai->ai_addrlen = sizeof(*in);
+  }

[binji, 2014/01/29 19:18:52]

default case?

[Sam Clegg, 2014/01/29 22:06:13]

We only support these to families.  I've added an assert here in the default
case.

+
+  if (*list_start == NULL) {
+    *list_start = ai;
+    *list_end = ai;
+    return;
+  }
+
+  (*list_end)->ai_next = ai;
+  *list_end = ai;
+}
+
+}  // namespace
+
 namespace nacl_io {
 
 HostResolver::HostResolver() : hostent_(), ppapi_(NULL) {
@@ -29,74 +89,49 @@ void HostResolver::Init(PepperInterface* ppapi) {
 struct hostent* HostResolver::gethostbyname(const char* name) {
   h_errno = NETDB_INTERNAL;
 
-  if (NULL == ppapi_)
-    return NULL;
-
-  HostResolverInterface* resolver_interface =
-      ppapi_->GetHostResolverInterface();
-  VarInterface* var_interface = ppapi_->GetVarInterface();
-  NetAddressInterface* netaddr_iface = ppapi_->GetNetAddressInterface();
-
-  if (NULL == resolver_interface ||
-      NULL == netaddr_iface ||
-      NULL == var_interface)
-    return NULL;
-
-  ScopedResource resolver(ppapi_,
-                          resolver_interface->Create(ppapi_->GetInstance()));
-
-  struct PP_CompletionCallback callback;
-  callback.func = NULL;
-  struct PP_HostResolver_Hint hint;
-  hint.family = PP_NETADDRESS_FAMILY_IPV4;
-  hint.flags = PP_HOSTRESOLVER_FLAG_CANONNAME;
-
-  int err = resolver_interface->Resolve(resolver.pp_resource(),
-                                        name, 0, &hint, callback);
+  struct addrinfo* ai;
+  struct addrinfo hints;
+  memset(&hints, 0, sizeof(hints));
+  hints.ai_flags = AI_CANONNAME;
+  int err = getaddrinfo(name, NULL, &hints, &ai);
   if (err) {
     switch (err) {
-    case PP_ERROR_NOACCESS:
-      h_errno = NO_RECOVERY;
-      break;
-    case PP_ERROR_NAME_NOT_RESOLVED:
-      h_errno = HOST_NOT_FOUND;
-      break;
-    default:
-      h_errno = NETDB_INTERNAL;
-      break;
+      case PP_ERROR_NOACCESS:

[binji, 2014/01/29 19:18:52]

getaddrinfo seems to return EAI_ errors, not PP_ errors.

[Sam Clegg, 2014/01/29 22:06:13]

Good call.  Added tests for that too.

+        h_errno = NO_RECOVERY;
+        break;
+      case PP_ERROR_NAME_NOT_RESOLVED:
+        h_errno = HOST_NOT_FOUND;
+        break;
+      default:
+        h_errno = NETDB_INTERNAL;
+        break;
     }
     return NULL;
   }
 
   // We use a single hostent struct for all calls to to gethostbyname
   // (as explicitly permitted by the spec - gethostbyname is NOT supposed to
-  // be threadsafe!), so the first thing we do is free all the malloced data
-  // left over from the last call.
-  hostent_cleanup();
-
-  PP_Var name_var =
-    resolver_interface->GetCanonicalName(resolver.pp_resource());
-  if (PP_VARTYPE_STRING != name_var.type)
-    return NULL;
+  // be threadsafe!).  However by using a lock around all the global data
+  // manipulation we can at least ensure that the call doesn't crash.
+  AUTO_LOCK(gethostbyname_lock_);
 
-  uint32_t len;
-  const char* name_ptr = var_interface->VarToUtf8(name_var, &len);
-  if (NULL == name_ptr)
-    return NULL;
-  if (0 == len) {
-    // Sometimes GetCanonicalName gives up more easily than gethostbyname should
-    // (for example, it returns "" when asked to resolve "localhost"), so if we
-    // get an empty string we copy over the input string instead.
-    len = strlen(name);
-    name_ptr = name;
-  }
-  hostent_.h_name = static_cast<char*>(malloc(len + 1));
-  if (NULL == hostent_.h_name)
-    return NULL;
-  memcpy(hostent_.h_name, name_ptr, len);
-  hostent_.h_name[len] = '\0';
+  // The first thing we do is free any malloced data left over from
+  // the last call.
+  hostent_cleanup();
 
-  var_interface->Release(name_var);
+  switch (ai->ai_family) {
+    case AF_INET:
+      hostent_.h_addrtype = AF_INET;
+      hostent_.h_length = sizeof(in_addr);
+      break;
+    case AF_INET6:
+      hostent_.h_addrtype = AF_INET6;
+      hostent_.h_length = sizeof(in6_addr);
+      break;
+    default:
+      return NULL;
+  }
+  hostent_.h_name = strdup(ai->ai_canonname);
 
   // Aliases aren't supported at the moment, so we just make an empty list.
   hostent_.h_aliases = static_cast<char**>(malloc(sizeof(char*)));
@@ -104,59 +139,239 @@ struct hostent* HostResolver::gethostbyname(const char* name) {
     return NULL;
   hostent_.h_aliases[0] = NULL;
 
-  ScopedResource addr(ppapi_);
-  addr.Reset(resolver_interface->GetNetAddress(resolver.pp_resource(), 0));
-  if (!PP_ToBool(netaddr_iface->IsNetAddress(addr.pp_resource())))
+  // Count number of address in list
+  int num_addresses = 0;
+  struct addrinfo* current = ai;
+  while (current != NULL) {
+    // Only count address that have the same type as first address
+    if (current->ai_family == hostent_.h_addrtype)
+      num_addresses++;
+    current = current->ai_next;
+  }
+
+  // Allocate address list
+  hostent_.h_addr_list = static_cast<char**>(calloc(num_addresses + 1,
+                                                    sizeof(char*)));
+  if (NULL == hostent_.h_addr_list)
     return NULL;
 
-  switch (netaddr_iface->GetFamily(addr.pp_resource())) {
-    case PP_NETADDRESS_FAMILY_IPV4:
-      hostent_.h_addrtype = AF_INET;
-      hostent_.h_length = 4;
-      break;
-    case PP_NETADDRESS_FAMILY_IPV6:
-      hostent_.h_addrtype = AF_INET6;
-      hostent_.h_length = 16;
+  // Copy all addresses of the relevant family.
+  current = ai;
+  for (int i = 0; i < num_addresses; i++) {

[binji, 2014/01/29 19:18:52]

Now that I look at it closely, this loop is wrong. I think i++ should be moved
past the continue guard below; otherwise we'll drop addresses at the end of the
list.

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+    if (current->ai_family != hostent_.h_addrtype) {
+      continue;

[binji, 2014/01/29 19:18:52]

This continue is broken; it needs to always do

current = current->ai_next.

Perhaps the loop should be a while loop like above?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+    }
+    hostent_.h_addr_list[i] = static_cast<char*>(malloc(hostent_.h_length));
+    switch (current->ai_family) {
+      case AF_INET: {
+        sockaddr_in* in = reinterpret_cast<sockaddr_in*>(current->ai_addr);
+        memcpy(hostent_.h_addr_list[i],
+               &in->sin_addr.s_addr,
+               hostent_.h_length);
+        break;
+      }
+      case AF_INET6: {
+        sockaddr_in6* in6 = reinterpret_cast<sockaddr_in6*>(current->ai_addr);
+        memcpy(hostent_.h_addr_list[i],
+               &in6->sin6_addr.s6_addr,
+               hostent_.h_length);
+        break;
+      }
+    }
+    current = current->ai_next;
+  }
+
+  freeaddrinfo(ai);
+  return &hostent_;
+}
+
+void HostResolver::freeaddrinfo(struct addrinfo *res) {
+  while (res) {
+    struct addrinfo* cur = res;
+    res = res->ai_next;
+    free(cur);

[binji, 2014/01/29 19:18:52]

move this below the other frees

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+    free(cur->ai_addr);
+    free(cur->ai_canonname);
+  }
+}
+
+int HostResolver::getaddrinfo(const char* node, const char* service,
+                              const struct addrinfo* hints_in,
+                              struct addrinfo** result) {
+  if (node == NULL && service == NULL)
+    return EAI_NONAME;
+
+  // Check the service name (port).  Currently we only handle numeric
+  // services.
+  long port = 0;
+  if (service != NULL) {
+    char* cp;
+    port = strtol(service, &cp, 10);
+    if (port > 0 && port <= 65535 && *cp == '\0') {
+      port = htons(port);
+    } else {
+      return EAI_SERVICE;
+    }
+  }
+
+  struct addrinfo default_hints;
+  memset(&default_hints, 0, sizeof(default_hints));
+  const struct addrinfo* hints = hints_in ? hints_in : &default_hints;
+
+  // Verify values passed in hints structure
+  switch (hints->ai_family) {
+    case AF_INET6:
+    case AF_INET:
+    case 0:

[binji, 2014/01/29 19:18:52]

AF_UNSPEC?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

       break;
     default:
-      return NULL;
+      return EAI_FAMILY;
+  }
+
+  struct sockaddr_in addr_in;
+  memset(&addr_in, 0, sizeof(addr_in));
+  addr_in.sin_family = AF_INET;
+  addr_in.sin_port = port;
+
+  struct sockaddr_in6 addr_in6;
+  memset(&addr_in6, 0, sizeof(addr_in6));
+  addr_in6.sin6_family = AF_INET6;
+  addr_in6.sin6_port = port;
+
+  *result = NULL;

[binji, 2014/01/29 19:18:52]

set this to NULL earlier in the function?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+  struct addrinfo* end = NULL;
+
+  if (node) {
+    // Handle numeric node name.
+    if (hints->ai_family == AF_INET || hints->ai_family == AF_UNSPEC) {
+      in_addr in;
+      if (inet_pton(AF_INET, node, &in)) {
+        addr_in.sin_addr = in;
+        CreateAddrInfo(hints, (sockaddr*)&addr_in, node, result, &end);
+        return 0;
+      }
+    }
+
+    if (hints->ai_family == AF_INET6 || hints->ai_family == AF_UNSPEC) {
+      in6_addr in6;
+      if (inet_pton(AF_INET6, node, &in6)) {
+        addr_in6.sin6_addr = in6;
+        CreateAddrInfo(hints, (sockaddr*)&addr_in6, node, result, &end);
+        return 0;
+      }
+    }
+  }
+
+  // Handle AI_PASSIVE (used for listening sockets, e.g. INADDR_ANY)
+  if (node == NULL && (hints->ai_flags & AI_PASSIVE)) {
+    if (hints->ai_family == AF_INET6 || hints->ai_family == 0) {

[binji, 2014/01/29 19:18:52]

AF_UNSPEC?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+      const in6_addr in6addr_any = IN6ADDR_ANY_INIT;
+      memcpy(&addr_in6.sin6_addr.s6_addr, &in6addr_any, sizeof(in6addr_any));
+      CreateAddrInfo(hints, (sockaddr*)&addr_in6, NULL, result, &end);

[binji, 2014/01/29 19:18:52]

Is it correct to return both v4 and v6 addresses in this case?

[Sam Clegg, 2014/01/29 22:06:13]

Yes. At least this is what glibc/linux does.

+    }
+
+    if (hints->ai_family == AF_INET || hints->ai_family == 0) {
+      addr_in.sin_addr.s_addr = INADDR_ANY;
+      CreateAddrInfo(hints, (sockaddr*)&addr_in, NULL, result, &end);
+    }
+    return 0;
   }
 
-  const uint32_t num_addresses =
-    resolver_interface->GetNetAddressCount(resolver.pp_resource());
+  if (NULL == ppapi_)
+    return EAI_SYSTEM;
+
+  // Use PPAPI interface to resolve nodename
+  HostResolverInterface* resolver_iface = ppapi_->GetHostResolverInterface();
+  VarInterface* var_interface = ppapi_->GetVarInterface();
+  NetAddressInterface* netaddr_iface = ppapi_->GetNetAddressInterface();
+
+  if (NULL == resolver_iface || NULL == var_interface || NULL == netaddr_iface)
+    return EAI_SYSTEM;
+
+  ScopedResource scoped_resolver(ppapi_,
+                                 resolver_iface->Create(ppapi_->GetInstance()));
+  PP_Resource resolver = scoped_resolver.pp_resource();
+
+  struct PP_HostResolver_Hint pp_hints;
+  HintsToPPHints(hints, &pp_hints);
+
+  struct PP_CompletionCallback callback;
+  callback.func = NULL;

[binji, 2014/01/29 19:18:52]

Just use PP_BlockUntilComplete()?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+  int err = resolver_iface->Resolve(resolver, node, 0, &pp_hints, callback);
+  if (err) {
+    switch (err) {
+      case PP_ERROR_NOACCESS:
+        return EAI_SYSTEM;
+      case PP_ERROR_NAME_NOT_RESOLVED:
+        return EAI_NONAME;
+      default:
+        return EAI_SYSTEM;
+    }
+  }
+
+  char* canon_name = NULL;
+  if (hints != NULL && hints->ai_flags & AI_CANONNAME) {
+    PP_Var name_var = resolver_iface->GetCanonicalName(resolver);
+
+    uint32_t len = 0;
+    const char* tmp = var_interface->VarToUtf8(name_var, &len);
+    if (len > 0) {
+      // Copy and NULL-terminate the UTF8 string var.
+      canon_name = static_cast<char*>(alloca(len+1));
+      strncpy(canon_name, tmp, len);
+      canon_name[len] = '\0';
+    }
+    var_interface->Release(name_var);
+  }
+
+  ScopedResource addr(ppapi_);
+  addr.Reset(resolver_iface->GetNetAddress(resolver, 0));

[binji, 2014/01/29 19:18:52]

just construct this directly, like above?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

+  if (addr.pp_resource() == 0 ||
+      !PP_ToBool(netaddr_iface->IsNetAddress(addr.pp_resource())))
+    return EAI_SYSTEM;
+
+  int num_addresses = resolver_iface->GetNetAddressCount(resolver);
   if (0 == num_addresses)
-    return NULL;
-  hostent_.h_addr_list =
-    static_cast<char**>(calloc(num_addresses + 1, sizeof(char*)));
-  if (NULL == hostent_.h_addr_list)
-    return NULL;
+    return EAI_NONAME;

[binji, 2014/01/29 19:18:52]

Maybe EAI_NODATA is more correct here?

[Sam Clegg, 2014/01/29 22:06:13]

Done.

 
-  for (uint32_t i = 0; i < num_addresses; i++) {
-    addr.Reset(resolver_interface->GetNetAddress(resolver.pp_resource(), i));
-    if (!PP_ToBool(netaddr_iface->IsNetAddress(addr.pp_resource())))
-      return NULL;
-    if (AF_INET == hostent_.h_addrtype) {
-      struct PP_NetAddress_IPv4 addr_struct;
-      if (!netaddr_iface->DescribeAsIPv4Address(addr.pp_resource(),
-                                                &addr_struct))
-        return NULL;
-      hostent_.h_addr_list[i] = static_cast<char*>(malloc(hostent_.h_length));
-      if (NULL == hostent_.h_addr_list[i])
-        return NULL;
-      memcpy(hostent_.h_addr_list[i], addr_struct.addr, hostent_.h_length);
-    } else { // IPv6
-      struct PP_NetAddress_IPv6 addr_struct;
-      if (!netaddr_iface->DescribeAsIPv6Address(addr.pp_resource(),
-                                                &addr_struct))
-        return NULL;
-      hostent_.h_addr_list[i] = static_cast<char*>(malloc(hostent_.h_length));
-      if (NULL == hostent_.h_addr_list[i])
-        return NULL;
-      memcpy(hostent_.h_addr_list[i], addr_struct.addr, hostent_.h_length);
+  // Convert address to sockaddr struct.
+  for (int i = 0; i < num_addresses; i++) {
+    addr.Reset(resolver_iface->GetNetAddress(resolver, i));
+    struct sockaddr* sockaddr = NULL;
+    switch (netaddr_iface->GetFamily(addr.pp_resource())) {
+      case PP_NETADDRESS_FAMILY_IPV4: {
+        struct PP_NetAddress_IPv4 addr_struct;
+        if (!netaddr_iface->DescribeAsIPv4Address(addr.pp_resource(),
+                                                  &addr_struct)) {
+          assert(false);

[binji, 2014/01/29 19:18:52]

If this fires, it is a PPAPI failure, not ours. Maybe we should be resilient to
that?

[Sam Clegg, 2014/01/29 22:06:13]

This is resilient, at least in release builds it will just skip over the
address.

+          break;
+        }
+        memcpy(&addr_in.sin_addr, addr_struct.addr, sizeof(addr_in.sin_addr));
+        sockaddr = (struct sockaddr*)&addr_in;
+        break;
+      }
+      case PP_NETADDRESS_FAMILY_IPV6: {
+        struct PP_NetAddress_IPv6 addr_struct;
+        if (!netaddr_iface->DescribeAsIPv6Address(addr.pp_resource(),
+                                                  &addr_struct)) {
+          assert(false);
+          break;
+        }
+        memcpy(&addr_in6.sin6_addr, addr_struct.addr,
+               sizeof(addr_in6.sin6_addr));
+        sockaddr = (struct sockaddr*)&addr_in6;
+        break;
+      }
+      default:
+        return EAI_SYSTEM;
     }
+
+    if (sockaddr != NULL)
+      CreateAddrInfo(hints, sockaddr, canon_name, result, &end);
+    canon_name = NULL;
   }
 
-  return &hostent_;
+  return 0;
 }
 
 // Frees all of the deep pointers in a hostent struct. Called between uses of