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.

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 3c09fe3911486b0c41aed4e90a3b09de7a780b1c..036eb6ce914cb68c95a451840d676e1ee210157b 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -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))