3924. [bug] Improve 'rndc addzone' error reporting. RT #35187

(cherry picked from commit d4859b0b2a0510d8c4f3c48c606a5568a3b0c1d8)

diff --git a/CHANGES b/CHANGES
index 144ed7ba3a315ef70806c8429309ce2ebcc46ac6..8376cccf2c73d829ad6f6f88f8313c64d1e36593 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,5 @@
+3924.  [bug]           Improve 'rndc addzone' error reporting. RT #35187]
+
 3923.  [bug]           Sanity check the xml2-config output. [RT #22246]
 
 3922.  [bug]           When resigning, dnssec-signzone was removing

diff --git a/bin/named/control.c b/bin/named/control.c
index f82535838b2846026dcc7f7089f5ac5ced03c392..8a8313b73ca098723677d3918ab0050aaee87c85 100644 (file)
--- a/bin/named/control.c
+++ b/bin/named/control.c
@@ -206,7 +206,7 @@ ns_control_docommand(isccc_sexpr_t *message, isc_buffer_t *text) {
                   command_compare(command, NS_COMMAND_LOADKEYS)) {
                result = ns_server_rekey(ns_g_server, command, text);
        } else if (command_compare(command, NS_COMMAND_ADDZONE)) {
-               result = ns_server_add_zone(ns_g_server, command);
+               result = ns_server_add_zone(ns_g_server, command, text);
        } else if (command_compare(command, NS_COMMAND_DELZONE)) {
                result = ns_server_del_zone(ns_g_server, command, text);
        } else if (command_compare(command, NS_COMMAND_SIGNING)) {

diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h
index 376810f574253f6e33e9939aaf8b55d0c29d434c..cb91321ad885ecedb006411e3171d9eb5719df2d 100644 (file)
--- a/bin/named/include/named/server.h
+++ b/bin/named/include/named/server.h
@@ -372,7 +372,7 @@ ns_server_validation(ns_server_t *server, char *args, isc_buffer_t *text);
  * Add a zone to a running process
  */
 isc_result_t
-ns_server_add_zone(ns_server_t *server, char *args);
+ns_server_add_zone(ns_server_t *server, char *args, isc_buffer_t *text);
 
 /*%
  * Deletes a zone from a running process

diff --git a/bin/named/server.c b/bin/named/server.c
index 3d72e075adfb78641e936a01676a9a02abdaf49f..82c994b1a4fd06df28188f9717d31f46f72efc78 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -151,6 +151,14 @@
               if (result != ISC_R_SUCCESS) goto cleanup;        \
        } while (0)
 
+#define TCHECK(op) \
+       do { tresult = (op);                                     \
+               if (tresult != ISC_R_SUCCESS) {                  \
+                       isc_buffer_clear(text);                  \
+                       goto cleanup;                            \
+               }                                                \
+       } while (0)
+
 #define CHECKM(op, msg) \
        do { result = (op);                                       \
               if (result != ISC_R_SUCCESS) {                     \
@@ -411,6 +419,9 @@ newzone_cfgctx_destroy(void **cfgp);
 static isc_result_t
 putstr(isc_buffer_t *b, const char *str);
 
+static isc_result_t
+putnull(isc_buffer_t *b);
+
 isc_result_t
 add_comment(FILE *fp, const char *viewname);
 
@@ -7155,9 +7166,8 @@ zone_from_args(ns_server_t *server, char *args, const char *zonetxt,
                isc_result_t tresult;
 
                tresult = putstr(text, problem);
-               if (tresult == ISC_R_SUCCESS &&
-                   isc_buffer_availablelength(text) > 0U)
-                       isc_buffer_putuint8(text, 0);
+               if (tresult == ISC_R_SUCCESS)
+                       putnull(text);
        }
 
  cleanup:
@@ -8789,8 +8799,8 @@ add_comment(FILE *fp, const char *viewname) {
  * Act on an "addzone" command from the command channel.
  */
 isc_result_t
-ns_server_add_zone(ns_server_t *server, char *args) {
-       isc_result_t         result;
+ns_server_add_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
+       isc_result_t         result, tresult;
        isc_buffer_t         argbuf;
        size_t               arglen;
        cfg_parser_t        *parser = NULL;
@@ -8805,7 +8815,7 @@ ns_server_add_zone(ns_server_t *server, char *args) {
        const char          *argp;
        const char          *viewname = NULL;
        dns_rdataclass_t     rdclass;
-       dns_view_t          *view = 0;
+       dns_view_t          *view = NULL;
        isc_buffer_t         buf;
        dns_fixedname_t      fname;
        dns_name_t          *dnsname;
@@ -8886,7 +8896,14 @@ ns_server_add_zone(ns_server_t *server, char *args) {
        }
 
        /* Open save file for write configuration */
-       CHECK(isc_stdio_open(view->new_zone_file, "a", &fp));
+       result = isc_stdio_open(view->new_zone_file, "a", &fp);
+       if (result != ISC_R_SUCCESS) {
+               TCHECK(putstr(text, "unable to open '"));
+               TCHECK(putstr(text, view->new_zone_file));
+               TCHECK(putstr(text, "': "));
+               TCHECK(putstr(text, isc_result_totext(result)));
+               goto cleanup;
+       }
        CHECK(isc_stdio_tell(fp, &offset));
        if (offset == 0)
                CHECK(add_comment(fp, view->name));
@@ -8900,8 +8917,11 @@ ns_server_add_zone(ns_server_t *server, char *args) {
                                ISC_FALSE, ISC_FALSE);
        dns_view_freeze(view);
        isc_task_endexclusive(server->task);
-       if (result != ISC_R_SUCCESS)
+       if (result != ISC_R_SUCCESS) {
+               TCHECK(putstr(text, "configure_zone failed: "));
+               TCHECK(putstr(text, isc_result_totext(result)));
                goto cleanup;
+       }
 
        /* Is it there yet? */
        CHECK(dns_zt_find(view->zonetable, dnsname, 0, NULL, &zone));
@@ -8914,6 +8934,9 @@ ns_server_add_zone(ns_server_t *server, char *args) {
        if (result != ISC_R_SUCCESS) {
                dns_db_t *dbp = NULL;
 
+               TCHECK(putstr(text, "dns_zone_loadnew failed: "));
+               TCHECK(putstr(text, isc_result_totext(result)));
+
                isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL,
                              NS_LOGMODULE_SERVER, ISC_LOG_INFO,
                              "addzone failed; reverting.");
@@ -8935,7 +8958,7 @@ ns_server_add_zone(ns_server_t *server, char *args) {
        /* Emit the zone name, quoted and escaped */
        isc_buffer_init(&buf, namebuf, sizeof(namebuf));
        CHECK(dns_name_totext(dnsname, ISC_TRUE, &buf));
-       isc_buffer_putuint8(&buf, 0);
+       putnull(&buf);
        CHECK(isc_stdio_write("zone \"", 6, 1, fp, NULL));
        CHECK(isc_stdio_write(namebuf, strlen(namebuf), 1, fp, NULL));
        CHECK(isc_stdio_write("\" ", 2, 1, fp, NULL));
@@ -8974,6 +8997,8 @@ ns_server_add_zone(ns_server_t *server, char *args) {
        result = ISC_R_SUCCESS;
 
  cleanup:
+       if (isc_buffer_usedlength(text) > 0)
+               putnull(text);
        if (fp != NULL)
                isc_stdio_close(fp);
        if (parser != NULL) {
@@ -8998,10 +9023,10 @@ inuse(const char* file, isc_boolean_t first, isc_buffer_t *text) {
            strlen(file) + (first ? sizeof(INUSEMSG) : sizeof("\n")))
        {
                if (first)
-                       isc_buffer_putstr(text, INUSEMSG);
+                       putstr(text, INUSEMSG);
                else
-                       isc_buffer_putstr(text, "\n");
-               isc_buffer_putstr(text, file);
+                       putstr(text, "\n");
+               putstr(text, file);
                return (ISC_FALSE);
        }
        return (first);
@@ -9181,6 +9206,8 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
        dns_zone_getraw(zone, &raw);
        mayberaw = (raw != NULL) ? raw : zone;
        if (cleanup) {
+               isc_result_t tresult;
+
                file = dns_zone_getfile(mayberaw);
                if (isc_file_exists(file))
                        isc_file_remove(file);
@@ -9198,10 +9225,10 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
                        if (isc_file_exists(file))
                                isc_file_remove(file);
                }
-               isc_buffer_putstr(text, "zone ");
-               isc_buffer_putstr(text, zonename);
-               isc_buffer_putstr(text, " and associated files deleted");
-               isc_buffer_putuint8(text, 0);
+               TCHECK(putstr(text, "zone "));
+               TCHECK(putstr(text, zonename));
+               TCHECK(putstr(text, " and associated files deleted"));
+               TCHECK(putnull(text));
        } else if (dns_zone_gettype(mayberaw) == dns_zone_slave ||
                   dns_zone_gettype(mayberaw) == dns_zone_stub)
        {
@@ -9220,8 +9247,7 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
                        file = dns_zone_getjournal(zone);
                        (void)inuse(file, first, text);
                }
-               if (isc_buffer_availablelength(text) > 0)
-                       isc_buffer_putuint8(text, 0);
+               putnull(text);
        }
 
        CHECK(dns_zt_unmount(view->zonetable, zone));
@@ -9236,6 +9262,8 @@ ns_server_del_zone(ns_server_t *server, char *args, isc_buffer_t *text) {
        result = ISC_R_SUCCESS;
 
  cleanup:
+       if (isc_buffer_usedlength(text) > 0)
+               putnull(text);
        if (exclusive)
                isc_task_endexclusive(server->task);
        if (ifp != NULL)
@@ -9414,15 +9442,15 @@ ns_server_signing(ns_server_t *server, char *args, isc_buffer_t *text) {
 
        if (clear) {
                CHECK(dns_zone_keydone(zone, keystr));
-               isc_buffer_putstr(text, "request queued");
-               isc_buffer_putuint8(text, 0);
+               putstr(text, "request queued");
+               putnull(text);
        } else if (chain) {
                CHECK(dns_zone_setnsec3param(zone, (isc_uint8_t)hash,
                                             (isc_uint8_t)flags, iter,
                                             (isc_uint8_t)saltlen, salt,
                                             ISC_TRUE));
-               isc_buffer_putstr(text, "request queued");
-               isc_buffer_putuint8(text, 0);
+               putstr(text, "request queued");
+               putnull(text);
        } else if (list) {
                privatetype = dns_zone_getprivatetype(zone);
                origin = dns_zone_getorigin(zone);
@@ -9434,8 +9462,8 @@ ns_server_signing(ns_server_t *server, char *args, isc_buffer_t *text) {
                                             dns_rdatatype_none, 0,
                                             &privset, NULL);
                if (result == ISC_R_NOTFOUND) {
-                       isc_buffer_putstr(text, "No signing records found");
-                       isc_buffer_putuint8(text, 0);
+                       putstr(text, "No signing records found");
+                       putnull(text);
                        result = ISC_R_SUCCESS;
                        goto cleanup;
                }
@@ -9454,7 +9482,7 @@ ns_server_signing(ns_server_t *server, char *args, isc_buffer_t *text) {
                        CHECK(dns_private_totext(&priv, &buf));
 
                        if (!first)
-                               isc_buffer_putstr(text, "\n");
+                               putstr(text, "\n");
                        first = ISC_FALSE;
 
                        n = snprintf((char *)isc_buffer_used(text),
@@ -9465,8 +9493,8 @@ ns_server_signing(ns_server_t *server, char *args, isc_buffer_t *text) {
 
                        isc_buffer_add(text, (unsigned int)n);
                }
-               if (!first && isc_buffer_availablelength(text) > 0)
-                       isc_buffer_putuint8(text, 0);
+               if (!first)
+                       putnull(text);
 
                if (result == ISC_R_NOMORE)
                        result = ISC_R_SUCCESS;
@@ -9501,6 +9529,15 @@ putstr(isc_buffer_t *b, const char *str) {
        return (ISC_R_SUCCESS);
 }
 
+static isc_result_t
+putnull(isc_buffer_t *b) {
+       if (isc_buffer_availablelength(b) == 0)
+               return (ISC_R_NOSPACE);
+
+       isc_buffer_putuint8(b, 0);
+       return (ISC_R_SUCCESS);
+}
+
 isc_result_t
 ns_server_zonestatus(ns_server_t *server, char *args, isc_buffer_t *text) {
        isc_result_t result = ISC_R_SUCCESS;
@@ -9733,11 +9770,10 @@ ns_server_zonestatus(ns_server_t *server, char *args, isc_buffer_t *text) {
 
  cleanup:
        /* Indicate truncated output if possible. */
-       if (result == ISC_R_NOSPACE && isc_buffer_availablelength(text) > 4U)
-               isc_buffer_putstr(text, "\n...");
-       if ((result == ISC_R_SUCCESS || result == ISC_R_NOSPACE) &&
-           isc_buffer_availablelength(text) > 0)
-               isc_buffer_putuint8(text, 0);
+       if (result == ISC_R_NOSPACE)
+               putstr(text, "\n...");
+       if ((result == ISC_R_SUCCESS || result == ISC_R_NOSPACE))
+               putnull(text);
 
        if (db != NULL)
                dns_db_detach(&db);