Set up stale response lookup before query_done() is called

When something goes wrong while recursing for an answer to a query,
query_gotanswer() sets a flag (qctx->want_stale) in the query context.
query_done() is subsequently called and it can either set up a stale
response lookup (if serve-stale is enabled) or conclude that a SERVFAIL
response should be sent.  This may cause confusion when looking at query
error logs since the QUERY_ERROR() line responsible for setting the
response's RCODE to SERVFAIL is not in a catch-all branch of a switch
statement inside query_gotanswer() (like it is for authoritative
responses) but rather in a code branch which appears to have something
to do with serve-stale, even when the latter is not enabled.

Extract the part of query_done() responsible for checking serve-stale
configuration and optionally setting up a stale response lookup into a
separate function, query_usestale(), shifting the responsibility for
setting the response's RCODE to SERVFAIL to the same QUERY_ERROR() line
in query_gotanswer() which is evaluated for authoritative responses.
This commit is contained in:
Michał Kępień 2018-10-08 12:47:28 +02:00
parent e8f66bcfa3
commit cb48d410d8

View file

@ -6617,6 +6617,56 @@ root_key_sentinel_return_servfail(query_ctx_t *qctx, isc_result_t result) {
return (false);
}
/*%
* If serving stale answers is allowed, set up 'qctx' to look for one and
* return true; otherwise, return false.
*/
static bool
query_usestale(query_ctx_t *qctx) {
bool staleanswersok = false;
dns_ttl_t stale_ttl = 0;
isc_result_t result;
qctx_clean(qctx);
qctx_freedata(qctx);
/*
* Stale answers only make sense if stale_ttl > 0 but we want rndc to
* be able to control returning stale answers if they are configured.
*/
dns_db_attach(qctx->client->view->cachedb, &qctx->db);
result = dns_db_getservestalettl(qctx->db, &stale_ttl);
if (result == ISC_R_SUCCESS && stale_ttl > 0) {
switch (qctx->client->view->staleanswersok) {
case dns_stale_answer_yes:
staleanswersok = true;
break;
case dns_stale_answer_conf:
staleanswersok =
qctx->client->view->staleanswersenable;
break;
case dns_stale_answer_no:
staleanswersok = false;
break;
}
} else {
staleanswersok = false;
}
if (staleanswersok) {
qctx->want_stale = true;
qctx->client->query.dboptions |= DNS_DBFIND_STALEOK;
inc_stats(qctx->client, ns_statscounter_trystale);
if (qctx->client->query.fetch != NULL) {
dns_resolver_destroyfetch(&qctx->client->query.fetch);
}
} else {
dns_db_detach(&qctx->db);
}
return (staleanswersok);
}
/*%
* Continue after doing a database lookup or returning from
* recursion, and call out to the next function depending on the
@ -6706,11 +6756,14 @@ query_gotanswer(query_ctx_t *qctx, isc_result_t result) {
"query_gotanswer: unexpected error: %s",
isc_result_totext(result));
CCTRACE(ISC_LOG_ERROR, errmsg);
if (qctx->resuming) {
qctx->want_stale = true;
} else {
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
if (qctx->resuming && query_usestale(qctx)) {
/*
* If serve-stale is enabled, query_usestale() already
* set up 'qctx' for looking up a stale response.
*/
return (query_lookup(qctx));
}
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
return (query_done(qctx));
}
}
@ -10639,49 +10692,6 @@ query_done(query_ctx_t *qctx) {
return (ns__query_start(qctx));
}
if (qctx->want_stale) {
dns_ttl_t stale_ttl = 0;
isc_result_t result;
bool staleanswersok = false;
/*
* Stale answers only make sense if stale_ttl > 0 but
* we want rndc to be able to control returning stale
* answers if they are configured.
*/
dns_db_attach(qctx->client->view->cachedb, &qctx->db);
result = dns_db_getservestalettl(qctx->db, &stale_ttl);
if (result == ISC_R_SUCCESS && stale_ttl > 0) {
switch (qctx->client->view->staleanswersok) {
case dns_stale_answer_yes:
staleanswersok = true;
break;
case dns_stale_answer_conf:
staleanswersok =
qctx->client->view->staleanswersenable;
break;
case dns_stale_answer_no:
staleanswersok = false;
break;
}
} else {
staleanswersok = false;
}
if (staleanswersok) {
qctx->client->query.dboptions |= DNS_DBFIND_STALEOK;
inc_stats(qctx->client, ns_statscounter_trystale);
if (qctx->client->query.fetch != NULL)
dns_resolver_destroyfetch(
&qctx->client->query.fetch);
return (query_lookup(qctx));
}
dns_db_detach(&qctx->db);
qctx->want_stale = false;
QUERY_ERROR(qctx, DNS_R_SERVFAIL);
return (query_done(qctx));
}
if (qctx->result != ISC_R_SUCCESS &&
(!PARTIALANSWER(qctx->client) || WANTRECURSION(qctx->client) ||
qctx->result == DNS_R_DROP))