hostip: move code to resolve IP address literals to Curl_resolv

The code was duplicated in the various resolver backends.

Also, it was called after the call to `Curl_ipvalid`, which matters in
case of `CURLRES_IPV4` when called from `connect.c:bindlocal`. This
caused test 1048 to fail on classic MinGW.

The code ignores `conn->ip_version` as done previously in the
individual resolver backends.

Move the call to the `resolver_start` callback up to appease test 655,
which wants it to be called also for literal addresses.

Closes https://github.com/curl/curl/pull/4798

lib/asyn-ares.c

@@ -626,26 +626,11 @@ Curl_addrinfo *Curl_resolver_getaddrinfo(struct connectdata *conn,
 {
   char *bufp;
   struct Curl_easy *data = conn->data;
-  struct in_addr in;
   int family = PF_INET;
-#ifdef ENABLE_IPV6 /* CURLRES_IPV6 */
-  struct in6_addr in6;
-#endif /* CURLRES_IPV6 */
 
   *waitp = 0; /* default to synchronous response */
 
-  /* First check if this is an IPv4 address string */
-  if(Curl_inet_pton(AF_INET, hostname, &in) > 0) {
-    /* This is a dotted IP address 123.123.123.123-style */
-    return Curl_ip2addr(AF_INET, &in, hostname, port);
-  }
-
 #ifdef ENABLE_IPV6 /* CURLRES_IPV6 */
-  /* Otherwise, check if this is an IPv6 address string */
-  if(Curl_inet_pton (AF_INET6, hostname, &in6) > 0)
-    /* This must be an IPv6 address literal.  */
-    return Curl_ip2addr(AF_INET6, &in6, hostname, port);
-
   switch(conn->ip_version) {
   default:
 #if ARES_VERSION >= 0x010601

lib/asyn-thread.c

@@ -71,7 +71,6 @@
 #include "strerror.h"
 #include "url.h"
 #include "multiif.h"
-#include "inet_pton.h"
 #include "inet_ntop.h"
 #include "curl_threads.h"
 #include "connect.h"
@@ -692,26 +691,11 @@ Curl_addrinfo *Curl_resolver_getaddrinfo(struct connectdata *conn,
                                          int port,
                                          int *waitp)
 {
-  struct in_addr in;
   struct Curl_easy *data = conn->data;
   struct resdata *reslv = (struct resdata *)data->state.resolver;
 
   *waitp = 0; /* default to synchronous response */
 
-#ifdef ENABLE_IPV6
-  {
-    struct in6_addr in6;
-    /* check if this is an IPv6 address string */
-    if(Curl_inet_pton(AF_INET6, hostname, &in6) > 0)
-      /* This is an IPv6 address literal */
-      return Curl_ip2addr(AF_INET6, &in6, hostname, port);
-  }
-#endif /* ENABLE_IPV6 */
-
-  if(Curl_inet_pton(AF_INET, hostname, &in) > 0)
-    /* This is a dotted IP address 123.123.123.123-style */
-    return Curl_ip2addr(AF_INET, &in, hostname, port);
-
   reslv->start = Curl_now();
 
   /* fire up a new resolver thread! */
@@ -743,25 +727,6 @@ Curl_addrinfo *Curl_resolver_getaddrinfo(struct connectdata *conn,
 
   *waitp = 0; /* default to synchronous response */
 
-#ifndef USE_RESOLVE_ON_IPS
-  {
-    struct in_addr in;
-    /* First check if this is an IPv4 address string */
-    if(Curl_inet_pton(AF_INET, hostname, &in) > 0)
-      /* This is a dotted IP address 123.123.123.123-style */
-      return Curl_ip2addr(AF_INET, &in, hostname, port);
-  }
-#ifdef ENABLE_IPV6
-  {
-    struct in6_addr in6;
-    /* check if this is an IPv6 address string */
-    if(Curl_inet_pton(AF_INET6, hostname, &in6) > 0)
-      /* This is an IPv6 address literal */
-      return Curl_ip2addr(AF_INET6, &in6, hostname, port);
-  }
-#endif /* ENABLE_IPV6 */
-#endif /* !USE_RESOLVE_ON_IPS */
-
 #ifdef CURLRES_IPV6
   /*
    * Check if a limited name resolve has been requested.

lib/hostip.c

@@ -59,6 +59,7 @@
 #include "strerror.h"
 #include "url.h"
 #include "inet_ntop.h"
+#include "inet_pton.h"
 #include "multiif.h"
 #include "doh.h"
 #include "warnless.h"
@@ -512,13 +513,11 @@ int Curl_resolv(struct connectdata *conn,
   if(!dns) {
     /* The entry was not in the cache. Resolve it to IP address */
 
-    Curl_addrinfo *addr;
+    Curl_addrinfo *addr = NULL;
     int respwait = 0;
-
+#ifndef USE_RESOLVE_ON_IPS
-    /* Check what IP specifics the app has requested and if we can provide it.
+    struct in_addr in;
-     * If not, bail out. */
+#endif
-    if(!Curl_ipvalid(conn))
-      return CURLRESOLV_ERROR;
 
     /* notify the resolver start callback */
     if(data->set.resolver_start) {
@@ -531,20 +530,43 @@ int Curl_resolv(struct connectdata *conn,
         return CURLRESOLV_ERROR;
     }
 
-    if(allowDOH && data->set.doh) {
+#ifndef USE_RESOLVE_ON_IPS
-      addr = Curl_doh(conn, hostname, port, &respwait);
+    /* First check if this is an IPv4 address string */
+    if(Curl_inet_pton(AF_INET, hostname, &in) > 0)
+      /* This is a dotted IP address 123.123.123.123-style */
+      addr = Curl_ip2addr(AF_INET, &in, hostname, port);
+#ifdef ENABLE_IPV6
+    if(!addr) {
+      struct in6_addr in6;
+      /* check if this is an IPv6 address string */
+      if(Curl_inet_pton(AF_INET6, hostname, &in6) > 0)
+        /* This is an IPv6 address literal */
+        addr = Curl_ip2addr(AF_INET6, &in6, hostname, port);
     }
-    else {
+#endif /* ENABLE_IPV6 */
-      /* If Curl_getaddrinfo() returns NULL, 'respwait' might be set to a
+#endif /* !USE_RESOLVE_ON_IPS */
-         non-zero value indicating that we need to wait for the response to the
+
-         resolve call */
+    if(!addr) {
-      addr = Curl_getaddrinfo(conn,
+      /* Check what IP specifics the app has requested and if we can provide
+       * it. If not, bail out. */
+      if(!Curl_ipvalid(conn))
+        return CURLRESOLV_ERROR;
+
+      if(allowDOH && data->set.doh) {
+        addr = Curl_doh(conn, hostname, port, &respwait);
+      }
+      else {
+        /* If Curl_getaddrinfo() returns NULL, 'respwait' might be set to a
+           non-zero value indicating that we need to wait for the response to
+           the resolve call */
+        addr = Curl_getaddrinfo(conn,
 #ifdef DEBUGBUILD
-                              (data->set.str[STRING_DEVICE]
+                                (data->set.str[STRING_DEVICE]
-                               && !strcmp(data->set.str[STRING_DEVICE],
+                                 && !strcmp(data->set.str[STRING_DEVICE],
-                                          "LocalHost"))?"localhost":
+                                            "LocalHost"))?"localhost":
 #endif
-                              hostname, port, &respwait);
+                                hostname, port, &respwait);
+      }
     }
     if(!addr) {
       if(respwait) {

lib/hostip4.c

@@ -52,7 +52,6 @@
 #include "share.h"
 #include "strerror.h"
 #include "url.h"
-#include "inet_pton.h"
 /* The last 3 #include files should be in this order */
 #include "curl_printf.h"
 #include "curl_memory.h"
@@ -128,38 +127,22 @@ Curl_addrinfo *Curl_ipv4_resolve_r(const char *hostname,
 #endif
   Curl_addrinfo *ai = NULL;
   struct hostent *h = NULL;
-  struct in_addr in;
   struct hostent *buf = NULL;
 
-#ifdef ENABLE_IPV6
-  {
-    struct in6_addr in6;
-    /* check if this is an IPv6 address string */
-    if(Curl_inet_pton(AF_INET6, hostname, &in6) > 0)
-      /* This is an IPv6 address literal */
-      return Curl_ip2addr(AF_INET6, &in6, hostname, port);
-  }
-#endif /* ENABLE_IPV6 */
-
-  if(Curl_inet_pton(AF_INET, hostname, &in) > 0)
-    /* This is a dotted IP address 123.123.123.123-style */
-    return Curl_ip2addr(AF_INET, &in, hostname, port);
-
 #if defined(HAVE_GETADDRINFO_THREADSAFE)
-  else {
+  struct addrinfo hints;
-    struct addrinfo hints;
+  char sbuf[12];
-    char sbuf[12];
+  char *sbufptr = NULL;
-    char *sbufptr = NULL;
 
-    memset(&hints, 0, sizeof(hints));
+  memset(&hints, 0, sizeof(hints));
-    hints.ai_family = PF_INET;
+  hints.ai_family = PF_INET;
-    hints.ai_socktype = SOCK_STREAM;
+  hints.ai_socktype = SOCK_STREAM;
-    if(port) {
+  if(port) {
-      msnprintf(sbuf, sizeof(sbuf), "%d", port);
+    msnprintf(sbuf, sizeof(sbuf), "%d", port);
-      sbufptr = sbuf;
+    sbufptr = sbuf;
-    }
+  }
 
-    (void)Curl_getaddrinfo_ex(hostname, sbufptr, &hints, &ai);
+  (void)Curl_getaddrinfo_ex(hostname, sbufptr, &hints, &ai);
 
 #elif defined(HAVE_GETHOSTBYNAME_R)
   /*
@@ -167,144 +150,141 @@ Curl_addrinfo *Curl_ipv4_resolve_r(const char *hostname,
    * Since there are three different versions of it, the following code is
    * somewhat #ifdef-ridden.
    */
-  else {
+  int h_errnop;
-    int h_errnop;
 
-    buf = calloc(1, CURL_HOSTENT_SIZE);
+  buf = calloc(1, CURL_HOSTENT_SIZE);
-    if(!buf)
+  if(!buf)
-      return NULL; /* major failure */
+    return NULL; /* major failure */
-    /*
+  /*
-     * The clearing of the buffer is a workaround for a gethostbyname_r bug in
+   * The clearing of the buffer is a workaround for a gethostbyname_r bug in
-     * qnx nto and it is also _required_ for some of these functions on some
+   * qnx nto and it is also _required_ for some of these functions on some
-     * platforms.
+   * platforms.
-     */
+   */
 
 #if defined(HAVE_GETHOSTBYNAME_R_5)
-    /* Solaris, IRIX and more */
+  /* Solaris, IRIX and more */
-    h = gethostbyname_r(hostname,
+  h = gethostbyname_r(hostname,
-                        (struct hostent *)buf,
+                      (struct hostent *)buf,
-                        (char *)buf + sizeof(struct hostent),
+                      (char *)buf + sizeof(struct hostent),
-                        CURL_HOSTENT_SIZE - sizeof(struct hostent),
+                      CURL_HOSTENT_SIZE - sizeof(struct hostent),
-                        &h_errnop);
+                      &h_errnop);
 
-    /* If the buffer is too small, it returns NULL and sets errno to
+  /* If the buffer is too small, it returns NULL and sets errno to
-     * ERANGE. The errno is thread safe if this is compiled with
+   * ERANGE. The errno is thread safe if this is compiled with
-     * -D_REENTRANT as then the 'errno' variable is a macro defined to get
+   * -D_REENTRANT as then the 'errno' variable is a macro defined to get
-     * used properly for threads.
+   * used properly for threads.
-     */
+   */
 
-    if(h) {
+  if(h) {
-      ;
+    ;
-    }
-    else
-#elif defined(HAVE_GETHOSTBYNAME_R_6)
-    /* Linux */
-
-    (void)gethostbyname_r(hostname,
-                        (struct hostent *)buf,
-                        (char *)buf + sizeof(struct hostent),
-                        CURL_HOSTENT_SIZE - sizeof(struct hostent),
-                        &h, /* DIFFERENCE */
-                        &h_errnop);
-    /* Redhat 8, using glibc 2.2.93 changed the behavior. Now all of a
-     * sudden this function returns EAGAIN if the given buffer size is too
-     * small. Previous versions are known to return ERANGE for the same
-     * problem.
-     *
-     * This wouldn't be such a big problem if older versions wouldn't
-     * sometimes return EAGAIN on a common failure case. Alas, we can't
-     * assume that EAGAIN *or* ERANGE means ERANGE for any given version of
-     * glibc.
-     *
-     * For now, we do that and thus we may call the function repeatedly and
-     * fail for older glibc versions that return EAGAIN, until we run out of
-     * buffer size (step_size grows beyond CURL_HOSTENT_SIZE).
-     *
-     * If anyone has a better fix, please tell us!
-     *
-     * -------------------------------------------------------------------
-     *
-     * On October 23rd 2003, Dan C dug up more details on the mysteries of
-     * gethostbyname_r() in glibc:
-     *
-     * In glibc 2.2.5 the interface is different (this has also been
-     * discovered in glibc 2.1.1-6 as shipped by Redhat 6). What I can't
-     * explain, is that tests performed on glibc 2.2.4-34 and 2.2.4-32
-     * (shipped/upgraded by Redhat 7.2) don't show this behavior!
-     *
-     * In this "buggy" version, the return code is -1 on error and 'errno'
-     * is set to the ERANGE or EAGAIN code. Note that 'errno' is not a
-     * thread-safe variable.
-     */
-
-    if(!h) /* failure */
-#elif defined(HAVE_GETHOSTBYNAME_R_3)
-    /* AIX, Digital Unix/Tru64, HPUX 10, more? */
-
-    /* For AIX 4.3 or later, we don't use gethostbyname_r() at all, because of
-     * the plain fact that it does not return unique full buffers on each
-     * call, but instead several of the pointers in the hostent structs will
-     * point to the same actual data! This have the unfortunate down-side that
-     * our caching system breaks down horribly. Luckily for us though, AIX 4.3
-     * and more recent versions have a "completely thread-safe"[*] libc where
-     * all the data is stored in thread-specific memory areas making calls to
-     * the plain old gethostbyname() work fine even for multi-threaded
-     * programs.
-     *
-     * This AIX 4.3 or later detection is all made in the configure script.
-     *
-     * Troels Walsted Hansen helped us work this out on March 3rd, 2003.
-     *
-     * [*] = much later we've found out that it isn't at all "completely
-     * thread-safe", but at least the gethostbyname() function is.
-     */
-
-    if(CURL_HOSTENT_SIZE >=
-       (sizeof(struct hostent) + sizeof(struct hostent_data))) {
-
-      /* August 22nd, 2000: Albert Chin-A-Young brought an updated version
-       * that should work! September 20: Richard Prescott worked on the buffer
-       * size dilemma.
-       */
-
-      res = gethostbyname_r(hostname,
-                            (struct hostent *)buf,
-                            (struct hostent_data *)((char *)buf +
-                                                    sizeof(struct hostent)));
-      h_errnop = SOCKERRNO; /* we don't deal with this, but set it anyway */
-    }
-    else
-      res = -1; /* failure, too smallish buffer size */
-
-    if(!res) { /* success */
-
-      h = buf; /* result expected in h */
-
-      /* This is the worst kind of the different gethostbyname_r() interfaces.
-       * Since we don't know how big buffer this particular lookup required,
-       * we can't realloc down the huge alloc without doing closer analysis of
-       * the returned data. Thus, we always use CURL_HOSTENT_SIZE for every
-       * name lookup. Fixing this would require an extra malloc() and then
-       * calling Curl_addrinfo_copy() that subsequent realloc()s down the new
-       * memory area to the actually used amount.
-       */
-    }
-    else
-#endif /* HAVE_...BYNAME_R_5 || HAVE_...BYNAME_R_6 || HAVE_...BYNAME_R_3 */
-    {
-      h = NULL; /* set return code to NULL */
-      free(buf);
-    }
-#else /* HAVE_GETADDRINFO_THREADSAFE || HAVE_GETHOSTBYNAME_R */
-    /*
-     * Here is code for platforms that don't have a thread safe
-     * getaddrinfo() nor gethostbyname_r() function or for which
-     * gethostbyname() is the preferred one.
-     */
-  else {
-    h = gethostbyname((void *)hostname);
-#endif /* HAVE_GETADDRINFO_THREADSAFE || HAVE_GETHOSTBYNAME_R */
   }
+  else
+#elif defined(HAVE_GETHOSTBYNAME_R_6)
+  /* Linux */
+
+  (void)gethostbyname_r(hostname,
+                      (struct hostent *)buf,
+                      (char *)buf + sizeof(struct hostent),
+                      CURL_HOSTENT_SIZE - sizeof(struct hostent),
+                      &h, /* DIFFERENCE */
+                      &h_errnop);
+  /* Redhat 8, using glibc 2.2.93 changed the behavior. Now all of a
+   * sudden this function returns EAGAIN if the given buffer size is too
+   * small. Previous versions are known to return ERANGE for the same
+   * problem.
+   *
+   * This wouldn't be such a big problem if older versions wouldn't
+   * sometimes return EAGAIN on a common failure case. Alas, we can't
+   * assume that EAGAIN *or* ERANGE means ERANGE for any given version of
+   * glibc.
+   *
+   * For now, we do that and thus we may call the function repeatedly and
+   * fail for older glibc versions that return EAGAIN, until we run out of
+   * buffer size (step_size grows beyond CURL_HOSTENT_SIZE).
+   *
+   * If anyone has a better fix, please tell us!
+   *
+   * -------------------------------------------------------------------
+   *
+   * On October 23rd 2003, Dan C dug up more details on the mysteries of
+   * gethostbyname_r() in glibc:
+   *
+   * In glibc 2.2.5 the interface is different (this has also been
+   * discovered in glibc 2.1.1-6 as shipped by Redhat 6). What I can't
+   * explain, is that tests performed on glibc 2.2.4-34 and 2.2.4-32
+   * (shipped/upgraded by Redhat 7.2) don't show this behavior!
+   *
+   * In this "buggy" version, the return code is -1 on error and 'errno'
+   * is set to the ERANGE or EAGAIN code. Note that 'errno' is not a
+   * thread-safe variable.
+   */
+
+  if(!h) /* failure */
+#elif defined(HAVE_GETHOSTBYNAME_R_3)
+  /* AIX, Digital Unix/Tru64, HPUX 10, more? */
+
+  /* For AIX 4.3 or later, we don't use gethostbyname_r() at all, because of
+   * the plain fact that it does not return unique full buffers on each
+   * call, but instead several of the pointers in the hostent structs will
+   * point to the same actual data! This have the unfortunate down-side that
+   * our caching system breaks down horribly. Luckily for us though, AIX 4.3
+   * and more recent versions have a "completely thread-safe"[*] libc where
+   * all the data is stored in thread-specific memory areas making calls to
+   * the plain old gethostbyname() work fine even for multi-threaded
+   * programs.
+   *
+   * This AIX 4.3 or later detection is all made in the configure script.
+   *
+   * Troels Walsted Hansen helped us work this out on March 3rd, 2003.
+   *
+   * [*] = much later we've found out that it isn't at all "completely
+   * thread-safe", but at least the gethostbyname() function is.
+   */
+
+  if(CURL_HOSTENT_SIZE >=
+     (sizeof(struct hostent) + sizeof(struct hostent_data))) {
+
+    /* August 22nd, 2000: Albert Chin-A-Young brought an updated version
+     * that should work! September 20: Richard Prescott worked on the buffer
+     * size dilemma.
+     */
+
+    res = gethostbyname_r(hostname,
+                          (struct hostent *)buf,
+                          (struct hostent_data *)((char *)buf +
+                                                  sizeof(struct hostent)));
+    h_errnop = SOCKERRNO; /* we don't deal with this, but set it anyway */
+  }
+  else
+    res = -1; /* failure, too smallish buffer size */
+
+  if(!res) { /* success */
+
+    h = buf; /* result expected in h */
+
+    /* This is the worst kind of the different gethostbyname_r() interfaces.
+     * Since we don't know how big buffer this particular lookup required,
+     * we can't realloc down the huge alloc without doing closer analysis of
+     * the returned data. Thus, we always use CURL_HOSTENT_SIZE for every
+     * name lookup. Fixing this would require an extra malloc() and then
+     * calling Curl_addrinfo_copy() that subsequent realloc()s down the new
+     * memory area to the actually used amount.
+     */
+  }
+  else
+#endif /* HAVE_...BYNAME_R_5 || HAVE_...BYNAME_R_6 || HAVE_...BYNAME_R_3 */
+  {
+    h = NULL; /* set return code to NULL */
+    free(buf);
+  }
+#else /* HAVE_GETADDRINFO_THREADSAFE || HAVE_GETHOSTBYNAME_R */
+  /*
+   * Here is code for platforms that don't have a thread safe
+   * getaddrinfo() nor gethostbyname_r() function or for which
+   * gethostbyname() is the preferred one.
+   */
+  h = gethostbyname((void *)hostname);
+#endif /* HAVE_GETADDRINFO_THREADSAFE || HAVE_GETHOSTBYNAME_R */
 
   if(h) {
     ai = Curl_he2ai(h, port);