hostip: only cache negative resolves for authoritative answers

Closes #22302
This commit is contained in:
Graham Campbell 2026-07-12 03:58:18 +01:00 committed by Daniel Stenberg
parent 353d05ef0c
commit c3ae9ef822
No known key found for this signature in database
GPG key ID: 5CC908FDB71E12C2
13 changed files with 253 additions and 22 deletions

View file

@ -308,6 +308,12 @@ CURLcode Curl_async_take_result(struct Curl_easy *data,
* CURLE_COULDNT_RESOLVE_* code */
if(!result && !*pdns) {
const char *msg = NULL;
/* only an authoritative "does not exist" answer from every query
may be cached as a negative entry, not transient failures like
timeouts or server troubles */
async->negative_answer = !ares->transient_err &&
((ares->ares_status == ARES_ENOTFOUND) ||
(ares->ares_status == ARES_ENODATA));
if(ares->ares_status != ARES_SUCCESS)
msg = ares_strerror(ares->ares_status);
result = Curl_async_failed(data, async, msg);
@ -569,8 +575,12 @@ static void async_ares_A_cb(void *user_data, int status, int timeouts,
ares->res_A = async_ares_node2addr(ares_ai->nodes);
ares_freeaddrinfo(ares_ai);
}
else if(ares->ares_status != ARES_SUCCESS) /* do not overwrite success */
ares->ares_status = status;
else {
if((status != ARES_ENOTFOUND) && (status != ARES_ENODATA))
ares->transient_err = TRUE;
if(ares->ares_status != ARES_SUCCESS) /* do not overwrite success */
ares->ares_status = status;
}
}
#ifdef CURLRES_IPV6
@ -592,8 +602,12 @@ static void async_ares_AAAA_cb(void *user_data, int status, int timeouts,
ares->res_AAAA = async_ares_node2addr(ares_ai->nodes);
ares_freeaddrinfo(ares_ai);
}
else if(ares->ares_status != ARES_SUCCESS) /* do not overwrite success */
ares->ares_status = status;
else {
if((status != ARES_ENOTFOUND) && (status != ARES_ENODATA))
ares->transient_err = TRUE;
if(ares->ares_status != ARES_SUCCESS) /* do not overwrite success */
ares->ares_status = status;
}
}
#endif /* CURLRES_IPV6 */

View file

@ -117,9 +117,11 @@ struct async_thrdd_item {
uint16_t port;
uint8_t transport;
uint8_t dns_queries;
BIT(negative); /* resolver answered that the name does not exist */
#ifdef DEBUGBUILD
uint32_t delay_ms;
uint32_t delay_fail_ms;
BIT(dbg_negative);
#endif
char hostname[1];
};
@ -182,6 +184,8 @@ static struct async_thrdd_item *async_thrdd_item_create(
item->delay_fail_ms = (uint32_t)l + c;
}
}
if(getenv("CURL_DBG_RESOLV_FAIL_NEGATIVE"))
item->dbg_negative = TRUE;
}
#endif
@ -331,6 +335,26 @@ CURLcode Curl_async_await(struct Curl_easy *data, uint32_t resolv_id,
#ifdef HAVE_GETADDRINFO
/* Was the getaddrinfo() failure an authoritative negative answer,
i.e. the resolver responded that the name (or its data) does not
exist? Transient failures like EAI_AGAIN and local troubles must
not count as negative answers. */
static bool gai_negative(int rc)
{
switch(rc) {
#ifdef EAI_NONAME
case EAI_NONAME:
#endif
#if defined(EAI_NODATA) && \
(!defined(EAI_NONAME) || (EAI_NODATA != EAI_NONAME))
case EAI_NODATA:
#endif
return TRUE;
default:
return FALSE;
}
}
/* Process the item, using Curl_getaddrinfo_ex() */
static void async_thrdd_item_process(void *arg)
{
@ -346,6 +370,7 @@ static void async_thrdd_item_process(void *arg)
}
if(item->delay_fail_ms) {
curlx_wait_ms(item->delay_fail_ms);
item->negative = item->dbg_negative;
return;
}
#endif
@ -375,6 +400,7 @@ static void async_thrdd_item_process(void *arg)
item->sockerr = SOCKERRNO ? SOCKERRNO : rc;
if(item->sockerr == 0)
item->sockerr = RESOLVER_ENOMEM;
item->negative = gai_negative(rc);
}
else {
Curl_addrinfo_set_port(item->res, item->port);
@ -394,6 +420,7 @@ static void async_thrdd_item_process(void *arg)
}
if(item->delay_fail_ms) {
curlx_wait_ms(item->delay_fail_ms);
item->negative = item->dbg_negative;
return;
}
#endif
@ -402,6 +429,9 @@ static void async_thrdd_item_process(void *arg)
item->sockerr = SOCKERRNO;
if(item->sockerr == 0)
item->sockerr = RESOLVER_ENOMEM;
/* this resolver cannot tell a transient failure from an
authoritative negative answer, treat it as before */
item->negative = TRUE;
}
}
@ -714,6 +744,24 @@ CURLcode Curl_async_take_result(struct Curl_easy *data,
return CURLE_AGAIN;
Curl_expire_done(data, EXPIRE_ASYNC_NAME);
/* A failure is an authoritative negative answer, eligible for
negative caching, only when every A/AAAA query performed came
back answering that the name does not exist. A query that
failed transiently or never returned is not an answer. */
{
const uint8_t ip_queries =
async->dns_queries & (CURL_DNSQ_A | CURL_DNSQ_AAAA);
bool negative = (async->dns_responses & ip_queries) == ip_queries;
if(thrdd->res_A && (thrdd->res_A->res || !thrdd->res_A->negative))
negative = FALSE;
if(thrdd->res_AAAA && (thrdd->res_AAAA->res || !thrdd->res_AAAA->negative))
negative = FALSE;
if(!thrdd->res_A && !thrdd->res_AAAA)
negative = FALSE;
async->negative_answer = negative;
}
if(async->result) {
result = async->result;
goto out;

View file

@ -122,6 +122,8 @@ struct async_ares_ctx {
#ifdef USE_HTTPSRR
struct Curl_https_rrinfo hinfo;
#endif
BIT(transient_err); /* an A/AAAA query failed without the resolver
answering that the name does not exist */
};
void Curl_async_ares_shutdown(struct Curl_easy *data,
@ -251,6 +253,10 @@ struct Curl_resolv_async {
BIT(for_proxy);
BIT(done);
BIT(shutdown);
BIT(negative_answer); /* resolver answered that the name does not
exist. Only such failures may be cached as
negative entries, not transient or local
resolver failures. */
char hostname[1];
};

View file

@ -58,12 +58,13 @@ static const char * const errors[] = {
"Unexpected CLASS",
"No content",
"Bad ID",
"Name too long"
"Name too long",
"No such name"
};
static const char *doh_strerror(DOHcode code)
{
if((code >= DOH_OK) && (code <= DOH_DNS_NAME_TOO_LONG))
if((code >= DOH_OK) && (code <= DOH_DNS_NXDOMAIN))
return errors[code];
return "bad error code";
}
@ -751,6 +752,8 @@ UNITTEST DOHcode doh_resp_decode(const unsigned char *doh,
if(!doh || doh[0] || doh[1])
return DOH_DNS_BAD_ID; /* bad ID */
rcode = doh[3] & 0x0f;
if(rcode == 3)
return DOH_DNS_NXDOMAIN; /* name does not exist */
if(rcode)
return DOH_DNS_BAD_RCODE; /* bad rcode */
@ -1248,6 +1251,7 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
}
else if(!dohp->pending) {
DOHcode rc[DOH_SLOT_COUNT];
bool negative = TRUE;
int slot;
memset(rc, 0, sizeof(rc));
@ -1262,6 +1266,11 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
rc[slot] = doh_resp_decode(curlx_dyn_uptr(&p->body),
curlx_dyn_len(&p->body),
p->dnstype, &de);
/* Failing without an NXDOMAIN answer - a SERVFAIL-class rcode or
an undecodable response - says nothing about the name. Such a
failure must not be cached as a negative entry. */
if(rc[slot] && (rc[slot] != DOH_DNS_NXDOMAIN))
negative = FALSE;
if(rc[slot]) {
CURL_TRC_DNS(data, "DoH: %s type %s for %s", doh_strerror(rc[slot]),
doh_type2name(p->dnstype), dohp->host);
@ -1279,8 +1288,13 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
}
result = doh2ai(&de, dohp->host, dohp->port, &ai);
if(result)
if(result) {
/* a decoded response without any usable address, e.g. only
CNAME records, is an authoritative "no data" answer */
if((result == CURLE_COULDNT_RESOLVE_HOST) && negative)
async->negative_answer = TRUE;
goto error;
}
/* we got a response, create a dns entry. */
dns = Curl_dnscache_mk_entry(data, async->dns_queries,
@ -1314,6 +1328,9 @@ CURLcode Curl_doh_take_result(struct Curl_easy *data,
*pdns = dns;
} /* address processing done */
else {
/* every query failed. Only NXDOMAIN answers for all of them
make this a negative answer, eligible for caching. */
async->negative_answer = negative;
result = async->for_proxy ?
CURLE_COULDNT_RESOLVE_PROXY : CURLE_COULDNT_RESOLVE_HOST;
}

View file

@ -39,12 +39,13 @@ typedef enum {
DOH_OUT_OF_MEM, /* 5 */
DOH_DNS_RDATA_LEN, /* 6 */
DOH_DNS_MALFORMAT, /* 7 */
DOH_DNS_BAD_RCODE, /* 8 - no such name */
DOH_DNS_BAD_RCODE, /* 8 - unsuccessful rcode, not NXDOMAIN */
DOH_DNS_UNEXPECTED_TYPE, /* 9 */
DOH_DNS_UNEXPECTED_CLASS, /* 10 */
DOH_NO_CONTENT, /* 11 */
DOH_DNS_BAD_ID, /* 12 */
DOH_DNS_NAME_TOO_LONG /* 13 */
DOH_DNS_NAME_TOO_LONG, /* 13 */
DOH_DNS_NXDOMAIN /* 14 - no such name */
} DOHcode;
typedef enum {

View file

@ -42,6 +42,7 @@ void r_freeaddrinfo(struct addrinfo *cahead)
struct context {
struct ares_addrinfo *addr;
int status;
};
static void async_addrinfo_cb(void *userp, int status, int timeouts,
@ -49,6 +50,7 @@ static void async_addrinfo_cb(void *userp, int status, int timeouts,
{
struct context *ctx = (struct context *)userp;
(void)timeouts;
ctx->status = status;
if(ARES_SUCCESS == status) {
ctx->addr = addr;
}
@ -190,8 +192,10 @@ int r_getaddrinfo(const char *node,
/* free the old */
ares_freeaddrinfo(ctx.addr);
}
else if((ctx.status == ARES_ENOTFOUND) || (ctx.status == ARES_ENODATA))
rc = EAI_NONAME; /* no such name */
else
rc = EAI_NONAME; /* got nothing */
rc = EAI_AGAIN; /* failed without an authoritative answer */
/* Cleanup */
ares_destroy(channel);

View file

@ -452,9 +452,15 @@ static CURLcode hostip_resolv_take_result(struct Curl_easy *data,
async->queries_ongoing, async->hostname, async->port);
result = CURLE_OK;
}
else if(result) {
else if(IS_RESOLV_FAIL(result)) {
result = Curl_async_failed(data, async, NULL);
}
else if(result) {
/* a local failure, not a resolve answer. Keep the error as it
is so it does not get treated as one. */
CURL_TRC_DNS(data, "resolve error %d for %s:%u",
(int)result, async->hostname, async->port);
}
else {
CURL_TRC_DNS(data, "resolve complete for %s:%u",
async->hostname, async->port);
@ -528,6 +534,10 @@ bool Curl_resolv_knows_https(struct Curl_easy *data, uint32_t resolv_id)
#endif /* USE_CURL_ASYNC */
/* Start resolving. `*pnegative` is only meaningful when this returns
a CURLE_COULDNT_RESOLVE_* failure: TRUE when the resolver answered
that the name does not exist, FALSE on transient or local failures
that must not be cached as negative entries. */
static CURLcode hostip_resolv_start(struct Curl_easy *data,
uint8_t dns_queries,
const char *hostname,
@ -537,7 +547,8 @@ static CURLcode hostip_resolv_start(struct Curl_easy *data,
timediff_t timeout_ms,
bool allowDOH,
uint32_t *presolv_id,
struct Curl_dns_entry **pdns)
struct Curl_dns_entry **pdns,
bool *pnegative)
{
#ifdef USE_CURL_ASYNC
struct Curl_resolv_async *async = NULL;
@ -546,6 +557,8 @@ static CURLcode hostip_resolv_start(struct Curl_easy *data,
size_t hostname_len;
CURLcode result = CURLE_OK;
*pnegative = FALSE;
(void)timeout_ms; /* not in all ifdefs */
*presolv_id = 0;
*pdns = NULL;
@ -628,8 +641,12 @@ static CURLcode hostip_resolv_start(struct Curl_easy *data,
if(result)
goto out;
addr = Curl_sync_getaddrinfo(data, dns_queries, hostname, port, transport);
if(!addr)
if(!addr) {
result = RESOLV_FAIL(for_proxy);
/* the synchronous resolvers do not tell a transient failure from
an authoritative negative answer, treat it as before */
*pnegative = TRUE;
}
#endif
out:
@ -657,6 +674,7 @@ out:
data->state.async = async;
}
else {
*pnegative = !!async->negative_answer;
Curl_async_destroy(data, async);
}
}
@ -678,6 +696,7 @@ static CURLcode hostip_resolv(struct Curl_easy *data,
size_t hostname_len;
CURLcode result = RESOLV_FAIL(for_proxy);
bool cache_dns = FALSE;
bool negative = FALSE;
(void)timeout_ms; /* not used in all ifdefs */
*presolv_id = 0;
@ -722,14 +741,14 @@ static CURLcode hostip_resolv(struct Curl_easy *data,
cache_dns = TRUE;
result = hostip_resolv_start(data, dns_queries, hostname, port,
transport, for_proxy, timeout_ms, allowDOH,
presolv_id, pdns);
presolv_id, pdns, &negative);
}
out:
if(result && (result != CURLE_AGAIN)) {
Curl_dns_entry_unlink(data, pdns);
if(IS_RESOLV_FAIL(result)) {
if(cache_dns)
if(cache_dns && negative)
Curl_dnscache_add_negative(data, dns_queries, hostname, port);
failf(data, "Could not resolve: %s:%u", hostname, port);
}
@ -1067,8 +1086,14 @@ CURLcode Curl_resolv_take_result(struct Curl_easy *data, uint32_t resolv_id,
Curl_dns_entry_unlink(data, pdns);
}
else if(IS_RESOLV_FAIL(result)) {
Curl_dnscache_add_negative(data, async->dns_queries,
async->hostname, async->port);
/* Only cache the failure when the resolver answered that the
name does not exist. Transient failures, e.g. an unreachable
or overloaded DNS server or local resource shortages, say
nothing about the name and would poison the cache for every
transfer using it. */
if(async->negative_answer)
Curl_dnscache_add_negative(data, async->dns_queries,
async->hostname, async->port);
failf(data, "Could not resolve: %s:%u", async->hostname, async->port);
}
else if(result) {