Skip to content

Commit

Permalink
hostip: Cache negative name resolves
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zagor committed Dec 14, 2023
1 parent d21bd21 commit 25d0422
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 143 deletions.
38 changes: 16 additions & 22 deletions lib/hostasyn.c
Expand Up @@ -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
Expand Down
140 changes: 83 additions & 57 deletions lib/hostip.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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)
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 &&
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/*
Expand Down Expand Up @@ -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;
}

Expand Down
13 changes: 0 additions & 13 deletions lib/hostip.h
Expand Up @@ -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.
*
Expand Down
26 changes: 1 addition & 25 deletions lib/multi.c
Expand Up @@ -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
Expand Down
35 changes: 9 additions & 26 deletions lib/socks.c
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 25d0422

Please sign in to comment.