Fixed: Fail on return errors when reading from datastore

CHANGELOG.md

@@ -28,6 +28,8 @@ Expected: June 2024
 
 ### Corrected Bugs
 
+* Fixed: Fail on return errors when reading from datastore
+  * Can happen if running is not upgraded for example
 * Fixed: [Duplicate config files in configdir causes merge problems -> set ? = NULL](https://github.com/clicon/clixon/issues/510)
 
 ## 7.0.1

apps/backend/backend_client.c

@@ -388,14 +388,17 @@ clixon_stats_datastore_get(clixon_handle h,
     uint64_t  nr = 0;
     size_t    sz = 0;
     cxobj    *xn = NULL;
+    int       ret;
 
     clixon_debug(CLIXON_DBG_BACKEND | CLIXON_DBG_DETAIL, "%s", dbname);
     /* This is the db cache */
     if ((xt = xmldb_cache_get(h, dbname)) == NULL){
         /* Trigger cache if no exist (trick to ensure cache is present) */
-        if (xmldb_get(h, dbname, NULL, "/", &xn) < 0)
+        if ((ret = xmldb_get0(h, dbname, YB_MODULE, NULL, "/", 1, 0, &xn, NULL, NULL)) < 0)
             //goto done;
             goto ok;
+        if (ret == 0)
+            goto ok;
         xt = xmldb_cache_get(h, dbname);
     }
     if (xt != NULL){
@@ -1416,6 +1419,7 @@ from_client_stats(clixon_handle h,
     yang_stmt *yspec;
     yang_stmt *ymodext;
     cxobj     *xt = NULL;
+    int        ret;
 
     if ((str = xml_find_body(xe, "modules")) != NULL)
         modules = strcmp(str, "true") == 0;
@@ -1470,8 +1474,12 @@ from_client_stats(clixon_handle h,
     cprintf(cbret, "</module-set>");
     /* Mountpoints */
     if ((ymodext = yang_find(yspec, Y_MODULE, "ietf-yang-schema-mount")) != NULL){
-        if (xmldb_get(h, "running", NULL, "/", &xt) < 0)
+        if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/", 1, 0, &xt, NULL, NULL)) < 0)
             goto done;
+        if (ret == 0){
+            clixon_err(OE_DB, 0, "Error when reading from running, unknown error");
+            goto done;
+        }
         if (xt && yang_schema_mount_statistics(h, xt, modules, cbret) < 0)
             goto done;
     }
@@ -1793,12 +1801,15 @@ from_client_msg(clixon_handle        h,
         xnacm = NULL;
 
         /* NACM intial pre- access control enforcements. Retval:
-         * 0: Use NACM validation and xnacm is set.
+         * 0: nacm declaration error
-         * 1: Permit, skip NACM
+         * 1: Use NACM validation and xnacm is set.
+         * 2: Permit, skip NACM
          * Therefore, xnacm=NULL means no NACM checks needed.
          */
-        if ((ret = nacm_access_pre(h, ce->ce_username, username, &xnacm)) < 0)
+        if ((ret = nacm_access_pre(h, ce->ce_username, username, &xnacm, cbret)) < 0)
             goto done;
+        if (ret == 2)
+            goto reply;
         /* Cache XML NACM tree here. Use with caution, only valid on from_client_msg stack 
          */
         if (clicon_nacm_cache_set(h, xnacm) < 0)

apps/backend/backend_commit.c

@@ -974,9 +974,9 @@ from_client_restart_one(clixon_handle    h,
     if ((td = transaction_new()) == NULL)
         goto done;
     /* This is the state we are going to */
-    if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, 0, &td->td_target, NULL, NULL) < 0)
+    if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, 0, &td->td_target, NULL, &xerr)) < 0)
         goto done;
-    if ((ret = xml_yang_validate_all_top(h, td->td_target, &xerr)) < 0)
+    if (ret == 1 && (ret = xml_yang_validate_all_top(h, td->td_target, &xerr)) < 0)
         goto done;
     if (ret == 0){
         if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)

apps/backend/backend_get.c

@@ -541,12 +541,12 @@ get_list_pagination(clixon_handle        h,
     cbuf           *cbmsg = NULL; /* For error msg */
     cxobj          *xret = NULL;
     char           *xpath2; /* With optional pagination predicate */
-    int             ret;
     uint32_t        iddb; /* DBs lock, if any */
     int             locked;
     cbuf           *cberr = NULL; 
     cxobj         **xvec = NULL;
     size_t          xlen;
+    int             ret;
 #ifdef NOTYET
     cxobj          *x;
     char           *direction = NULL;
@@ -651,7 +651,7 @@ get_list_pagination(clixon_handle        h,
         /* Append predicate to original xpath and replace it */
         xpath2 = cbuf_get(cbpath);
         /* specific xpath */
-        if (xmldb_get0(h, db, YB_MODULE, nsc, xpath2?xpath2:"/", 1, wdef, &xret, NULL, NULL) < 0) {
+        if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath2?xpath2:"/", 1, wdef, &xret, NULL, &xerr)) < 0) {
             if ((cbmsg = cbuf_new()) == NULL){
                 clixon_err(OE_UNIX, errno, "cbuf_new");
                 goto done;
@@ -661,6 +661,12 @@ get_list_pagination(clixon_handle        h,
                 goto done;
             goto ok;
         }
+        if (ret == 0){
+            if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)
+                goto done;
+
+            goto ok;
+        }
         break;
     case CONTENT_NONCONFIG: /* state data only */
         if ((xret = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)/* Only top tree */
@@ -885,7 +891,7 @@ get_common(clixon_handle        h,
     switch (content){
     case CONTENT_CONFIG:    /* config data only */
         /* specific xpath. with-default gets masked in get_nacm_and_reply */
-        if (xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) {
+        if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) {
             if ((cbmsg = cbuf_new()) == NULL){
                 clixon_err(OE_UNIX, errno, "cbuf_new");
                 goto done;
@@ -895,12 +901,17 @@ get_common(clixon_handle        h,
                 goto done;
             goto ok;
         }
+        if (ret == 0){
+            if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)
+                goto done;
+            goto ok;
+        }
         break;
     case CONTENT_ALL:       /* both config and state */
     case CONTENT_NONCONFIG: /* state data only */
         if (clicon_option_bool(h, "CLICON_VALIDATE_STATE_XML")){
             /* Whole config tree, for validate debug */
-            if (xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) {
+            if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) {
                 if ((cbmsg = cbuf_new()) == NULL){
                     clixon_err(OE_UNIX, errno, "cbuf_new");
                     goto done;
@@ -910,10 +921,15 @@ get_common(clixon_handle        h,
                     goto done;
                 goto ok;
             }
+            if (ret == 0){
+                if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)
+                    goto done;
+                goto ok;
+            }
         }
         else if (content == CONTENT_ALL){
             /* specific xpath */
-            if (xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) {
+            if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) {
                 if ((cbmsg = cbuf_new()) == NULL){
                     clixon_err(OE_UNIX, errno, "cbuf_new");
                     goto done;
@@ -923,6 +939,11 @@ get_common(clixon_handle        h,
                     goto done;
                 goto ok;
             }
+            if (ret == 0){
+                if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)
+                    goto done;
+                goto ok;
+            }
         }
         /* CONTENT_NONCONFIG */
         else if ((xret = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)/* Only top tree */

apps/backend/backend_plugin_restconf.c

@@ -202,6 +202,7 @@ restconf_rpc_wrapper(clixon_handle    h,
 {
     int    retval = -1;
     cxobj *xt = NULL;
+    int    ret;
 
     clixon_debug(CLIXON_DBG_BACKEND, "");
     switch (*operation){
@@ -211,8 +212,12 @@ restconf_rpc_wrapper(clixon_handle    h,
     case PROC_OP_START:
         /* RPC op is start & enable is true, then start the service,
                            & enable is false, error or ignore it */
-        if (xmldb_get(h, "running", NULL,  "/restconf", &xt) < 0)
+        if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/restconf", 1, 0, &xt, NULL, NULL)) < 0)
             goto done;
+        if (ret == 0){
+            clixon_err(OE_DB, 0, "Error when reading from running, unknown error");
+            goto done;
+        }
         if (xt != NULL &&
             xpath_first(xt, NULL, "/restconf[enable='false']") != NULL) {
             *operation = PROC_OP_NONE;

apps/backend/backend_startup.c

@@ -309,8 +309,12 @@ startup_extraxml(clixon_handle h,
      * It should be empty if extra-xml is null and reset plugins did nothing
      * then skip validation.
      */
-    if (xmldb_get(h, tmp_db, NULL, NULL, &xt0) < 0)
+    if ((ret = xmldb_get0(h, tmp_db, YB_MODULE, NULL, NULL, 1, 0, &xt0, NULL, NULL)) < 0)
         goto done;
+    if (ret == 0){
+        clixon_err(OE_DB, 0, "Error when reading from %s, unknown error", tmp_db);
+        goto done;
+    }
     if ((ret = xmldb_empty_get(h, tmp_db)) < 0)
         goto done;
     if (ret == 1)

example/main/example_backend.c

@@ -467,6 +467,7 @@ example_statedata(clixon_handle   h,
     char      *name;
     cvec      *nsc1 = NULL;
     yang_stmt *yspec = NULL;
+    int        ret;
 
     if (!_state)
         goto ok;
@@ -481,8 +482,12 @@ example_statedata(clixon_handle   h,
      * Get config according to xpath */
     if ((nsc1 = xml_nsctx_init(NULL, "urn:ietf:params:xml:ns:yang:ietf-interfaces")) == NULL)
         goto done;
-    if (xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, 0, &xt, NULL, NULL) < 0)
+    if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, 0, &xt, NULL, NULL)) < 0)
         goto done;
+    if (ret == 0){
+        clixon_err(OE_DB, 0, "Error when reading from running, unknown error");
+        goto done;
+    }
     if (xpath_vec(xt, nsc1, "/interfaces/interface/name", &xvec, &xlen) < 0)
         goto done;
     if (xlen){

lib/clixon/clixon_nacm.h

@@ -62,7 +62,7 @@ int nacm_datanode_read(clixon_handle h, cxobj *xt, cxobj **xvec, size_t xlen, ch
 int nacm_datanode_write(clixon_handle h, cxobj *xr, cxobj *xt,
                         enum nacm_access access,
                         char *username, cxobj *xnacm, cbuf *cbret);
-int nacm_access_pre(clixon_handle h, char *peername, char *username, cxobj **xnacmp);
+int nacm_access_pre(clixon_handle h, char *peername, char *username, cxobj **xnacmp, cbuf *cbret);
 int verify_nacm_user(clixon_handle h, enum nacm_credentials_t cred, char *peername, char *nacmname, char *rpcname, cbuf *cbret);
 
 #endif /* _CLIXON_NACM_H */

lib/src/clixon_datastore_read.c

@@ -817,11 +817,14 @@ xmldb_get_cache(clixon_handle     h,
  * @retval    -1      Error
  * @note Use of 1 for OK
  * @code
- *   if (xmldb_get(xh, "running", NULL, "/interfaces/interface[name="eth"]", &xt) < 0)
+ *   if ((ret = xmldb_get(xh, "running", NULL, "/interfaces/interface[name="eth"]", &xt)) < 0)
+ *      err;
+ *   if (ret == 0)
  *      err;
  *   xml_free(xt);
  * @endcode
  * @see xmldb_get0  Underlying more capable API for enabling zero-copy
+ * XXX: OBSOLETE: use xmldb_get0 directly
  */
 int
 xmldb_get(clixon_handle    h,

lib/src/clixon_nacm.c

@@ -72,6 +72,7 @@
 #include "clixon_datastore.h"
 #include "clixon_xml_nsctx.h"
 #include "clixon_xml_map.h"
+#include "clixon_xml_io.h"
 #include "clixon_path.h"
 #include "clixon_xml_vec.h"
 #include "clixon_nacm.h"
@@ -1196,7 +1197,9 @@ nacm_access_check(clixon_handle h,
  * @param[in]  h        Clixon handle
  * @param[in]  peername Peer username if any
  * @param[in]  username User name of requestor
- * @param[out] xncam    NACM XML tree, set if retval=0. Free after use
+ * @param[out] xnacm    NACM XML tree, set if retval=0. Free after use
+ * @param[out] cbret    Error if ret == 2
+ * @retval     2        Failed on reading NACM from running (internal), cbret has error
  * @retval     1        OK permitted. You do not need to do next NACM step.
  * @retval     0        OK but not validated. Need to do NACM step using xnacm
  * @retval    -1        Error
@@ -1215,7 +1218,8 @@ int
 nacm_access_pre(clixon_handle  h,
                 char          *peername,
                 char          *username,
-                cxobj        **xnacmp)
+                cxobj        **xnacmp,
+                cbuf          *cbret)
 {
     int    retval = -1;
     char  *mode;
@@ -1223,6 +1227,8 @@ nacm_access_pre(clixon_handle  h,
     cxobj *xnacm0 = NULL;
     cxobj *xnacm = NULL;
     cvec  *nsc = NULL;
+    cxobj *xerr = NULL;
+    int    ret;
 
     /* Check clixon option: disabled, external tree or internal */
     mode = clicon_option_str(h, "CLICON_NACM_MODE");
@@ -1236,8 +1242,13 @@ nacm_access_pre(clixon_handle  h,
                 goto done;
     }
     else if (strcmp(mode, "internal")==0){
-        if (xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, 0, &xnacm0, NULL, NULL) < 0)
+        if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, 0, &xnacm0, NULL, &xerr)) < 0)
             goto done;
+        if (ret == 0){
+            if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0)
+                goto done;
+            goto fail;
+        }
     }
     else{
         clixon_err(OE_XML, 0, "Invalid NACM mode: %s", mode);
@@ -1268,10 +1279,15 @@ nacm_access_pre(clixon_handle  h,
         xml_free(xnacm0);
     else if (xnacm)
         xml_free(xnacm);
+    else if (xerr)
+        xml_free(xerr);
     return retval;
  permit:
     retval = 1;
     goto done;
+ fail:
+    retval = 2;
+    goto done;
 }
 
 /*! Verify nacm user with peer uid credentials