]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
dig +nssearch: send more queries even if setting up one of them fails
authorAram Sargsyan <aram@isc.org>
Wed, 22 Jun 2022 14:52:26 +0000 (14:52 +0000)
committerAram Sargsyan <aram@isc.org>
Fri, 22 Jul 2022 09:37:05 +0000 (09:37 +0000)
In the NSSEARCH followup lookup, when one of the queries fails to be
set up (UDP) or connected (TCP), DiG doesn't start the next query.
This is a mistake, because in NSSEARCH mode the queries are independent
and DiG shouldn't stop the lookup process just because setting up (or
connecting to) one of the name servers returns an error code in the
`udp_ready()` or `tcp_connected()` callbacks.

Write a new `nssearch_next()` function which takes care of starting the
next query in NSSEARCH mode, so it can be used in several places without
code repetition.

Make sure that the `udp_ready()` and `tcp_connected()` functions call
`nssearch_next()` in case they won't be calling `send_udp()` and
`send_tcp()` respectively, because in that case the `send_done()`
callback, which usually does the job, won't be called.

Refactor `send_done()` to use the newly written `nssearch_next()`
function.

bin/dig/dighost.c

index dc88010b592956a0a8e7b01fdfae7ec06cba30a8..3d174e4d73f234a29bffbaece5be302baa0487f8 100644 (file)
@@ -2656,6 +2656,54 @@ setup_lookup(dig_lookup_t *lookup) {
        return (true);
 }
 
+/*%
+ * NSSEARCH mode special mode handling function to start the next query in the
+ * list. The lookup lock must be held by the caller. The function will detach
+ * both the lookup and the query, and may cancel the lookup and clear the
+ * current lookup.
+ */
+static void
+nssearch_next(dig_lookup_t *l, dig_query_t *q) {
+       dig_query_t *next = ISC_LIST_NEXT(q, link);
+       bool tcp_mode = l->tcp_mode;
+
+       INSIST(l->ns_search_only && !l->trace_root);
+       INSIST(l == current_lookup);
+
+       if (next == NULL) {
+               /*
+                * If this is the last query, and if there was
+                * not a single successful query in the whole
+                * lookup, then treat the situation as an error,
+                * cancel and clear the lookup.
+                */
+               if (check_if_queries_done(l, q) && !l->ns_search_success) {
+                       dighost_error("NS servers could not be reached");
+                       if (exitcode < 9) {
+                               exitcode = 9;
+                       }
+
+                       cancel_lookup(l);
+                       query_detach(&q);
+                       lookup_detach(&l);
+                       clear_current_lookup();
+               } else {
+                       query_detach(&q);
+                       lookup_detach(&l);
+               }
+       } else {
+               query_detach(&q);
+               lookup_detach(&l);
+
+               debug("sending next, since searching");
+               if (tcp_mode) {
+                       start_tcp(next);
+               } else {
+                       start_udp(next);
+               }
+       }
+}
+
 /*%
  * Event handler for send completion.  Track send counter, and clear out
  * the query if the send was canceled.
@@ -2696,23 +2744,7 @@ send_done(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
        }
 
        if (l->ns_search_only && !l->trace_root) {
-               dig_query_t *next = ISC_LIST_NEXT(query, link);
-               bool tcp_mode = l->tcp_mode;
-
-               query_detach(&query);
-               lookup_detach(&l);
-
-               if (next == NULL) {
-                       clear_current_lookup();
-               } else {
-                       debug("sending next, since searching");
-
-                       if (tcp_mode) {
-                               start_tcp(next);
-                       } else {
-                               start_udp(next);
-                       }
-               }
+               nssearch_next(l, query);
        } else {
                query_detach(&query);
                lookup_detach(&l);
@@ -3128,13 +3160,29 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
                clear_current_lookup();
                UNLOCK_LOOKUP;
                return;
-       } else if (eresult != ISC_R_SUCCESS) {
+       }
+
+       if (eresult != ISC_R_SUCCESS) {
                debug("udp setup failed: %s", isc_result_totext(eresult));
                isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr));
                dighost_warning("UDP setup with %s(%s) for %s failed: %s.",
                                sockstr, query->servname, l->textname,
                                isc_result_totext(eresult));
 
+               /*
+                * NSSEARCH mode: if the current query failed to start properly,
+                * then send_done() will not be called, and we want to make sure
+                * that the next query gets a chance to start in order to not
+                * break the chain.
+                */
+               if (l->ns_search_only && !l->trace_root) {
+                       nssearch_next(l, query);
+
+                       check_if_done();
+                       UNLOCK_LOOKUP;
+                       return;
+               }
+
                if (exitcode < 9) {
                        exitcode = 9;
                }
@@ -3516,7 +3564,9 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
                clear_current_lookup();
                UNLOCK_LOOKUP;
                return;
-       } else if (eresult != ISC_R_SUCCESS) {
+       }
+
+       if (eresult != ISC_R_SUCCESS) {
                debug("unsuccessful connection: %s",
                      isc_result_totext(eresult));
                isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr));
@@ -3524,6 +3574,20 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) {
                                sockstr, query->servname, l->textname,
                                isc_result_totext(eresult));
 
+               /*
+                * NSSEARCH mode: if the current query failed to start properly,
+                * then send_done() will not be called, and we want to make sure
+                * that the next query gets a chance to start in order to not
+                * break the chain.
+                */
+               if (l->ns_search_only && !l->trace_root) {
+                       nssearch_next(l, query);
+
+                       check_if_done();
+                       UNLOCK_LOOKUP;
+                       return;
+               }
+
                /* XXX Clean up exitcodes */
                if (exitcode < 9) {
                        exitcode = 9;
@@ -3916,7 +3980,8 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region,
                         * treat the situation as an error.
                         */
                        if (!l->ns_search_success) {
-                               dighost_error("no NS servers could be reached");
+                               dighost_error(
+                                       "NS servers could not be reached");
                                if (exitcode < 9) {
                                        exitcode = 9;
                                }