From 875314ed0bf3b7c156e8762f30e1a2c11fb301ba Mon Sep 17 00:00:00 2001 From: Marcel Raad Date: Mon, 6 Jan 2020 12:56:44 +0100 Subject: 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 | 15 --- lib/asyn-thread.c | 35 ------- lib/hostip.c | 56 +++++++---- lib/hostip4.c | 292 +++++++++++++++++++++++++----------------------------- 4 files changed, 175 insertions(+), 223 deletions(-) diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c index 835cfa48f..afa039c6b 100644 --- a/lib/asyn-ares.c +++ b/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 diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c index b08497aaa..3e39de312 100644 --- a/lib/asyn-thread.c +++ b/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. diff --git a/lib/hostip.c b/lib/hostip.c index b434b390a..6c05ffb7d 100644 --- a/lib/hostip.c +++ b/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; - - /* 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; +#ifndef USE_RESOLVE_ON_IPS + struct in_addr in; +#endif /* 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) { - addr = Curl_doh(conn, hostname, port, &respwait); +#ifndef USE_RESOLVE_ON_IPS + /* 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 { - /* 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, +#endif /* ENABLE_IPV6 */ +#endif /* !USE_RESOLVE_ON_IPS */ + + if(!addr) { + /* 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] - && !strcmp(data->set.str[STRING_DEVICE], - "LocalHost"))?"localhost": + (data->set.str[STRING_DEVICE] + && !strcmp(data->set.str[STRING_DEVICE], + "LocalHost"))?"localhost": #endif - hostname, port, &respwait); + hostname, port, &respwait); + } } if(!addr) { if(respwait) { diff --git a/lib/hostip4.c b/lib/hostip4.c index 2636851e6..c6fc68fcc 100644 --- a/lib/hostip4.c +++ b/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; - char sbuf[12]; - char *sbufptr = NULL; - - memset(&hints, 0, sizeof(hints)); - hints.ai_family = PF_INET; - hints.ai_socktype = SOCK_STREAM; - if(port) { - msnprintf(sbuf, sizeof(sbuf), "%d", port); - sbufptr = sbuf; - } + struct addrinfo hints; + char sbuf[12]; + char *sbufptr = NULL; + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = PF_INET; + hints.ai_socktype = SOCK_STREAM; + if(port) { + msnprintf(sbuf, sizeof(sbuf), "%d", port); + 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; - - buf = calloc(1, CURL_HOSTENT_SIZE); - if(!buf) - return NULL; /* major failure */ - /* - * 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 - * platforms. - */ + int h_errnop; + + buf = calloc(1, CURL_HOSTENT_SIZE); + if(!buf) + return NULL; /* major failure */ + /* + * 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 + * platforms. + */ #if defined(HAVE_GETHOSTBYNAME_R_5) - /* Solaris, IRIX and more */ - h = gethostbyname_r(hostname, - (struct hostent *)buf, - (char *)buf + sizeof(struct hostent), - CURL_HOSTENT_SIZE - sizeof(struct hostent), - &h_errnop); - - /* If the buffer is too small, it returns NULL and sets errno to - * ERANGE. The errno is thread safe if this is compiled with - * -D_REENTRANT as then the 'errno' variable is a macro defined to get - * used properly for threads. - */ + /* Solaris, IRIX and more */ + h = gethostbyname_r(hostname, + (struct hostent *)buf, + (char *)buf + sizeof(struct hostent), + CURL_HOSTENT_SIZE - sizeof(struct hostent), + &h_errnop); + + /* If the buffer is too small, it returns NULL and sets errno to + * ERANGE. The errno is thread safe if this is compiled with + * -D_REENTRANT as then the 'errno' variable is a macro defined to get + * used properly for threads. + */ - if(h) { - ; - } - else + 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. - */ + /* 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 */ + 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. + /* 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. */ - 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 + 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); - } + { + 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); + /* + * 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); -- cgit v1.2.1