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

(cherry picked from commit d4859b0b2a0510d8c4f3c48c606a5568a3b0c1d8)

diff --git a/CHANGES b/CHANGES
index a61ac262be7d7943a4d9d94017bc72260105b06e..140745be51fcc43044b916b75a415e15a2788cad 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 35312fee59f37ef58fb8aa192afdfa67744d3515..bb82cb441cf8dbebe29cf1785aab9d945616deea 100644 (file)
--- a/bin/named/control.c
+++ b/bin/named/control.c
@@ -203,7 +203,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 52ba94d2953864a454c6452d1e76b62ac91888a7..38176151cd4571548554cce9401633dd585bb87e 100644 (file)
--- a/bin/named/include/named/server.h
+++ b/bin/named/include/named/server.h
@@ -345,7 +345,7 @@ ns_server_validation(ns_server_t *server, char *args);
  * 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 b6a9e61309b8e7582994dd396a3dbc8d73a321c2..84fd34f314bf4ff41aa6af3422b96faa8f1f4a7f 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -130,6 +130,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) {                     \
@@ -389,6 +397,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);
 
@@ -6628,9 +6639,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:
@@ -8207,8 +8217,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;
@@ -8223,7 +8233,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;
@@ -8304,7 +8314,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));
@@ -8317,8 +8334,11 @@ ns_server_add_zone(ns_server_t *server, char *args) {
                                server->mctx, view, cfg->actx, 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));
@@ -8331,6 +8351,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.");
@@ -8352,7 +8375,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));
@@ -8388,6 +8411,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) {
@@ -8566,6 +8591,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 (ifp != NULL)
                isc_stdio_close(ifp);
        if (ofp != NULL) {
@@ -8695,15 +8722,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);
@@ -8715,8 +8742,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;
                }
@@ -8735,7 +8762,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),
@@ -8746,8 +8773,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;
@@ -8781,3 +8808,12 @@ putstr(isc_buffer_t *b, const char *str) {
        isc_buffer_putmem(b, (const unsigned char *)str, l);
        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);
+}