pullup:

1014.   [bug]           Some queries would cause statistics counters to
                        increment more than once or not at all. [RT #1321]

diff --git a/CHANGES b/CHANGES
index c19f1aab0d7b3d267482aa6fa5c22dd5cfc36a09..4e9b4d124723f87e4e16ff1a4407675b4fd3e816 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -98,6 +98,9 @@
 
 1123.  [bug]           dig +[no]fail did not match description. [RT #2052]
 
+1014.  [bug]           Some queries would cause statistics counters to
+                       increment more than once or not at all. [RT #1321]
+
 1012.  [bug]           The -p option to named did not behave as documented.
 
  988.  [bug]           'additional-from-auth no;' did not work reliably
@@ -369,6 +372,7 @@
  
        --- 9.2.0rc4 released ---
 
+
 1013.  [bug]           It was possible to cancel a query twice when marking
                        a server as bogus or by having a blackhole acl.
                        [RT #1776]

diff --git a/bin/named/include/named/query.h b/bin/named/include/named/query.h
index 8840338b8ac3eeb645d8a6a1781641800719f081..aaae78a145d431f3c0170a4ba382dfcf03ef1d28 100644 (file)
--- a/bin/named/include/named/query.h
+++ b/bin/named/include/named/query.h
@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: query.h,v 1.28.2.1 2002/01/24 03:49:02 marka Exp $ */
+/* $Id: query.h,v 1.28.2.2 2002/01/24 04:22:58 marka Exp $ */
 
 #ifndef NAMED_QUERY_H
 #define NAMED_QUERY_H 1
@@ -46,7 +46,9 @@ struct ns_query {
        unsigned int                    fetchoptions;
        dns_db_t *                      gluedb;
        dns_db_t *                      authdb;
-       isc_boolean_t                   authdbvalid;
+       dns_zone_t *                    authzone;
+       isc_boolean_t                   authdbset;
+       isc_boolean_t                   isreferral;
        dns_fetch_t *                   fetch;
        dns_a6context_t                 a6ctx;
        isc_bufferlist_t                namebufs;

diff --git a/bin/named/query.c b/bin/named/query.c
index 270fdc7f06d17be8c5ac1935c78336ed663fd91b..dfc5992b16d7d7b92bac3714019dd65545b3fa06 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: query.c,v 1.198.2.3 2002/01/24 03:49:01 marka Exp $ */
+/* $Id: query.c,v 1.198.2.4 2002/01/24 04:22:57 marka Exp $ */
 
 #include <config.h>
 
@@ -143,20 +143,55 @@ synth_rev_respond(ns_client_t *client, dns_byaddrevent_t *bevent);
  * Increment query statistics counters.
  */
 static inline void
-count_query(dns_zone_t *zone, isc_boolean_t is_zone,
-           dns_statscounter_t counter)
-{
+inc_stats(ns_client_t *client, dns_statscounter_t counter) {
+       dns_zone_t *zone = client->query.authzone;
+       
        REQUIRE(counter < DNS_STATS_NCOUNTERS);
 
        ns_g_server->querystats[counter]++;
 
-       if (is_zone && zone != NULL) {
+       if (zone != NULL) {
                isc_uint64_t *zonestats = dns_zone_getstatscounters(zone);
                if (zonestats != NULL)
                        zonestats[counter]++;
        }
 }
 
+static void
+query_send(ns_client_t *client) {
+       dns_statscounter_t counter;
+       if (client->message->rcode == dns_rcode_noerror) {
+               if (ISC_LIST_EMPTY(client->message->sections[DNS_SECTION_ANSWER])) {
+                       if (client->query.isreferral) {
+                               counter = dns_statscounter_referral;
+                       } else {
+                               counter = dns_statscounter_nxrrset;
+                       }
+               } else {
+                       counter = dns_statscounter_success;
+               }
+       } else if (client->message->rcode == dns_rcode_nxdomain) {
+               counter = dns_statscounter_nxdomain;
+       } else {
+               /* We end up here in case of YXDOMAIN, and maybe others */
+               counter = dns_statscounter_failure;
+       }
+       inc_stats(client, counter);
+       ns_client_send(client);
+}
+
+static void
+query_error(ns_client_t *client, isc_result_t result) {
+       inc_stats(client, dns_statscounter_failure);
+       ns_client_error(client, result);
+}
+
+static void
+query_next(ns_client_t *client, isc_result_t result) {
+       inc_stats(client, dns_statscounter_failure);
+       ns_client_next(client, result);
+}
+
 static inline void
 query_maybeputqname(ns_client_t *client) {
        if (client->query.restarts > 0) {
@@ -205,6 +240,8 @@ query_reset(ns_client_t *client, isc_boolean_t everything) {
 
        if (client->query.authdb != NULL)
                dns_db_detach(&client->query.authdb);
+       if (client->query.authzone != NULL)
+               dns_zone_detach(&client->query.authzone);
 
        /*
         * Clean up free versions.
@@ -246,11 +283,12 @@ query_reset(ns_client_t *client, isc_boolean_t everything) {
        client->query.dboptions = 0;
        client->query.fetchoptions = 0;
        client->query.gluedb = NULL;
-       client->query.authdbvalid = ISC_FALSE;
+       client->query.authdbset = ISC_FALSE;
+       client->query.isreferral = ISC_FALSE;
 }
 
 static void
-query_next(ns_client_t *client) {
+query_next_callback(ns_client_t *client) {
        query_reset(client, ISC_FALSE);
 }
 
@@ -473,7 +511,9 @@ ns_query_init(ns_client_t *client) {
        client->query.qname = NULL;
        client->query.fetch = NULL;
        client->query.authdb = NULL;
-       client->query.authdbvalid = ISC_FALSE;
+       client->query.authzone = NULL;
+       client->query.authdbset = ISC_FALSE;
+       client->query.isreferral = ISC_FALSE;   
        query_reset(client, ISC_FALSE);
        result = query_newdbversion(client, 3);
        if (result != ISC_R_SUCCESS)
@@ -557,7 +597,7 @@ query_getzonedb(ns_client_t *client, dns_name_t *name, unsigned int options,
         * additional data from other zones.
         */
        if (!client->view->additionalfromauth &&
-           client->query.authdbvalid &&
+           client->query.authdbset &&
            db != client->query.authdb)
                goto refuse;
 
@@ -838,7 +878,6 @@ query_simplefind(void *arg, dns_name_t *name, dns_rdatatype_t type,
                    dns_rdataset_isassociated(sigrdataset))
                        dns_rdataset_disassociate(sigrdataset);
                if (is_zone) {
-                       count_query(zone, is_zone, dns_statscounter_referral);
                        if (USECACHE(client)) {
                                /*
                                 * Either the answer is in the cache, or we
@@ -903,11 +942,8 @@ query_simplefind(void *arg, dns_name_t *name, dns_rdatatype_t type,
        }
        /*
         * If we get here, the result is ISC_R_SUCCESS, and we found the
-        * answer we were looking for in the zone.  Update the zone's
-        * query counter.
+        * answer we were looking for in the zone.
         */
-       if (result == ISC_R_SUCCESS)
-               count_query(zone, is_zone, dns_statscounter_success);
 
  cleanup:
        if (dns_rdataset_isassociated(&zrdataset)) {
@@ -2032,7 +2068,7 @@ query_resume(isc_task_t *task, isc_event_t *event) {
                if (devent->sigrdataset != NULL)
                        query_putrdataset(client, &devent->sigrdataset);
                isc_event_free(&event);
-               ns_client_next(client, ISC_R_CANCELED);
+               query_next(client, ISC_R_CANCELED);
                /*
                 * This may destroy the client.
                 */
@@ -2049,6 +2085,8 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qdomain,
        isc_result_t result;
        dns_rdataset_t *rdataset, *sigrdataset;
 
+       inc_stats(client, dns_statscounter_recursion);
+
        /*
         * We are about to recurse, which means that this client will
         * be unavailable for serving new requests for an indeterminate
@@ -2376,20 +2414,17 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                 */
                dbuf = query_getnamebuf(client);
                if (dbuf == NULL) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
                fname = query_newname(client, dbuf, &b);
                if (fname == NULL) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
                tname = dns_fixedname_name(&event->foundname);
                result = dns_name_copy(tname, fname, NULL);
                if (result != ISC_R_SUCCESS) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
@@ -2423,7 +2458,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
        result = query_getdb(client, client->query.qname, 0, &zone, &db,
                             &version, &is_zone);
        if (result != ISC_R_SUCCESS) {
-               count_query(NULL, ISC_FALSE, dns_statscounter_failure);
                if (result == DNS_R_REFUSED)
                        QUERY_ERROR(DNS_R_REFUSED);
                else
@@ -2435,9 +2469,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                authoritative = ISC_TRUE;
        
        if (event == NULL && client->query.restarts == 0) {
-               if (is_zone)
+               if (is_zone) {
+                       dns_zone_attach(zone, &client->query.authzone);
                        dns_db_attach(db, &client->query.authdb);
-               client->query.authdbvalid = ISC_TRUE;
+               }
+               client->query.authdbset = ISC_TRUE;
        }
 
  db_find:
@@ -2447,21 +2483,18 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
         */
        dbuf = query_getnamebuf(client);
        if (dbuf == NULL) {
-               count_query(zone, is_zone, dns_statscounter_failure);
                QUERY_ERROR(DNS_R_SERVFAIL);
                goto cleanup;
        }
        fname = query_newname(client, dbuf, &b);
        rdataset = query_newrdataset(client);
        if (fname == NULL || rdataset == NULL) {
-               count_query(zone, is_zone, dns_statscounter_failure);
                QUERY_ERROR(DNS_R_SERVFAIL);
                goto cleanup;
        }
        if (WANTDNSSEC(client)) {
                sigrdataset = query_newrdataset(client);
                if (sigrdataset == NULL) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
@@ -2518,8 +2551,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                        result = dns_name_copy(client->query.qname,
                                               fname, NULL);
                        if (result != ISC_R_SUCCESS) {
-                               count_query(zone, is_zone,
-                                           dns_statscounter_failure);
                                QUERY_ERROR(DNS_R_SERVFAIL);
                                goto cleanup;
                        }
@@ -2537,7 +2568,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
        CTRACE("query_find: resume");
        switch (result) {
        case ISC_R_SUCCESS:
-               count_query(zone, is_zone, dns_statscounter_success);
                /*
                 * This case is handled in the main line below.
                 */
@@ -2587,8 +2617,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                         * recurse anyway.
                         */
                        if (RECURSIONOK(client)) {
-                               count_query(zone, is_zone,
-                                           dns_statscounter_recursion);
                                result = query_recurse(client, qtype,
                                                       NULL, NULL);
                                if (result == ISC_R_SUCCESS)
@@ -2596,15 +2624,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                                NS_QUERYATTR_RECURSING;
                                else {
                                        /* Unable to recurse. */
-                                       count_query(zone, is_zone,
-                                                   dns_statscounter_failure);
                                        QUERY_ERROR(DNS_R_SERVFAIL);
                                }
                                goto cleanup;
                        } else {
                                /* Unable to give root server referral. */
-                               count_query(zone, is_zone,
-                                           dns_statscounter_failure);
                                QUERY_ERROR(DNS_R_SERVFAIL);
                                goto cleanup;
                        }
@@ -2636,6 +2660,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                 * database by setting client->query.gluedb.
                                 */
                                client->query.gluedb = db;
+                               client->query.isreferral = ISC_TRUE;
                                /*
                                 * We must ensure NOADDITIONAL is off,
                                 * because the generation of
@@ -2648,9 +2673,9 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                        sigrdatasetp = &sigrdataset;
                                else
                                        sigrdatasetp = NULL;
-                               query_addrrset(client, &fname, &rdataset,
-                                              sigrdatasetp, dbuf,
-                                              DNS_SECTION_AUTHORITY);
+                               query_addrrset(client, &fname,
+                                              &rdataset, sigrdatasetp,
+                                              dbuf, DNS_SECTION_AUTHORITY);
                                client->query.gluedb = NULL;
                        } else {
                                /*
@@ -2711,8 +2736,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                /*
                                 * Recurse!
                                 */
-                               count_query(zone, is_zone,
-                                           dns_statscounter_recursion);
                                if (type == dns_rdatatype_key)
                                        result = query_recurse(client, qtype,
                                                               NULL, NULL);
@@ -2722,20 +2745,16 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                if (result == ISC_R_SUCCESS)
                                        client->query.attributes |=
                                                NS_QUERYATTR_RECURSING;
-                               else {
-                                       count_query(zone, is_zone,
-                                                   dns_statscounter_failure);
+                               else
                                        QUERY_ERROR(DNS_R_SERVFAIL);
-                               }
                        } else {
                                /*
                                 * This is the best answer.
                                 */
-                               count_query(zone, is_zone,
-                                           dns_statscounter_referral);
-                               client->query.gluedb = zdb;
                                client->query.attributes |=
                                        NS_QUERYATTR_CACHEGLUEOK;
+                               client->query.gluedb = zdb;
+                               client->query.isreferral = ISC_TRUE;
                                /*
                                 * We must ensure NOADDITIONAL is off,
                                 * because the generation of
@@ -2759,7 +2778,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                goto cleanup;
        case DNS_R_NXRRSET:
                INSIST(is_zone);
-               count_query(zone, is_zone, dns_statscounter_nxrrset);
                if (dns_rdataset_isassociated(rdataset)) {
                        /*
                         * If we've got a NXT record, we need to save the
@@ -2780,7 +2798,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                 */
                result = query_addsoa(client, db, ISC_FALSE);
                if (result != ISC_R_SUCCESS) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(result);
                        goto cleanup;
                }
@@ -2796,7 +2813,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                goto cleanup;
        case DNS_R_NXDOMAIN:
                INSIST(is_zone);
-               count_query(zone, is_zone, dns_statscounter_nxdomain);
                if (dns_rdataset_isassociated(rdataset)) {
                        /*
                         * If we've got a NXT record, we need to save the
@@ -2823,7 +2839,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                else
                        result = query_addsoa(client, db, ISC_FALSE);
                if (result != ISC_R_SUCCESS) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(result);
                        goto cleanup;
                }
@@ -2842,13 +2857,8 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                client->message->rcode = dns_rcode_nxdomain;
                goto cleanup;
        case DNS_R_NCACHENXDOMAIN:
-               INSIST(!is_zone);
-               count_query(NULL, is_zone, dns_statscounter_nxdomain);
-               goto ncachenxrrset;
        case DNS_R_NCACHENXRRSET:
                INSIST(!is_zone);
-               count_query(NULL, is_zone, dns_statscounter_nxrrset);
-       ncachenxrrset:
                authoritative = ISC_FALSE;
                /*
                 * Set message rcode, if required.
@@ -3036,7 +3046,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                /*
                 * Something has gone wrong.
                 */
-               count_query(zone, is_zone, dns_statscounter_failure);
                QUERY_ERROR(DNS_R_SERVFAIL);
                goto cleanup;
        }
@@ -3050,7 +3059,6 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                rdsiter = NULL;
                result = dns_db_allrdatasets(db, node, version, 0, &rdsiter);
                if (result != ISC_R_SUCCESS) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
@@ -3126,14 +3134,11 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                                /*
                                 * Something went wrong.
                                 */
-                               count_query(zone, is_zone,
-                                           dns_statscounter_failure);
                                result = DNS_R_SERVFAIL;
                        }
                }
                dns_rdatasetiter_destroy(&rdsiter);
                if (result != ISC_R_NOMORE) {
-                       count_query(zone, is_zone, dns_statscounter_failure);
                        QUERY_ERROR(DNS_R_SERVFAIL);
                        goto cleanup;
                }
@@ -3229,7 +3234,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                 * or if the client requested recursion and thus wanted
                 * the complete answer, send an error response.
                 */
-                ns_client_error(client, eresult);
+                query_error(client, eresult);
                 ns_client_detach(&client);
        } else if (!RECURSING(client)) {
                /*
@@ -3244,7 +3249,7 @@ query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype)
                    client->view->auth_nxdomain == ISC_TRUE)
                        client->message->flags |= DNS_MESSAGEFLAG_AA;
 
-               ns_client_send(client);
+               query_send(client);
                ns_client_detach(&client);
        }
        CTRACE("query_find: done");
@@ -3284,7 +3289,7 @@ ns_query_start(ns_client_t *client) {
        /*
         * Ensure that appropriate cleanups occur.
         */
-       client->next = query_next;
+       client->next = query_next_callback;
 
        if ((message->flags & DNS_MESSAGEFLAG_RD) != 0)
                client->query.attributes |= NS_QUERYATTR_WANTRECURSION;
@@ -3321,7 +3326,7 @@ ns_query_start(ns_client_t *client) {
         */
        result = dns_message_firstname(message, DNS_SECTION_QUESTION);
        if (result != ISC_R_SUCCESS) {
-               ns_client_error(client, result);
+               query_error(client, result);
                return;
        }
        dns_message_currentname(message, DNS_SECTION_QUESTION,
@@ -3334,9 +3339,9 @@ ns_query_start(ns_client_t *client) {
                         * There's more than one QNAME in the question
                         * section.
                         */
-                       ns_client_error(client, DNS_R_FORMERR);
+                       query_error(client, DNS_R_FORMERR);
                } else
-                       ns_client_error(client, result);
+                       query_error(client, result);
                return;
        }
 
@@ -3347,7 +3352,7 @@ ns_query_start(ns_client_t *client) {
         * Check for multiple question queries, since edns1 is dead.
         */
        if (message->counts[DNS_SECTION_QUESTION] > 1) {
-               ns_client_error(client, DNS_R_FORMERR);
+               query_error(client, DNS_R_FORMERR);
                return;
        }
 
@@ -3367,19 +3372,19 @@ ns_query_start(ns_client_t *client) {
                        return;
                case dns_rdatatype_maila:
                case dns_rdatatype_mailb:
-                       ns_client_error(client, DNS_R_NOTIMP);
+                       query_error(client, DNS_R_NOTIMP);
                        return;
                case dns_rdatatype_tkey:
                        result = dns_tkey_processquery(client->message,
                                                ns_g_server->tkeyctx,
                                                client->view->dynamickeys);
                        if (result == ISC_R_SUCCESS)
-                               ns_client_send(client);
+                               query_send(client);
                        else
-                               ns_client_error(client, result);
+                               query_error(client, result);
                        return;
                default: /* TSIG, etc. */
-                       ns_client_error(client, DNS_R_FORMERR);
+                       query_error(client, DNS_R_FORMERR);
                        return;
                }
        }
@@ -3401,7 +3406,7 @@ ns_query_start(ns_client_t *client) {
         */
        result = dns_message_reply(message, ISC_TRUE);
        if (result != ISC_R_SUCCESS) {
-               ns_client_next(client, result);
+               query_next(client, result);
                return;
        }
 
@@ -3670,9 +3675,9 @@ synth_fwd_respond(ns_client_t *client, dns_adbfind_t *find) {
 static void
 synth_finish(ns_client_t *client, isc_result_t result) {
        if (result == ISC_R_SUCCESS)
-               ns_client_send(client);
+               query_send(client);
        else
-               ns_client_error(client, result);
+               query_error(client, result);
        ns_client_detach(&client);      
 }