diff --git a/CHANGELOG.md b/CHANGELOG.md index 124100f27..8411e995d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,7 +92,7 @@ ``` ### Minor changes -* Empty leaf values, eg are now checked at vlidation. +* Empty leaf values, eg 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 diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index c75921cfd..419c8a5f8 100644 --- a/apps/cli/cli_common.c +++ b/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; diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c index ee7e75111..65a096531 100644 --- a/apps/cli/cli_show.c +++ b/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 ("" "") - * @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; diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index b42436895..7ada04988 100644 --- a/apps/restconf/restconf_methods.c +++ b/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: */ } - 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) diff --git a/lib/clixon/clixon_xml_map.h b/lib/clixon/clixon_xml_map.h index f4e399583..c1398cd25 100644 --- a/lib/clixon/clixon_xml_map.h +++ b/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); diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 768b63c65..d8f57149a 100644 --- a/lib/src/clixon_plugin.c +++ b/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"); diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 957a81354..f7bc7b4ab 100644 --- a/lib/src/clixon_xml.c +++ b/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, diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 2c33fa892..d49257f3e 100644 --- a/lib/src/clixon_xml_map.c +++ b/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); diff --git a/test/test_restconf.sh b/test/test_restconf.sh index a5a763217..4b3ab3dcc 100755 --- a/test/test_restconf.sh +++ b/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