Merge pull request #1774 from fadams/fix-getaddrinfo-null-hints

Provided an implementation for gai_strerror and gave getaddrinfo sensibl...

3 files changed, 113 insertions, 6 deletions

diff --git a/src/library.js b/src/library.js
index 7a954662..b25e48ed 100644
--- a/src/library.js
+++ b/src/library.js
@@ -7414,6 +7414,9 @@ LibraryManager.library = {
 
   getaddrinfo__deps: ['$Sockets', '$DNS', '_inet_pton4_raw', '_inet_ntop4_raw', '_inet_pton6_raw', '_inet_ntop6_raw', '_write_sockaddr', 'htonl'],
   getaddrinfo: function(node, service, hint, out) {
+    // Note getaddrinfo currently only returns a single addrinfo with ai_next defaulting to NULL. When NULL
+    // hints are specified or ai_family set to AF_UNSPEC or ai_socktype or ai_protocol set to 0 then we
+    // really should provide a linked list of suitable addrinfo values.
     var addrs = [];
     var canon = null;
     var addr = 0;
@@ -7468,6 +7471,15 @@ LibraryManager.library = {
       type = proto === {{{ cDefine('IPPROTO_UDP') }}} ? {{{ cDefine('SOCK_DGRAM') }}} : {{{ cDefine('SOCK_STREAM') }}};
     }
 
+    // If type or proto are set to zero in hints we should really be returning multiple addrinfo values, but for
+    // now default to a TCP STREAM socket so we can at least return a sensible addrinfo given NULL hints.
+    if (proto === 0) {
+      proto = {{{ cDefine('IPPROTO_TCP') }}};
+    }
+    if (type === 0) {
+      type = {{{ cDefine('SOCK_STREAM') }}};
+    }
+
     if (!node && !service) {
       return {{{ cDefine('EAI_NONAME') }}};
     }
@@ -7475,14 +7487,14 @@ LibraryManager.library = {
         {{{ cDefine('AI_NUMERICSERV') }}}|{{{ cDefine('AI_V4MAPPED') }}}|{{{ cDefine('AI_ALL') }}}|{{{ cDefine('AI_ADDRCONFIG') }}})) {
       return {{{ cDefine('EAI_BADFLAGS') }}};
     }
-    if (({{{ makeGetValue('hint', C_STRUCTS.addrinfo.ai_flags, 'i32') }}} & {{{ cDefine('AI_CANONNAME') }}}) && !node) {
+    if (hint !== 0 && ({{{ makeGetValue('hint', C_STRUCTS.addrinfo.ai_flags, 'i32') }}} & {{{ cDefine('AI_CANONNAME') }}}) && !node) {
       return {{{ cDefine('EAI_BADFLAGS') }}};
     }
     if (flags & {{{ cDefine('AI_ADDRCONFIG') }}}) {
       // TODO
       return {{{ cDefine('EAI_NONAME') }}};
     }
-    if (type !== {{{ cDefine('SOCK_STREAM') }}} && type !== {{{ cDefine('SOCK_DGRAM') }}}) {
+    if (type !== 0 && type !== {{{ cDefine('SOCK_STREAM') }}} && type !== {{{ cDefine('SOCK_DGRAM') }}}) {
       return {{{ cDefine('EAI_SOCKTYPE') }}};
     }
     if (family !== {{{ cDefine('AF_UNSPEC') }}} && family !== {{{ cDefine('AF_INET') }}} && family !== {{{ cDefine('AF_INET6') }}}) {
@@ -7612,12 +7624,43 @@ LibraryManager.library = {
 
     return 0;
   },
+  // Can't use a literal for $GAI_ERRNO_MESSAGES as was done for $ERRNO_MESSAGES as the keys (e.g. EAI_BADFLAGS)
+  // are actually negative numbers and you can't have expressions as keys in JavaScript literals.
+  $GAI_ERRNO_MESSAGES: {},
 
+  gai_strerror__deps: ['$GAI_ERRNO_MESSAGES'],
   gai_strerror: function(val) {
-    if (!_gai_strerror.error) {
-      _gai_strerror.error = allocate(intArrayFromString("unknown error"), 'i8', ALLOC_NORMAL);
+    var buflen = 256;
+
+    // On first call to gai_strerror we initialise the buffer and populate the error messages.
+    if (!_gai_strerror.buffer) {
+        _gai_strerror.buffer = _malloc(buflen);
+
+        GAI_ERRNO_MESSAGES['0'] = 'Success';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_BADFLAGS') }}}] = 'Invalid value for \'ai_flags\' field';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_NONAME') }}}] = 'NAME or SERVICE is unknown';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_AGAIN') }}}] = 'Temporary failure in name resolution';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_FAIL') }}}] = 'Non-recoverable failure in name res';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_FAMILY') }}}] = '\'ai_family\' not supported';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_SOCKTYPE') }}}] = '\'ai_socktype\' not supported';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_SERVICE') }}}] = 'SERVICE not supported for \'ai_socktype\'';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_MEMORY') }}}] = 'Memory allocation failure';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_SYSTEM') }}}] = 'System error returned in \'errno\'';
+        GAI_ERRNO_MESSAGES['' + {{{ cDefine('EAI_OVERFLOW') }}}] = 'Argument buffer overflow';
+    }
+
+    var msg = 'Unknown error';
+
+    if (val in GAI_ERRNO_MESSAGES) {
+      if (GAI_ERRNO_MESSAGES[val].length > buflen - 1) {
+        msg = 'Message too long'; // EMSGSIZE message. This should never occur given the GAI_ERRNO_MESSAGES above. 
+      } else {
+        msg = GAI_ERRNO_MESSAGES[val];
+      }
     }
-    return _gai_strerror.error;
+
+    writeAsciiToMemory(msg, _gai_strerror.buffer);
+    return _gai_strerror.buffer;
   },
 
   // ==========================================================================
diff --git a/src/struct_info.json b/src/struct_info.json
index f6499295..b91d077e 100644
--- a/src/struct_info.json
+++ b/src/struct_info.json
@@ -290,7 +290,11 @@
             "AI_CANONNAME", 
             "AI_PASSIVE", 
             "NI_NAMEREQD", 
-            "EAI_NONAME", 
+            "EAI_NONAME",
+            "EAI_AGAIN", 
+            "EAI_FAIL",
+            "EAI_MEMORY",
+            "EAI_SYSTEM",  
             "EAI_SOCKTYPE", 
             "EAI_BADFLAGS"
         ],
diff --git a/tests/sockets/test_getaddrinfo.c b/tests/sockets/test_getaddrinfo.c
index 717a9ae7..1f912c69 100644
--- a/tests/sockets/test_getaddrinfo.c
+++ b/tests/sockets/test_getaddrinfo.c
@@ -174,6 +174,7 @@ int main() {
   assert(servinfo->ai_family == AF_INET);
   assert(servinfo->ai_socktype == SOCK_STREAM);
   assert(sa4->sin_port == ntohs(89));
+  freeaddrinfo(servinfo);
 
   // test non-numeric host with AF_INET6
   memset(&hints, 0, sizeof(hints));
@@ -189,6 +190,65 @@ int main() {
          *((uint32_t*)&(sa6->sin6_addr)+2) != 0 ||
          *((uint32_t*)&(sa6->sin6_addr)+3) != 0);
   assert(sa6->sin6_port == ntohs(90));
+  freeaddrinfo(servinfo);
+
+  // test with NULL hints
+  // Specifying hints as NULL is equivalent to setting ai_socktype and ai_protocol to 0;
+  // ai_family to AF_UNSPEC; and ai_flags to (AI_V4MAPPED | AI_ADDRCONFIG)
+  // N.B. with NULL hints getaddrinfo should really be passing back multiple addrinfo structures in a
+  // linked list with next values given in ai_next. The current implementation doesn't do that yet but the
+  // following tests have assert(servinfo->ai_next == NULL) so that they will fail when multiple values do
+  // eventually get implemented, so we know to improve the tests then to cope with multiple values.
+
+  // test numeric host
+  err = getaddrinfo("1.2.3.4", "85", NULL, &servinfo);
+  assert(!err);
+  sa4 = ((struct sockaddr_in*)servinfo->ai_addr);
+  assert(servinfo->ai_family == AF_INET);
+  assert(servinfo->ai_socktype == SOCK_STREAM);
+  assert(servinfo->ai_protocol == IPPROTO_TCP);
+  assert(sa4->sin_port == ntohs(85));
+  assert(servinfo->ai_next == NULL);
+  freeaddrinfo(servinfo);
+
+  // test non-numeric host
+  err = getaddrinfo("www.mozilla.org", "89", NULL, &servinfo);
+  assert(!err);
+  sa4 = ((struct sockaddr_in*)servinfo->ai_addr);
+  assert(servinfo->ai_family == AF_INET);
+  assert(servinfo->ai_socktype == SOCK_STREAM);
+  assert(servinfo->ai_protocol == IPPROTO_TCP);
+  assert(sa4->sin_port == ntohs(89));
+  assert(servinfo->ai_next == NULL);
+  freeaddrinfo(servinfo);
+
+  // test loopback resolution
+  err = getaddrinfo(NULL, "80", NULL, &servinfo);
+  assert(!err);
+  sa4 = ((struct sockaddr_in*)servinfo->ai_addr);
+  assert(servinfo->ai_family == AF_INET);
+  assert(servinfo->ai_socktype == SOCK_STREAM);
+  assert(servinfo->ai_protocol == IPPROTO_TCP);
+  assert(sa4->sin_port == ntohs(80));
+  assert(servinfo->ai_next == NULL);
+  freeaddrinfo(servinfo);
+
+  // test gai_strerror
+  assert(strncmp(gai_strerror(0), "Success", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_BADFLAGS), "Invalid value for 'ai_flags' field", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_NONAME), "NAME or SERVICE is unknown", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_AGAIN), "Temporary failure in name resolution", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_FAIL), "Non-recoverable failure in name res", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_FAMILY), "'ai_family' not supported", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_SOCKTYPE), "'ai_socktype' not supported", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_SERVICE), "SERVICE not supported for 'ai_socktype'", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_MEMORY), "Memory allocation failure", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_SYSTEM), "System error returned in 'errno'", 256) == 0);
+  assert(strncmp(gai_strerror(EAI_OVERFLOW), "Argument buffer overflow", 256) == 0);
+  assert(strncmp(gai_strerror(-5), "Unknown error", 256) == 0);
+  assert(strncmp(gai_strerror(-9), "Unknown error", 256) == 0);
+  assert(strncmp(gai_strerror(-13), "Unknown error", 256) == 0);
+  assert(strncmp(gai_strerror(-100), "Unknown error", 256) == 0);
 
   puts("success");