* Backend plugin returning NULL was still installed - is now logged a…

…nd skipped.

* [Parent list key is not validated if not provided via RESTCONF #83](#83), thanks achernavin22.

CHANGELOG.md

@@ -92,7 +92,7 @@
 ```
 
 ### Minor changes
-* Empty leaf values, eg <a></a> are now checked at vlidation.
+* Empty leaf values, eg <a></a> are now checked at validation.
   * Empty values were skipped in validation.
   * They are now checked and invalid for ints, dec64, etc, but are treated as empty string "" for string types.
 * Optimized validation by making xml_diff work on raw cache tree (not copies)
@@ -118,6 +118,8 @@
 * Added libgen.h for baseline()
 
 ### Corrected Bugs
+* Backend plugin returning NULL was still installed - is now logged and skipped.
+* [Parent list key is not validated if not provided via RESTCONF #83](https://github.com/clicon/clixon/issues/83), thanks achernavin22.
 * [Invalid JSON if GET /operations via RESTCONF #82](https://github.com/clicon/clixon/issues/82), thanks achernavin22
 * List ordering bug - lists with ints as keys behaved wrongly and slow.
 * NACM read default rule did not work properly if nacm was enabled AND no groups were defined 

apps/cli/cli_common.c

@@ -236,7 +236,7 @@ cli_dbxml(clicon_handle       h,
     if ((xtop = xml_new("config", NULL, NULL)) == NULL)
 	goto done;
     xbot = xtop;
-    if (api_path && api_path2xml(api_path, yspec, xtop, YC_DATANODE, &xbot, &y) < 1)
+    if (api_path && api_path2xml(api_path, yspec, xtop, YC_DATANODE, 1, &xbot, &y) < 1)
 	goto done; 
     if ((xa = xml_new("operation", xbot, NULL)) == NULL)
 	goto done;

apps/cli/cli_show.c

@@ -84,7 +84,6 @@
  * @param[in]   name     Name of this function (eg "expand_dbvar")
  * @param[in]   cvv      The command so far. Eg: cvec [0]:"a 5 b"; [1]: x=5;
  * @param[in]   argv     Arguments given at the callback ("<db>" "<xmlkeyfmt>")
- * @param[out]  len      len of return commands & helptxt 
  * @param[out]  commands vector of function pointers to callback functions
  * @param[out]  helptxt  vector of pointers to helptexts
  * @see cli_expand_var_generate  This is where arg is generated
@@ -171,7 +170,7 @@ expand_dbvar(void   *h,
     /* This is primarily to get "y", 
      * xpath2xml would have worked!!
      */
-    if (api_path && api_path2xml(api_path, yspec, xtop, YC_DATANODE, &xbot, &y) < 1)
+    if (api_path && api_path2xml(api_path, yspec, xtop, YC_DATANODE, 0, &xbot, &y) < 1)
 	goto done;
     if (y==NULL)
 	goto ok;

apps/restconf/restconf_methods.c

@@ -469,7 +469,7 @@ api_data_post(clicon_handle h,
     /* Translate api_path to xtop/xbot */
     xbot = xtop;
     if (api_path){
-	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, &xbot, &y)) < 0)
+	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, 1, &xbot, &y)) < 0)
 	    goto done;
 	if (ret == 0){ /* validation failed */
 	    if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0)
@@ -738,7 +738,7 @@ api_data_put(clicon_handle h,
     /* Translate api_path to xtop/xbot */
     xbot = xtop;
     if (api_path){
-	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, &xbot, &y)) < 0)
+	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, 1, &xbot, &y)) < 0)
 	    goto done;
 	if (ret == 0){ /* validation failed */
 	    if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0)
@@ -1011,7 +1011,7 @@ api_data_delete(clicon_handle h,
 	goto done;
     xbot = xtop;
     if (api_path){
-	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, &xbot, &y)) < 0)
+	if ((ret = api_path2xml(api_path, yspec, xtop, YC_DATANODE, 1, &xbot, &y)) < 0)
 	    goto done;
 	if (ret == 0){ /* validation failed */
 	    if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0)
@@ -1598,7 +1598,7 @@ api_operations_post(clicon_handle h,
 	    goto done;
 	/* Here xtop is: <rpc username="foo"/> */
     }
-    if ((ret = api_path2xml(oppath, yspec, xtop, YC_SCHEMANODE, &xbot, &y)) < 0)
+    if ((ret = api_path2xml(oppath, yspec, xtop, YC_SCHEMANODE, 1, &xbot, &y)) < 0)
 	goto done;
     if (ret == 0){ /* validation failed */
 	if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0)

lib/clixon/clixon_xml_map.h

@@ -68,7 +68,7 @@ int xml_spec_populate_rpc(clicon_handle h, cxobj *x, yang_stmt *yspec);
 int xml_spec_populate(cxobj *x, void *arg);
 int api_path2xpath(yang_stmt *yspec, cvec *cvv, int offset, cbuf *xpath);
 int api_path2xml(char *api_path, yang_stmt *yspec, cxobj *xtop, 
-		 yang_class nodeclass, cxobj **xpathp, yang_stmt **ypathp);
+		 yang_class nodeclass, int strict, cxobj **xpathp, yang_stmt **ypathp);
 int xml_merge(cxobj *x0, cxobj *x1, yang_stmt *yspec, char **reason);
 int yang_enum_int_value(cxobj *node, int32_t *val);
 

lib/src/clixon_plugin.c

@@ -173,23 +173,27 @@ clixon_plugin_find(clicon_handle h,
  * @param[in]  file    Which plugin to load
  * @param[in]  function Which function symbol to load and call
  * @param[in]  dlflags See man(3) dlopen
- * @retval     cp      Clixon plugin structure
- * @retval     NULL    Error
+ * @param[out] cpp      Clixon plugin structure (if retval is 1)
+ * @retval     1     OK
+ * @retval     0     Failed load, log, skip and continue with other plugins
+ * @retval     -1    Error
  * @see clixon_plugins_load  Load all plugins
  */
-static clixon_plugin *
+static int
 plugin_load_one(clicon_handle   h, 
 		char           *file,
 		char           *function,
-		int             dlflags)
+		int             dlflags,
+		clixon_plugin **cpp)
 {
-    char          *error;
-    void          *handle = NULL;
-    plginit2_t    *initfn;
+    int                retval = -1;
+    char              *error;
+    void              *handle = NULL;
+    plginit2_t        *initfn;
     clixon_plugin_api *api = NULL;
-    clixon_plugin *cp = NULL;
-    char          *name;
-    char          *p;
+    clixon_plugin     *cp = NULL;
+    char              *name;
+    char              *p;
 
     clicon_debug(1, "%s file:%s function:%s", __FUNCTION__, file, function);
     dlerror();    /* Clear any existing error */
@@ -201,7 +205,7 @@ plugin_load_one(clicon_handle   h,
     /* call plugin_init() if defined, eg CLIXON_PLUGIN_INIT or CLIXON_BACKEND_INIT */
     if ((initfn = dlsym(handle, function)) == NULL){
 	clicon_err(OE_PLUGIN, errno, "Failed to find %s when loading clixon plugin %s", CLIXON_PLUGIN_INIT, file);
-	goto err;
+	goto done;
     }
     if ((error = (char*)dlerror()) != NULL) {
 	clicon_err(OE_UNIX, 0, "dlsym: %s: %s", file, error);
@@ -211,11 +215,12 @@ plugin_load_one(clicon_handle   h,
     if ((api = initfn(h)) == NULL) {
 	if (!clicon_errno){ 	/* if clicon_err() is not called then log and continue */
 	    clicon_log(LOG_DEBUG, "Warning: failed to initiate %s", strrchr(file,'/')?strchr(file, '/'):file);
-	    dlclose(handle);
+	    retval = 0;
+	    goto done;
 	}
 	else{
 	    clicon_err(OE_PLUGIN, errno, "Failed to initiate %s", strrchr(file,'/')?strchr(file, '/'):file);
-	    goto err;
+	    goto done;
 	}
     }
     /* Note: sizeof clixon_plugin_api which is largest of clixon_plugin_api:s */
@@ -235,15 +240,19 @@ plugin_load_one(clicon_handle   h,
 
     snprintf(cp->cp_name, sizeof(cp->cp_name), "%*s",
 	     (int)strlen(name), name);
-    if (api)
-	cp->cp_api = *api;
+    cp->cp_api = *api;
     clicon_debug(1, "%s", __FUNCTION__);
+    if (cp){
+	*cpp = cp;
+	cp = NULL;
+    }
+    retval = 1;
  done:
-    return cp;
- err:
-    if (handle)
+    if (retval != 1 && handle)
 	dlclose(handle);
-    return NULL;
+    if (cp)
+	free(cp);
+    return retval;
 }
 
 /*! Load a set of plugin objects from a directory and and call their init-function
@@ -266,6 +275,7 @@ clixon_plugins_load(clicon_handle h,
     int            i;
     char           filename[MAXPATHLEN];
     clixon_plugin *cp;
+    int            ret;
 
     clicon_debug(1, "%s", __FUNCTION__); 
     /* Get plugin objects names from plugin directory */
@@ -276,8 +286,10 @@ clixon_plugins_load(clicon_handle h,
 	snprintf(filename, MAXPATHLEN-1, "%s/%s", dir, dp[i].d_name);
 	clicon_debug(1, "DEBUG: Loading plugin '%.*s' ...", 
 		     (int)strlen(filename), filename);
-	if ((cp = plugin_load_one(h, filename, function, RTLD_NOW)) == NULL)
+	if ((ret = plugin_load_one(h, filename, function, RTLD_NOW, &cp)) < 0)
 	    goto done;
+	if (ret == 0)
+	    continue;
 	_clixon_nplugins++;
 	if ((_clixon_plugins = realloc(_clixon_plugins, _clixon_nplugins*sizeof(clixon_plugin))) == NULL) {
 	    clicon_err(OE_UNIX, errno, "realloc");

lib/src/clixon_xml.c

@@ -791,7 +791,7 @@ xml_cv_set(cxobj  *x,
  *
  * @retval xmlobj     if found.
  * @retval NULL       if no such node found.
- * @see xml_find_type  wich is a more generic function
+ * @see xml_find_type  which is a more generic function
  */
 cxobj *
 xml_find(cxobj *x_up, 

lib/src/clixon_xml_map.c

@@ -1971,6 +1971,7 @@ api_path2xml_vec(char             **vec,
 		 cxobj             *x0,
 		 yang_stmt         *y0,
 		 yang_class         nodeclass,
+		 int                strict,
 		 cxobj            **xpathp,
 		 yang_stmt        **ypathp)
 {
@@ -2053,9 +2054,10 @@ api_path2xml_vec(char             **vec,
 	    valvec = NULL;
 	}
 	if (restval==NULL){
-	    // XXX patch to allow for lists without restval to be backward compat
-	    //	    clicon_err(OE_XML, 0, "malformed key, expected '=restval'");
-	    //	    goto done;
+	    if (strict){
+		clicon_err(OE_XML, 0, "malformed key, expected '=restval'");
+	        goto fail;
+	    }
 	}
 	else{
 	    if ((valvec = clicon_strsep(restval, ",", &nvalvec)) == NULL)
@@ -2086,9 +2088,11 @@ api_path2xml_vec(char             **vec,
 	}
 	break;
     default: /* eg Y_CONTAINER, Y_LEAF */
-	if ((x = xml_new(name, x0, y)) == NULL)
-	    goto done;
-	xml_type_set(x, CX_ELMNT);
+	if ((x = xml_find_type(x0, NULL, name, CX_ELMNT)) == NULL){ /* eg key of list */
+	    if ((x = xml_new(name, x0, y)) == NULL)
+		goto done;
+	    xml_type_set(x, CX_ELMNT);
+	}
 	break;
     }
     if (x && namespace){
@@ -2097,7 +2101,7 @@ api_path2xml_vec(char             **vec,
     }
     if ((retval = api_path2xml_vec(vec+1, nvec-1, 
 				   x, y, 
-				   nodeclass,
+				   nodeclass, strict,
 				   xpathp, ypathp)) < 1)
 	goto done;
  ok:
@@ -2144,6 +2148,7 @@ api_path2xml(char       *api_path,
 	     yang_stmt  *yspec,
 	     cxobj      *xtop,
 	     yang_class  nodeclass,
+	     int         strict,
 	     cxobj     **xbotp,
 	     yang_stmt **ybotp)
 {
@@ -2169,7 +2174,7 @@ api_path2xml(char       *api_path,
     }
     nvec--; /* NULL-terminated */
     if ((retval = api_path2xml_vec(vec+1, nvec, 
-				   xtop, yspec, nodeclass,
+				   xtop, yspec, nodeclass, strict,
 				   xbotp, ybotp)) < 1)
 	goto done;
     xml_yang_root(*xbotp, &xroot);

test/test_restconf.sh

@@ -263,6 +263,12 @@ if [ -z "$match" ]; then
     err "$expect" "$ret"
 fi
 
+new "restconf Add subtree without key (expected error)"
+expecteq "$(curl -s -X PUT -d '{"ietf-interfaces:interface":{"name":"eth/0/0","type":"ex:eth","enabled":true}}' http://localhost/restconf/data/ietf-interfaces:interfaces/interface)" 0 '{"ietf-restconf:errors" : {"error": {"error-type": "rpc","error-tag": "malformed-message","error-severity": "error","error-message": "malformed key, expected '"'"'=restval'"'"'"}}}'
+
+new "restconf Add subtree with too many keys (expected error)"
+expecteq "$(curl -s -X PUT -d '{"ietf-interfaces:interface":{"name":"eth/0/0","type":"ex:eth","enabled":true}}' http://localhost/restconf/data/ietf-interfaces:interfaces/interface=a,b)" 0 '{"ietf-restconf:errors" : {"error": {"error-type": "rpc","error-tag": "malformed-message","error-severity": "error","error-message": "List key interface length mismatch"}}}'
+
 new "Kill restconf daemon"
 stop_restconf