Don't print dig's startup banner with +nocmd when the lookup fails

This only affects the failure path.  On a successful lookup dig
reprints the banner and re-evaluates the final options as it does so,
so a late +nocmd is honored; but the "no servers could be reached"
error path prints the pre-built banner verbatim, without that
re-check, and that banner had been built too early to be correct.

printgreeting() was called the moment the first query name was seen,
so options appearing later on the command line (e.g. +[no]cmd, +short,
+yaml) were not yet in effect and the banner captured stale state.
That is why a failing "dig . soa @host +yaml" emitted the ";"-prefixed
banner ahead of the error, and why "+nocmd" placed after the query
name was ignored.

Assisted-by: Claude:claude-opus-4-8

diff --git a/bin/dig/dig.c b/bin/dig/dig.c
index ecb722b78e53c92b7e7fbd3f4c68410dbd2d7df6..f3696f6e009c6b573d289894a8aa7de845075056 100644 (file)
--- a/bin/dig/dig.c
+++ b/bin/dig/dig.c
@@ -2627,8 +2627,8 @@ static const char *single_dash_opts = "46dFhimnruv";
 static const char *dash_opts = "46bcdFfhikmnpqrtvyx";
 static bool
 dash_option(char *option, char *next, dig_lookup_t **lookup,
-           bool *open_type_class, bool *need_clone, bool config_only, int argc,
-           char **argv, bool *firstarg) {
+           bool *open_type_class, bool *need_clone, bool config_only,
+           bool *added_lookup) {
        char opt, *value, *ptr, *ptr2, *ptr3, *last;
        isc_result_t result;
        bool value_from_next;
@@ -2799,10 +2799,7 @@ dash_option(char *option, char *next, dig_lookup_t **lookup,
                        (*lookup)->trace_root = ((*lookup)->trace ||
                                                 (*lookup)->ns_search_only);
                        (*lookup)->new_search = true;
-                       if (*firstarg) {
-                               printgreeting(argc, argv, *lookup);
-                               *firstarg = false;
-                       }
+                       *added_lookup = true;
                        ISC_LIST_APPEND(lookup_list, *lookup, link);
                        debug("looking up %s", (*lookup)->textname);
                }
@@ -2910,10 +2907,7 @@ dash_option(char *option, char *next, dig_lookup_t **lookup,
                                (*lookup)->rdclass = dns_rdataclass_in;
                        }
                        (*lookup)->new_search = true;
-                       if (*firstarg) {
-                               printgreeting(argc, argv, *lookup);
-                               *firstarg = false;
-                       }
+                       *added_lookup = true;
                        ISC_LIST_APPEND(lookup_list, *lookup, link);
                } else {
                        fprintf(stderr, "Invalid IP address %s\n", value);
@@ -3032,8 +3026,9 @@ static void
 parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
        isc_result_t result;
        isc_textregion_t tr;
-       bool firstarg = true;
+       bool added_lookup = false;
        dig_lookup_t *lookup = NULL;
+       dig_lookup_t *last_existing_lookup = ISC_LIST_TAIL(lookup_list);
        dns_rdatatype_t rdtype;
        dns_rdataclass_t rdclass;
        bool open_type_class = true;
@@ -3146,8 +3141,7 @@ parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
                        if (rc <= 1) {
                                if (dash_option(&rv[0][1], NULL, &lookup,
                                                &open_type_class, &need_clone,
-                                               config_only, argc, argv,
-                                               &firstarg))
+                                               config_only, &added_lookup))
                                {
                                        rc--;
                                        rv++;
@@ -3155,8 +3149,7 @@ parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
                        } else {
                                if (dash_option(&rv[0][1], rv[1], &lookup,
                                                &open_type_class, &need_clone,
-                                               config_only, argc, argv,
-                                               &firstarg))
+                                               config_only, &added_lookup))
                                {
                                        rc--;
                                        rv++;
@@ -3260,10 +3253,7 @@ parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
                                lookup->trace_root = (lookup->trace ||
                                                      lookup->ns_search_only);
                                lookup->new_search = true;
-                               if (firstarg) {
-                                       printgreeting(argc, argv, lookup);
-                                       firstarg = false;
-                               }
+                               added_lookup = true;
                                ISC_LIST_APPEND(lookup_list, lookup, link);
                                debug("looking up %s", lookup->textname);
                        }
@@ -3271,6 +3261,26 @@ parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
                }
        }
 
+       /*
+        * The whole command line has now been parsed, so every option is in
+        * its final state.  Build the greeting only now: deferring it until
+        * here is what lets the banner reflect options such as +[no]cmd,
+        * +short and +yaml that may follow the query name on the command line.
+        *
+        * The greeting belongs to the first lookup this call appended.  Because
+        * lookup_list is global and may already hold lookups on entry (e.g.
+        * "dig foo -f batchfile" queues "foo" before the batch file's first
+        * line is parsed), that first lookup is the one right after
+        * last_existing_lookup, or the list head if the list was empty.
+        */
+       if (added_lookup) {
+               dig_lookup_t *greeting =
+                       (last_existing_lookup != NULL)
+                               ? ISC_LIST_NEXT(last_existing_lookup, link)
+                               : ISC_LIST_HEAD(lookup_list);
+               printgreeting(argc, argv, greeting);
+       }
+
        /*
         * If we have a batchfile, seed the lookup list with the
         * first entry, then trust the callback in dighost_shutdown
@@ -3321,10 +3331,7 @@ parse_args(bool is_batchfile, bool config_only, int argc, char **argv) {
                strlcpy(lookup->textname, ".", sizeof(lookup->textname));
                lookup->rdtype = dns_rdatatype_ns;
                lookup->rdtypeset = true;
-               if (firstarg) {
-                       printgreeting(argc, argv, lookup);
-                       firstarg = false;
-               }
+               printgreeting(argc, argv, lookup);
                ISC_LIST_APPEND(lookup_list, lookup, link);
        }
        if (!need_clone) {

diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh
index 85247c17e4cf2c75baddf232b978e4cbdf1310b2..bbdbe531d46a67dab00d1ff6f65aebfceda2b5f9 100644 (file)
--- a/bin/tests/system/digdelv/tests.sh
+++ b/bin/tests/system/digdelv/tests.sh
@@ -1248,6 +1248,30 @@ if [ -x "$DIG" ]; then
     status=$((status + ret))
   fi
 
+  # +nocmd placed after the query name must suppress the startup banner
+  # ("<<>> DiG ..." lines), including on the error path.  This regressed
+  # because the banner was built as soon as the query name was seen, before
+  # +nocmd had been parsed; it is now built after the whole command line has
+  # been processed.  The default (+cmd) case is checked first so the absence
+  # check below is meaningful.
+  n=$((n + 1))
+  echo_i "check that dig prints the startup banner by default ($n)"
+  ret=0
+  dig_with_opts silent.example @10.53.0.7 +notcp +timeout=1 +tries=1 >dig.out.test$n 2>&1 && ret=1
+  grep -F "<<>> DiG" dig.out.test$n >/dev/null || ret=1
+  grep -F "no servers could be reached" dig.out.test$n >/dev/null || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status + ret))
+
+  n=$((n + 1))
+  echo_i "check that dig +nocmd after the query name suppresses the startup banner ($n)"
+  ret=0
+  dig_with_opts silent.example @10.53.0.7 +notcp +timeout=1 +tries=1 +nocmd >dig.out.test$n 2>&1 && ret=1
+  grep -F "<<>> DiG" dig.out.test$n >/dev/null && ret=1
+  grep -F "no servers could be reached" dig.out.test$n >/dev/null || ret=1
+  if [ $ret -ne 0 ]; then echo_i "failed"; fi
+  status=$((status + ret))
+
   n=$((n + 1))
   echo_i "check that dig +bufsize=0 just sets the buffer size to 0 ($n)"
   ret=0