From 25d0422dade068abe7931b36713e6b0bd771a36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Stenberg?= Date: Thu, 14 Dec 2023 15:50:58 +0100 Subject: [PATCH] hostip: Cache negative name resolves TODO 1.9: Add failed name resolves to the host cache, so that repeated connection attempts do not immediately repeat the resolve request. Lifetime of negative entries is set to half of the positive ones. Refactor the hostip api so that Curl_resolv_check() also checks the cache, removing the need for Curl_fetch_addr(). This way hostip callers only need one function, and don't care if the result came from cache or not. --- lib/hostasyn.c | 38 ++++++-------- lib/hostip.c | 140 +++++++++++++++++++++++++++++-------------------- lib/hostip.h | 13 ----- lib/multi.c | 26 +-------- lib/socks.c | 35 ++++--------- 5 files changed, 109 insertions(+), 143 deletions(-) diff --git a/lib/hostasyn.c b/lib/hostasyn.c index faf01c5f4c030a..c9cfc5f47423c6 100644 --- a/lib/hostasyn.c +++ b/lib/hostasyn.c @@ -73,33 +73,27 @@ CURLcode Curl_addrinfo_callback(struct Curl_easy *data, conn->resolve_async.status = status; - if(CURL_ASYNC_SUCCESS == status) { - if(ai) { - if(data->share) - Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); + if(data->share) + Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); - dns = Curl_cache_addr(data, ai, - conn->resolve_async.hostname, 0, - conn->resolve_async.port); - if(data->share) - Curl_share_unlock(data, CURL_LOCK_DATA_DNS); + dns = Curl_cache_addr(data, ai, + conn->resolve_async.hostname, 0, + conn->resolve_async.port); + if(data->share) + Curl_share_unlock(data, CURL_LOCK_DATA_DNS); - if(!dns) { - /* failed to store, cleanup and return error */ - Curl_freeaddrinfo(ai); - result = CURLE_OUT_OF_MEMORY; - } - } - else { - result = CURLE_OUT_OF_MEMORY; - } + if(!dns) { + /* failed to store, cleanup and return error */ + Curl_freeaddrinfo(ai); + result = CURLE_OUT_OF_MEMORY; } - conn->resolve_async.dns = dns; + if(ai) + conn->resolve_async.dns = dns; - /* Set async.done TRUE last in this function since it may be used multi- - threaded and once this is TRUE the other thread may read fields from the - async struct */ + /* Set async.done TRUE last in this function since it may be used multi- + threaded and once this is TRUE the other thread may read fields from the + async struct */ conn->resolve_async.done = TRUE; /* IPv4: The input hostent struct will be freed by ares when we return from diff --git a/lib/hostip.c b/lib/hostip.c index e7c318af77f50e..b669182be466f1 100644 --- a/lib/hostip.c +++ b/lib/hostip.c @@ -203,7 +203,10 @@ hostcache_timestamp_remove(void *datap, void *hc) if(c->timestamp) { /* age in seconds */ time_t age = prune->now - c->timestamp; - if(age >= prune->cache_timeout) + int lifetime = prune->cache_timeout; + if(!c->addr) + lifetime /= 2; /* negative entries get half lifetime */ + if(age >= lifetime) return TRUE; if(age > prune->oldest) prune->oldest = age; @@ -279,7 +282,8 @@ static curl_simple_lock curl_jmpenv_lock; /* lookup address, returns entry if found and not stale */ static struct Curl_dns_entry *fetch_addr(struct Curl_easy *data, const char *hostname, - int port) + int port, + bool *name_found) { struct Curl_dns_entry *dns = NULL; char entry_id[MAX_HOSTCACHE_LEN]; @@ -315,7 +319,7 @@ static struct Curl_dns_entry *fetch_addr(struct Curl_easy *data, } /* See if the returned entry matches the required resolve mode */ - if(dns && data->conn->ip_version != CURL_IPRESOLVE_WHATEVER) { + if(dns && dns->addr && data->conn->ip_version != CURL_IPRESOLVE_WHATEVER) { int pf = PF_INET; bool found = false; struct Curl_addrinfo *addr = dns->addr; @@ -339,41 +343,12 @@ static struct Curl_dns_entry *fetch_addr(struct Curl_easy *data, Curl_hash_delete(data->dns.hostcache, entry_id, entry_len + 1); } } - return dns; -} - -/* - * Curl_fetch_addr() fetches a 'Curl_dns_entry' already in the DNS cache. - * - * Curl_resolv() checks initially and multi_runsingle() checks each time - * it discovers the handle in the state WAITRESOLVE whether the hostname - * has already been resolved and the address has already been stored in - * the DNS cache. This short circuits waiting for a lot of pending - * lookups for the same hostname requested by different handles. - * - * Returns the Curl_dns_entry entry pointer or NULL if not in the cache. - * - * The returned data *MUST* be "unlocked" with Curl_resolv_unlock() after - * use, or we'll leak memory! - */ -struct Curl_dns_entry * -Curl_fetch_addr(struct Curl_easy *data, - const char *hostname, - int port) -{ - struct Curl_dns_entry *dns = NULL; - - if(data->share) - Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); - - dns = fetch_addr(data, hostname, port); - - if(dns) - dns->inuse++; /* we use it! */ - - if(data->share) - Curl_share_unlock(data, CURL_LOCK_DATA_DNS); + if(dns) { + *name_found = true; + if(!dns->addr) + dns = NULL; + } return dns; } @@ -501,7 +476,8 @@ Curl_cache_addr(struct Curl_easy *data, entry_len = create_hostcache_id(hostname, hostlen, port, entry_id, sizeof(entry_id)); - dns->inuse = 1; /* the cache has the first reference */ + if(addr) + dns->inuse = 1; /* the cache has the first reference */ dns->addr = addr; /* this is the address(es) */ time(&dns->timestamp); if(dns->timestamp == 0) @@ -519,7 +495,8 @@ Curl_cache_addr(struct Curl_easy *data, } dns = dns2; - dns->inuse++; /* mark entry as in-use */ + if(addr) + dns->inuse++; /* mark entry as in-use */ return dns; } @@ -691,6 +668,8 @@ enum resolve_t Curl_resolv(struct Curl_easy *data, CURLcode result; enum resolve_t rc = CURLRESOLV_ERROR; /* default to failure */ struct connectdata *conn = data->conn; + bool found = false; + /* We should intentionally error and not resolve .onion TLDs */ size_t hostname_len = strlen(hostname); if(hostname_len >= 7 && @@ -709,18 +688,22 @@ enum resolve_t Curl_resolv(struct Curl_easy *data, if(data->share) Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); - dns = fetch_addr(data, hostname, port); + dns = fetch_addr(data, hostname, port, &found); if(dns) { infof(data, "Hostname %s was found in DNS cache", hostname); dns->inuse++; /* we use it! */ rc = CURLRESOLV_RESOLVED; } + else if(found) { + infof(data, "Hostname %s was found negative in DNS cache", hostname); + rc = CURLRESOLV_ERROR; + } if(data->share) Curl_share_unlock(data, CURL_LOCK_DATA_DNS); - if(!dns) { + if(!found) { /* The entry was not in the cache. Resolve it to IP address */ struct Curl_addrinfo *addr = NULL; @@ -1059,13 +1042,18 @@ void Curl_resolv_unlock(struct Curl_easy *data, struct Curl_dns_entry *dns) static void freednsentry(void *freethis) { struct Curl_dns_entry *dns = (struct Curl_dns_entry *) freethis; - DEBUGASSERT(dns && (dns->inuse>0)); + DEBUGASSERT(dns); + if(dns->addr) { + DEBUGASSERT(dns->inuse>0); - dns->inuse--; - if(dns->inuse == 0) { - Curl_freeaddrinfo(dns->addr); - free(dns); + dns->inuse--; + if(dns->inuse == 0) { + Curl_freeaddrinfo(dns->addr); + free(dns); + } } + else + free(dns); } /* @@ -1366,23 +1354,61 @@ static void show_resolve_info(struct Curl_easy *data, } #endif +static CURLcode check_cache(struct Curl_easy *data, + struct Curl_dns_entry **dns, + bool *found) +{ + CURLcode result = CURLE_OK; + +#ifdef CURLRES_ASYNCH + if(data->conn->resolve_async.hostname) { + if(data->share) + Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); + + *dns = fetch_addr(data, data->conn->resolve_async.hostname, + data->conn->port, found); + if(*found) { + if(*dns) + (*dns)->inuse++; + else { +#ifndef CURL_DISABLE_PROXY + if(data->conn->bits.httpproxy) + result = CURLE_COULDNT_RESOLVE_PROXY; +#endif + result = CURLE_COULDNT_RESOLVE_HOST; + } + } + if(data->share) + Curl_share_lock(data, CURL_LOCK_DATA_DNS, CURL_LOCK_ACCESS_SINGLE); + } +#endif + return result; +} + CURLcode Curl_resolv_check(struct Curl_easy *data, struct Curl_dns_entry **dns) { CURLcode result; -#if defined(CURL_DISABLE_DOH) && !defined(CURLRES_ASYNCH) - (void)data; - (void)dns; -#endif + bool found = false; + + result = check_cache(data, dns, &found); + if(!found) { #ifndef CURL_DISABLE_DOH - if(data->conn->bits.doh) { - result = Curl_doh_is_resolved(data, dns); - } - else + if(data->conn->bits.doh) { + result = Curl_doh_is_resolved(data, dns); + } + else +#endif + result = Curl_resolver_is_resolved(data, dns); + if(*dns) { + show_resolve_info(data, *dns); +#ifdef CURLRES_ASYNCH + data->conn->resolve_async.dns = *dns; + data->conn->resolve_async.done = TRUE; #endif - result = Curl_resolver_is_resolved(data, dns); - if(*dns) - show_resolve_info(data, *dns); + } + } + return result; } diff --git a/lib/hostip.h b/lib/hostip.h index fb53a5776bcb87..69b8b03c34d7cf 100644 --- a/lib/hostip.h +++ b/lib/hostip.h @@ -157,19 +157,6 @@ CURLcode Curl_addrinfo_callback(struct Curl_easy *data, void Curl_printable_address(const struct Curl_addrinfo *ip, char *buf, size_t bufsize); -/* - * Curl_fetch_addr() fetches a 'Curl_dns_entry' already in the DNS cache. - * - * Returns the Curl_dns_entry entry pointer or NULL if not in the cache. - * - * The returned data *MUST* be "unlocked" with Curl_resolv_unlock() after - * use, or we'll leak memory! - */ -struct Curl_dns_entry * -Curl_fetch_addr(struct Curl_easy *data, - const char *hostname, - int port); - /* * Curl_cache_addr() stores a 'Curl_addrinfo' struct in the DNS cache. * diff --git a/lib/multi.c b/lib/multi.c index 53dee05e00a124..9ee4e0ff2911a2 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1965,33 +1965,9 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, { struct Curl_dns_entry *dns = NULL; struct connectdata *conn = data->conn; - const char *hostname; DEBUGASSERT(conn); -#ifndef CURL_DISABLE_PROXY - if(conn->bits.httpproxy) - hostname = conn->http_proxy.host.name; - else -#endif - if(conn->bits.conn_to_host) - hostname = conn->conn_to_host.name; - else - hostname = conn->host.name; - - /* check if we have the name resolved by now */ - dns = Curl_fetch_addr(data, hostname, (int)conn->port); - - if(dns) { -#ifdef CURLRES_ASYNCH - conn->resolve_async.dns = dns; - conn->resolve_async.done = TRUE; -#endif - result = CURLE_OK; - infof(data, "Hostname '%s' was found in DNS cache", hostname); - } - - if(!dns) - result = Curl_resolv_check(data, &dns); + result = Curl_resolv_check(data, &dns); /* Update sockets here, because the socket(s) may have been closed and the application thus needs to be told, even if it diff --git a/lib/socks.c b/lib/socks.c index 3a396de620ec6c..47d9234e1df784 100644 --- a/lib/socks.c +++ b/lib/socks.c @@ -335,23 +335,15 @@ static CURLproxycode do_SOCKS4(struct Curl_cfilter *cf, case CONNECT_RESOLVING: /* check if we have the name resolved by now */ - dns = Curl_fetch_addr(data, sx->hostname, (int)conn->port); - + result = Curl_resolv_check(data, &dns); if(dns) { -#ifdef CURLRES_ASYNCH - conn->resolve_async.dns = dns; - conn->resolve_async.done = TRUE; -#endif infof(data, "Hostname '%s' was found", sx->hostname); sxstate(sx, data, CONNECT_RESOLVED); } else { - result = Curl_resolv_check(data, &dns); - if(!dns) { - if(result) - return CURLPX_RESOLVE_HOST; - return CURLPX_OK; - } + if(result) + return CURLPX_RESOLVE_HOST; + return CURLPX_OK; } /* FALLTHROUGH */ CONNECT_RESOLVED: @@ -802,23 +794,14 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf, case CONNECT_RESOLVING: /* check if we have the name resolved by now */ - dns = Curl_fetch_addr(data, sx->hostname, sx->remote_port); - + result = Curl_resolv_check(data, &dns); if(dns) { -#ifdef CURLRES_ASYNCH - conn->resolve_async.dns = dns; - conn->resolve_async.done = TRUE; -#endif infof(data, "SOCKS5: hostname '%s' found", sx->hostname); } - - if(!dns) { - result = Curl_resolv_check(data, &dns); - if(!dns) { - if(result) - return CURLPX_RESOLVE_HOST; - return CURLPX_OK; - } + else { + if(result) + return CURLPX_RESOLVE_HOST; + return CURLPX_OK; } /* FALLTHROUGH */ CONNECT_RESOLVED: