From d9136c89722e7693a5d5bf82231c94aa5a435ccd Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 29 Sep 2019 14:45:08 +0200 Subject: [PATCH] * Stricter handling of multi-namespace handling * This occurs in cases where there are more than one XML namespaces in a config tree, such as `augment`:ed trees. * Affects all parts of the system, including datastore, backend, restconf and cli. * Invalid api-path syntax (eg non-matching yang) error changed from 412 operation-failed to 400 Bad request invalid-value, or unknown-element. --- CHANGELOG.md | 21 +- apps/backend/backend_client.c | 3 +- apps/backend/backend_commit.c | 2 + apps/backend/backend_startup.c | 11 +- apps/restconf/restconf_lib.c | 1 + apps/restconf/restconf_methods_get.c | 41 ++-- lib/clixon/clixon_proto_client.h | 1 + lib/clixon/clixon_xml.h | 8 + lib/clixon/clixon_xml_map.h | 1 + lib/clixon/clixon_yang_module.h | 4 +- lib/src/clixon_datastore_read.c | 6 + lib/src/clixon_datastore_write.c | 327 +++++++++++++++++++-------- lib/src/clixon_proto_client.c | 76 ++++++- lib/src/clixon_xml.c | 116 ++++++++-- lib/src/clixon_xml_map.c | 258 ++++++++++++++++++--- lib/src/clixon_xml_nsctx.c | 4 +- lib/src/clixon_xml_sort.c | 7 +- test/test_augment.sh | 17 +- test/test_cli.sh | 1 + test/test_leafref.sh | 40 ++-- test/test_netconf.sh | 6 +- test/test_restconf_err.sh | 62 +++-- 22 files changed, 777 insertions(+), 236 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06dd9fcb..5f5e9a06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Clixon Changelog -## 4.2.0 (Expected: September) +## 4.2.0 (Expected: October) ### Major New features * Backend daemon can drop privileges after initialization to run as non-privileged user @@ -10,17 +10,15 @@ * If dropped temporary, you can restore privileges with `restore_priv()` ### API changes on existing features (you may need to change your code) +* Stricter handling of multi-namespace handling + * This occurs in cases where there are more than one XML namespaces in a config tree, such as `augment`:ed trees. + * Affects all parts of the system, including datastore, backend, restconf and cli. + * Examples of a mandated stricter usage of a simple augment `b` of symbol `a`. Assume `a` is in module `mod1` with namespace `urn:example:a` and `b` is in module `mod2` with namespace `urn:example:b`: + * RESTCONF: `GET http://localhost/restconf/data/mod1:a/mod2:b` + * NETCONF: `42` + * XPATH (in edit-config filter): `` * RESTCONF error reporting - * Invalid api-path syntax (eg non-matching yang) error changed from 412 operation-failed to 400 unknown-element / invalid-value. For example, change from - ``` - HTTP/1.1 412 Precondition Failed - {"ietf-restconf:errors":{"error":{"error-type":"protocol","error-tag":"operation-failed","error-severity":"error","error-message":"No such yang module: badmodule"}}} - ``` - to: - ``` - HTTP/1.1 400 Bad Request - {"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"No such yang module: badmodule"}}} - ``` + * Invalid api-path syntax (eg non-matching yang) error changed from 412 operation-failed to 400 Bad request invalid-value, or unknown-element. * Typical installation should now add a `clicon` user (as well as group) * New clixon-config@2019-09-11.yang revision * Added: CLICON_BACKEND_USER: drop of privileges to user, @@ -33,6 +31,7 @@ * Configure and test modification for better Freebsd port ### Corrected Bugs +* See "Stricter handling of multi-namespace handling" in API-changes above. * Hello netconf candidate capability misspelled, mentioned in [Can clixon_netconf receive netconf packets as a server? #93](https://github.com/clicon/clixon/issues/93) * [Cannot write to config using restconf example #91](https://github.com/clicon/clixon/issues/91) * Updated restconf documentation (the example was wrong) diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index bbdb0b54..385ca4de 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -527,11 +527,10 @@ from_client_edit_config(clicon_handle h, if (xml_spec(xc) != NULL) xml_spec_set(xc, NULL); /* Populate XML with Yang spec (why not do this in parser?) - * Maybe validate xml here as in text_modify_top? */ if (xml_apply(xc, CX_ELMNT, xml_spec_populate, yspec) < 0) goto done; - + /* Maybe validate xml here as in text_modify_top? */ if (xml_apply(xc, CX_ELMNT, xml_non_config_data, &non_config) < 0) goto done; if (non_config){ diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 3c115532..d67f1490 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -195,10 +195,12 @@ startup_common(clicon_handle h, goto ok; } if (msd){ + /* Here xt is old syntax */ if ((ret = clixon_module_upgrade(h, xt, msd, cbret)) < 0) goto done; if (ret == 0) goto fail; + /* Here xt is new syntax */ } if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, 0, "Yang spec not set"); diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index 35204067..96f35baa 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -70,7 +70,6 @@ #include "backend_commit.h" #include "backend_startup.h" - /*! Merge db1 into db2 without commit * @retval -1 Error * @retval 0 Validation failed (with cbret set) @@ -166,9 +165,10 @@ load_extraxml(clicon_handle h, const char *db, cbuf *cbret) { - int retval = -1; - cxobj *xt = NULL; - int fd = -1; + int retval = -1; + cxobj *xt = NULL; + int fd = -1; + yang_stmt *yspec = NULL; if (filename == NULL) return 1; @@ -176,7 +176,8 @@ load_extraxml(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", filename); goto done; } - if (xml_parse_file(fd, "", NULL, &xt) < 0) + yspec = clicon_dbspec_yang(h); + if (xml_parse_file(fd, "", yspec, &xt) < 0) goto done; /* Replace parent w first child */ if (xml_rootchild(xt, 0, &xt) < 0) diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index 268974bb..17f45500 100644 --- a/apps/restconf/restconf_lib.c +++ b/apps/restconf/restconf_lib.c @@ -526,6 +526,7 @@ api_return_err(clicon_handle h, FCGX_FPrintF(r->out, "%s", cbuf_get(cb)); FCGX_FPrintF(r->out, "}\r\n"); } + break; default: clicon_err(OE_YANG, EINVAL, "Invalid media type %d", media); goto done; diff --git a/apps/restconf/restconf_methods_get.c b/apps/restconf/restconf_methods_get.c index 8d8a72f6..1a9214dc 100644 --- a/apps/restconf/restconf_methods_get.c +++ b/apps/restconf/restconf_methods_get.c @@ -165,8 +165,16 @@ api_data_get2(clicon_handle h, if ((cbpath = cbuf_new()) == NULL) goto done; cprintf(cbpath, "/"); - /* We know "data" is element pi-1 */ - if ((ret = api_path2xpath_cvv(pcvec, pi, yspec, cbpath, &namespace, &xerr)) < 0) + /* Create a namespace context for ymod as the default namespace to use with + * xpath expressions */ + if ((nsc = cvec_new(0)) == NULL){ + clicon_err(OE_XML, errno, "cvec_new"); + goto done; + } + /* We know "data" is element pi-1. + * Translate api-path to xpath: xpath (cbpath) and namespace context (nsc) + */ + if ((ret = api_path2xpath_cvv2(pcvec, pi, yspec, cbpath, nsc, &xerr)) < 0) goto done; if (ret == 0){ clicon_err_reset(); @@ -178,36 +186,17 @@ api_data_get2(clicon_handle h, goto done; goto ok; } - if (ret == 0){ - if (netconf_operation_failed_xml(&xerr, "protocol", clicon_err_reason) < 0) - goto done; - clicon_err_reset(); - if ((xe = xpath_first(xerr, "rpc-error")) == NULL){ - clicon_err(OE_XML, EINVAL, "rpc-error not found (internal error)"); - goto done; - } - if (api_return_err(h, r, xe, pretty, media_out, 0) < 0) - goto done; - goto ok; - } xpath = cbuf_get(cbpath); clicon_debug(1, "%s path:%s", __FUNCTION__, xpath); - /* Create a namespace context for ymod as the default namespace to use with - * xpath expressions */ - if ((nsc = xml_nsctx_init(NULL, namespace)) == NULL) - goto done; switch (content){ case CONTENT_CONFIG: - ret = clicon_rpc_get(h, xpath, namespace, CONTENT_CONFIG, depth, &xret); - break; case CONTENT_NONCONFIG: - ret = clicon_rpc_get(h, xpath, namespace, CONTENT_NONCONFIG, depth, &xret); - break; case CONTENT_ALL: - ret = clicon_rpc_get(h, xpath, namespace, CONTENT_ALL, depth, &xret); + ret = clicon_rpc_get_nsc(h, xpath, nsc, content, depth, &xret); break; default: clicon_err(OE_XML, EINVAL, "Invalid content attribute %d", content); + goto done; break; } if (ret < 0){ @@ -291,14 +280,14 @@ api_data_get2(clicon_handle h, switch (media_out){ case YANG_DATA_XML: for (i=0; i */ + if (depth != -1) + cprintf(cb, " depth=\"%d\"", depth); + cprintf(cb, ">"); + if (xpath && strlen(xpath)) { + cprintf(cb, "<%s:filter %s:type=\"xpath\" %s:select=\"%s\"", + NETCONF_BASE_PREFIX, NETCONF_BASE_PREFIX, NETCONF_BASE_PREFIX, + xpath); + + while ((cv = cvec_each(nsc, cv)) != NULL){ + cprintf(cb, " xmlns"); + if ((prefix = cv_name_get(cv))) + cprintf(cb, ":%s", prefix); + cprintf(cb, "=\"%s\"", cv_string_get(cv)); + } + cprintf(cb, "/>"); + } + cprintf(cb, ""); + if ((msg = clicon_msg_encode("%s", cbuf_get(cb))) == NULL) + goto done; + if (clicon_rpc_msg(h, msg, &xret, NULL) < 0) + goto done; + /* Send xml error back: first check error, then ok */ + if ((xd = xpath_first(xret, "/rpc-reply/rpc-error")) != NULL) + xd = xml_parent(xd); /* point to rpc-reply */ + else if ((xd = xpath_first(xret, "/rpc-reply/data")) == NULL) + if ((xd = xml_new("data", NULL, NULL)) == NULL) + goto done; + if (xt){ + if (xml_rm(xd) < 0) + goto done; + *xt = xd; + } + retval = 0; + done: + if (cb) + cbuf_free(cb); + if (xret) + xml_free(xret); + if (msg) + free(msg); + return retval; +} + + /*! Close a (user) session * @param[in] h CLICON handle * @retval 0 OK diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index c5b1715f..f674b219 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -197,9 +197,9 @@ xml_name_set(cxobj *xn, return 0; } -/*! Get namespace of xnode - * @param[in] xn xml node - * @retval namespace of xml node +/*! Get prefix of xnode + * @param[in] xn xml node + * @retval prefix of xml node */ char* xml_prefix(cxobj *xn) @@ -207,9 +207,9 @@ xml_prefix(cxobj *xn) return xn->x_prefix; } -/*! Set name space of xnode, namespace is copied +/*! Set prefix of xnode, prefix is copied * @param[in] xn xml node - * @param[in] localname new namespace, null-terminated string, copied by function + * @param[in] localname new prefix, null-terminated string, copied by function * @retval -1 on error with clicon-err set * @retval 0 OK */ @@ -230,14 +230,14 @@ xml_prefix_set(cxobj *xn, return 0; } -/*! Get cached namespace +/*! Get cached namespace (given prefix) * @param[in] x XML node * @param[in] prefix Namespace prefix, or NULL for default * @retval ns Cached namespace * @retval NULL No namespace found (not cached or not found) * @note may want to distinguish between not set cache and no namespace? */ -static char* +char* nscache_get(cxobj *x, char *prefix) { @@ -246,6 +246,35 @@ nscache_get(cxobj *x, return NULL; } +/*! Get cached prefix (given namespace) + * @param[in] x XML node + * @param[in] namespace + * @param[out] prefix + * @retval 0 No prefix found + * @retval 1 Prefix found + */ +int +nscache_get_prefix(cxobj *x, + char *namespace, + char **prefix) +{ + if (x->x_ns_cache != NULL) + return xml_nsctx_get_prefix(x->x_ns_cache, namespace, prefix); + return 0; +} + +/*! Dump whole namespacet context cache of one xml node + * @param[in] x XML node + * @retval nsc Whole namespace context of x + * @retval NULL Empty nsc + * @see nscache_get For a single prefix + */ +cvec * +nscache_get_all(cxobj *x) +{ + return x->x_ns_cache; +} + /*! Set cached namespace for specific namespace. Replace if necessary * @param[in] x XML node * @param[in] prefix Namespace prefix, or NULL for default @@ -254,7 +283,7 @@ nscache_get(cxobj *x, * @retval -1 Error * @see nscache_replace to replace the whole context */ -static int +int nscache_set(cxobj *x, char *prefix, char *namespace) @@ -281,7 +310,7 @@ nscache_set(cxobj *x, */ int nscache_replace(cxobj *x, - cvec *nsc) + cvec *nsc) { int retval = -1; @@ -405,6 +434,63 @@ xmlns_set(cxobj *x, return retval; } +/*! Get namespace given prefix recursively + * @param[in] xn XML node + * @param[in] namespace Namespace + * @param[out] prefixp Pointer to prefix if found + * @retval -1 Error + * @retval 0 No namespace found + * @retval 1 Namespace found + */ +int +xml2prefix(cxobj *xn, + char *namespace, + char **prefixp) +{ + int retval = -1; + cxobj *xa = NULL; + char *prefix = NULL; + + while (xn != NULL){ + if (nscache_get_prefix(xn, namespace, &prefix) == 1) /* found */ + goto found; + // if (xn->x_ns_cache == NULL){ /* Look in node */ + xa = NULL; + while ((xa = xml_child_each(xn, xa, CX_ATTR)) != NULL) { + /* xmlns=namespace */ + if (strcmp("xmlns", xml_name(xa)) == 0){ + if (strcmp(xml_value(xa), namespace) == 0){ + clicon_debug(1, "%sA NULL %s", __FUNCTION__, namespace); + if (nscache_set(xn, NULL, namespace) < 0) + goto done; + prefix = NULL; + goto found; + } + + } + /* xmlns:prefix=namespace */ + else if (strcmp("xmlns", xml_prefix(xa)) == 0){ + if (strcmp(xml_value(xa), namespace) == 0){ + prefix = xml_name(xa); + assert(strcmp(prefix, "xmlns")); + if (nscache_set(xn, prefix, namespace) < 0) + goto done; + goto found; + } + } + } + // } + xn = xml_parent(xn); + } + retval = 0; + done: + return retval; + found: + *prefixp = prefix; + retval = 1; + goto done; +} + /*! See if xmlns:[=] exists, if so return * * @param[in] xn XML node @@ -982,6 +1068,7 @@ int xml_addsub(cxobj *xp, cxobj *xc) { + int retval = -1; cxobj *oldp; int i; char *pns = NULL; /* parent namespace */ @@ -1000,13 +1087,14 @@ xml_addsub(cxobj *xp, /* Add xc to new parent */ if (xp){ if (xml_child_append(xp, xc) < 0) - return -1; + goto done; /* Set new parent in child */ xml_parent_set(xc, xp); /* Ensure default namespace is not duplicated * here only remove duplicate default namespace, there may be more */ /* 1. Get parent default namespace */ - xml2ns(xp, NULL, &pns); + if (xml2ns(xp, NULL, &pns) < 0) + goto done; /* 2. Get child default namespace */ if (pns && (xa = xml_find_type(xc, NULL, "xmlns", CX_ATTR)) != NULL && @@ -1016,9 +1104,11 @@ xml_addsub(cxobj *xp, xml_purge(xa); } /* clear namespace context cache of child */ - nscache_clear(xp); + nscache_clear(xc); } - return 0; + retval = 0; + done: + return retval; } /*! Wrap a new node between a parent xml node (xp) and all its children diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index bbc86241..0c02bfe5 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -1749,54 +1749,59 @@ yang2api_path_fmt_1(yang_stmt *ys, cbuf *cb) { yang_stmt *yp; /* parent */ + yang_stmt *ymod; int i; cvec *cvk = NULL; /* vector of index keys */ int retval = -1; - if ((yp = ys->ys_parent) == NULL){ - clicon_err(OE_YANG, EINVAL, "yang expected parent %s", ys->ys_argument); + if ((yp = yang_parent_get(ys)) == NULL){ + clicon_err(OE_YANG, EINVAL, "yang expected parent %s", yang_argument_get(ys)); goto done; } if (yp != NULL && /* XXX rm */ - yp->ys_keyword != Y_MODULE && - yp->ys_keyword != Y_SUBMODULE){ + yang_keyword_get(yp) != Y_MODULE && + yang_keyword_get(yp) != Y_SUBMODULE){ + if (yang2api_path_fmt_1((yang_stmt *)yp, 1, cb) < 0) /* recursive call */ goto done; - if (yp->ys_keyword != Y_CHOICE && yp->ys_keyword != Y_CASE){ + if (yang_keyword_get(yp) != Y_CHOICE && yang_keyword_get(yp) != Y_CASE){ #if 0 /* In some cases, such as cli_show_auto, a trailing '/' should * NOT be present if ys is a key in a list. * But in other cases (I think most), the / should be there, * so a patch is added in cli_show_auto instead. */ - if (ys->ys_keyword == Y_LEAF && yp && - yp->ys_keyword == Y_LIST && + if (yang_keyword_get(ys) == Y_LEAF && yp && + yang_keyword_get(yp) == Y_LIST && yang_key_match(yp, ys->ys_argument) == 1) ; else #endif - cprintf(cb, "/"); + cprintf(cb, "/"); } + /* If parent namespace/module is different from child -> add child prefix */ + if (ys_real_module(yp) != (ymod = ys_real_module(ys))) + cprintf(cb, "%s:", yang_argument_get(ymod)); } else /* top symbol - mark with name prefix */ - cprintf(cb, "/%s:", yp->ys_argument); + cprintf(cb, "/%s:", yang_argument_get(yp)); if (inclkey){ - if (ys->ys_keyword != Y_CHOICE && ys->ys_keyword != Y_CASE) - cprintf(cb, "%s", ys->ys_argument); + if (yang_keyword_get(ys) != Y_CHOICE && yang_keyword_get(ys) != Y_CASE) + cprintf(cb, "%s", yang_argument_get(ys)); } else{ - if (ys->ys_keyword == Y_LEAF && yp && - yp->ys_keyword == Y_LIST){ - if (yang_key_match(yp, ys->ys_argument) == 0) - cprintf(cb, "%s", ys->ys_argument); /* Not if leaf and key */ + if (yang_keyword_get(ys) == Y_LEAF && yp && + yang_keyword_get(yp) == Y_LIST){ + if (yang_key_match(yp, yang_argument_get(ys)) == 0) + cprintf(cb, "%s", yang_argument_get(ys)); /* Not if leaf and key */ } else - if (ys->ys_keyword != Y_CHOICE && ys->ys_keyword != Y_CASE) - cprintf(cb, "%s", ys->ys_argument); + if (yang_keyword_get(ys) != Y_CHOICE && yang_keyword_get(ys) != Y_CASE) + cprintf(cb, "%s", yang_argument_get(ys)); } - switch (ys->ys_keyword){ + switch (yang_keyword_get(ys)){ case Y_LIST: cvk = ys->ys_cvec; /* Use Y_LIST cache, see ys_populate_list() */ if (cvec_len(cvk)) @@ -2168,6 +2173,8 @@ xml_default(cxobj *xt, cxobj *xb; char *str; int added=0; + char *namespace; + char *prefix; if ((ys = (yang_stmt*)xml_spec(xt)) == NULL){ retval = 0; @@ -2183,9 +2190,16 @@ xml_default(cxobj *xt, assert(y->ys_cv); if (!cv_flag(y->ys_cv, V_UNSET)){ /* Default value exists */ if (!xml_find(xt, y->ys_argument)){ - if ((xc = xml_new(y->ys_argument, NULL, y)) == NULL) goto done; + /* assign right prefix */ + if ((namespace = yang_find_mynamespace(y)) != NULL){ + prefix = NULL; + if (xml2prefix(xt, namespace, &prefix)) + if (xml_prefix_set(xc, prefix) < 0) + goto done; + } + xml_flag_set(xc, XML_FLAG_DEFAULT); if ((xb = xml_new("body", xc, NULL)) == NULL) goto done; @@ -2336,6 +2350,8 @@ xml_spec_populate(cxobj *x, yang_stmt *ymod; /* yang module */ cxobj *xp = NULL; /* xml parent */ char *name; + char *ns = NULL; /* XML namespace of x */ + char *nsy = NULL; /* Yang namespace of x */ yspec = (yang_stmt*)arg; xp = xml_parent(x); @@ -2343,8 +2359,11 @@ xml_spec_populate(cxobj *x, #ifdef DEBUG clicon_debug(1, "%s name:%s", __FUNCTION__, name); #endif - if (xp && (yparent = xml_spec(xp)) != NULL) + if (xml2ns(x, xml_prefix(x), &ns) < 0) + goto done; + if (xp && (yparent = xml_spec(xp)) != NULL){ y = yang_find_datanode(yparent, name); + } else if (yspec){ if (ys_module_by_xml(yspec, x, &ymod) < 0) goto done; @@ -2361,10 +2380,17 @@ xml_spec_populate(cxobj *x, #endif } if (y) { + nsy = yang_find_mynamespace(y); + if (ns == NULL || nsy == NULL){ + clicon_err(OE_XML, EFAULT, "Namespace NULL"); + goto done; + } #ifdef DEBUG clicon_debug(1, "%s y:%s", __FUNCTION__, yang_argument_get(y)); #endif - xml_spec_set(x, y); + /* Assign spec only if namespaces match */ + if (strcmp(ns, nsy) == 0) + xml_spec_set(x, y); } #ifdef DEBUG else @@ -2387,13 +2413,7 @@ xml_spec_populate(cxobj *x, * @retval 0 Invalid api_path or associated XML, netconf error xml set * @retval -1 Fatal error, clicon_err called * - * @note both retval -1 set clicon_err, retval 0 sets netconf xml msg - * @note Not proper namespace translation from api-path 2 xpath - * It works like this: - * Assume origin incoming path is - * "www.foo.com/restconf/a/b=c", pi is 2 and pcvec is: - * ["www.foo.com" "restconf" "a" "b=c"] - * which means the api-path is ["a" "b=c"] corresponding to "a/b=c" + * @code * cbuf *xpath = cbuf_new(); * cvec *cvv = NULL; @@ -2405,7 +2425,14 @@ xml_spec_populate(cxobj *x, * ... access xpath as cbuf_get(xpath) * cbuf_free(xpath) * @endcode + * It works like this: + * Assume origin incoming path is + * "www.foo.com/restconf/a/b=c", pi is 2 and pcvec is: + * ["www.foo.com" "restconf" "a" "b=c"] + * which means the api-path is ["a" "b=c"] corresponding to "a/b=c" * @note "api-path" is "URI-encoded path expression" definition in RFC8040 3.5.3 + * @note retval -1 sets clicon_err, retval 0 sets netconf xml msg + * @note Not proper namespace translation from api-path 2 xpath!!! * @see api_path2xml For api-path to xml tree * @see api_path2xpath Using strings as parameters */ @@ -2438,7 +2465,8 @@ api_path2xpath_cvv(cvec *api_path, nodeid = cv_name_get(cv); if (nodeid_split(nodeid, &prefix, &name) < 0) goto done; - clicon_debug(1, "%s [%d] cvname: %s:%s", __FUNCTION__, i, prefix?prefix:"", name); + clicon_debug(1, "%s [%d] cvname: %s:%s", + __FUNCTION__, i, prefix?prefix:"", name); if (i == offset){ /* top-node */ if (prefix == NULL){ if ((cberr = cbuf_new()) == NULL){ @@ -2536,6 +2564,170 @@ api_path2xpath_cvv(cvec *api_path, goto done; } +/*! Temp alternative to api_path2xpath_cvv */ +int +api_path2xpath_cvv2(cvec *api_path, + int offset, + yang_stmt *yspec, + cbuf *xpath, + cvec *nsc, + cxobj **xerr) +{ + int retval = -1; + int i; + cg_var *cv; + char *nodeid; + char *prefix = NULL; /* api-path (module) prefix */ + char *xprefix = NULL; /* xml xpath prefix */ + char *name = NULL; + cvec *cvk = NULL; /* vector of index keys */ + yang_stmt *y = NULL; + yang_stmt *ymod = NULL; + char *val; + cg_var *cvi; + char **valvec = NULL; + int vi; + int nvalvec; + cbuf *cberr = NULL; + char *namespace = NULL; + + for (i=offset; i get module + change namespace */ + if ((ymod = yang_find_module_by_name(yspec, prefix)) == NULL){ + if ((cberr = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cberr, "No such yang module: %s", prefix); + if (netconf_invalid_value_xml(xerr, "application", cbuf_get(cberr)) < 0) + goto done; + goto fail; + } + namespace = yang_find_mynamespace(ymod); /* change namespace */ + } + if (i == offset && ymod) /* root */ + y = yang_find_datanode(ymod, name); + else + y = yang_find_datanode(y, name); + if (y == NULL){ + if (netconf_unknown_element_xml(xerr, "application", name, "Unknown element") < 0) + goto done; + goto fail; + } + /* Get XML/xpath prefix given namespace. + * note different from api-path prefix + */ + if (xml_nsctx_get_prefix(nsc, namespace, &xprefix) == 0){ + + xprefix = yang_find_myprefix(y); + clicon_debug(1, "%s prefix not found add it %s", __FUNCTION__, xprefix); + /* not found, add it to nsc */ + if (xml_nsctx_set(nsc, xprefix, namespace) < 0) + goto done; + } + /* Check if has value, means '=' */ + if (cv2str(cv, NULL, 0) > 0){ + if ((val = cv2str_dup(cv)) == NULL) + goto done; + switch (yang_keyword_get(y)){ + case Y_LIST: + /* Transform value "a,b,c" to "a" "b" "c" (nvalvec=3) + * Note that vnr can be < length of cvk, due to empty or unset values + */ + if (valvec){ /* loop, valvec may have been used before */ + free(valvec); + valvec = NULL; + } + if ((valvec = clicon_strsep(val, ",", &nvalvec)) == NULL) + goto done; + cvk = y->ys_cvec; /* Use Y_LIST cache, see ys_populate_list() */ + cvi = NULL; + /* Iterate over individual yang keys */ + cprintf(xpath, "/"); + if (xprefix) + cprintf(xpath, "%s:", xprefix); + cprintf(xpath, "%s", name); + vi = 0; + while ((cvi = cvec_each(cvk, cvi)) != NULL && viys_keyword){ case Y_LEAF_LIST: if (0 && restval==NULL){ diff --git a/lib/src/clixon_xml_nsctx.c b/lib/src/clixon_xml_nsctx.c index db85456f..71b78bcb 100644 --- a/lib/src/clixon_xml_nsctx.c +++ b/lib/src/clixon_xml_nsctx.c @@ -94,8 +94,8 @@ xml_nsctx_get(cvec *cvv, * @note NULL is a valid prefix (default) */ int -xml_nsctx_get_prefix(cvec *cvv, - char *namespace, +xml_nsctx_get_prefix(cvec *cvv, + char *namespace, char **prefix) { cg_var *cv = NULL; diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index b79f0a81..0f1bb97a 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -338,7 +338,6 @@ xml_cmp_qsort(const void* arg1, return xml_cmp(*(struct xml**)arg1, *(struct xml**)arg2, 1); } - /*! Sort children of an XML node * Assume populated by yang spec. * @param[in] x0 XML node @@ -615,7 +614,11 @@ xml_insert2(cxobj *xp, } xc = xml_child_i(xp, mid); if ((yc = xml_spec(xc)) == NULL){ - clicon_err(OE_XML, 0, "No spec found %s", xml_name(xc)); + if (xml_type(xc) != CX_ELMNT) + clicon_err(OE_XML, 0, "No spec found %s (wrong xml type:%d)", + xml_name(xc), xml_type(xc)); + else + clicon_err(OE_XML, 0, "No spec found %s", xml_name(xc)); goto done; } if (yc == yn){ /* Same yang */ diff --git a/test/test_augment.sh b/test/test_augment.sh index ac2db916..27ba001b 100755 --- a/test/test_augment.sh +++ b/test/test_augment.sh @@ -6,7 +6,12 @@ # both defined in the basic ietf-interfaces module (type) as well as the main # module through the augmented module () # The ietf-interfaces is very restricted (not original). - +# From a namespace perspective, there are two modules, with symbols as follows: +# 1. ietf-interface - urn:ietf:params:xml:ns:yang:ietf-interfaces +# interfaces, interface, name, type +# 2. example-augment - urn:example:augment - mymod +# (augmented): mandatory-leaf, me, other, +# (uses/grouping): ip, port, lid, lport # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -175,7 +180,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^]]>]]>$" new "netconf verify get with refined ports" -expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' '^e1mymod:some-new-iftypetrue808080]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' '^e1mymod:some-new-iftypetrue808080]]>]]>$' new "netconf set identity defined in other" expecteof "$clixon_netconf -qf $cfg" 0 ' @@ -183,7 +188,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 'e2 fddi true - if:fddi + if:fddi ]]>]]>' "^]]>]]>$" new "netconf validate ok" @@ -195,7 +200,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 'e3 fddi true - mymod:you + mymod:you ]]>]]>' "^]]>]]>$" new "netconf commit ok" @@ -203,11 +208,11 @@ expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^e1mymod:some-new-iftypetrue808080e2fdditrueif:fddi808080e3fdditruemymod:you808080' +expectpart "$(curl -s -i -X GET -H 'Accept: application/yang-data+xml' http://localhost/restconf/data/ietf-interfaces:interfaces)" 0 'HTTP/1.1 200 OK ' 'e1mymod:some-new-iftypetrue808080e2fdditrueif:fddi808080e3fdditruemymod:you808080' new "Kill restconf daemon" stop_restconf diff --git a/test/test_cli.sh b/test/test_cli.sh index 8fd3dc35..52a24828 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -77,6 +77,7 @@ expectfn "$clixon_cli -1 -f $cfg -l o validate" 255 "Validate failed. Edit and t new "cli configure ip addr" expectfn "$clixon_cli -1 -f $cfg set interfaces interface eth/0/0 ipv4 address 1.2.3.4 prefix-length 24" 0 "^$" + new "cli configure ip descr" expectfn "$clixon_cli -1 -f $cfg set interfaces interface eth/0/0 description mydesc" 0 "^$" new "cli configure ip type" diff --git a/test/test_leafref.sh b/test/test_leafref.sh index 8cc16293..fabad06b 100755 --- a/test/test_leafref.sh +++ b/test/test_leafref.sh @@ -56,10 +56,10 @@ module example{ } } leaf address { - description "From RFC7950 9.9.5"; + description "From RFC7950 9.9.6"; type leafref { path "../../if:interfaces/if:interface[if:name = current()/../relname]" - + "/if:ipv4/if:address/if:ip"; + + "/ip:ipv4/ip:address/ip:ip"; } } leaf wrong { @@ -85,29 +85,29 @@ EOF BASEXML=$(cat < - + eth0 ex:eth - -
- 192.0.2.1 - 24 -
-
- 192.0.2.2 - 24 -
-
+ + + 192.0.2.1 + 24 + + + 192.0.2.2 + 24 + +
- + lo ex:lo - -
- 127.0.0.1 - 32 -
-
+ + + 127.0.0.1 + 32 + +
EOF diff --git a/test/test_netconf.sh b/test/test_netconf.sh index 3604fa9c..b08d3af9 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -94,7 +94,7 @@ new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" new "netconf edit config" -expecteof "$clixon_netconf -qf $cfg" 0 'eth/0/0eth1true
9.2.3.424
]]>]]>' "^]]>]]>$" +expecteof "$clixon_netconf -qf $cfg" 0 'eth/0/0eth1true9.2.3.424]]>]]>' "^]]>]]>$" # Too many quotes cat < $tmp # new @@ -102,7 +102,7 @@ cat < $tmp # new EOF new "netconf get config xpath" -expecteof "$clixon_netconf -qf $cfg" 0 "$(cat $tmp)" '^eth1true]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "$(cat $tmp)" '^eth1true]]>]]>$' # Too many quotes cat < $tmp # new @@ -110,7 +110,7 @@ cat < $tmp # new EOF new "netconf get config xpath parent" -expecteof "$clixon_netconf -qf $cfg" 0 "$(cat $tmp)" '^eth/0/0trueeth1truetruefalse
9.2.3.424
]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "$(cat $tmp)" '^eth/0/0trueeth1truetruefalse9.2.3.424]]>]]>$' new "netconf validate missing type" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^" diff --git a/test/test_restconf_err.sh b/test/test_restconf_err.sh index c62eb413..d6729d31 100755 --- a/test/test_restconf_err.sh +++ b/test/test_restconf_err.sh @@ -11,7 +11,10 @@ # usually contain a payload that represents the error condition, such # that it describes the error state and what next steps are suggested # for resolving it. - +# +# Note this is different from an api-path that is invalid from a yang point +# of view, this is interpreted as 400 Bad Request invalid-value/unknown-element +# XXX: complete non-existent yang with unknown-element for all PUT/POST/GET api-paths # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -20,7 +23,7 @@ APPNAME=example cfg=$dir/conf.xml fyang=$dir/example.yang -fyang2=$dir/aug.yang +fyang2=$dir/augment.yang fxml=$dir/initial.xml # example @@ -34,15 +37,24 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.sock $dir/restconf.pidfile /usr/local/var/$APPNAME + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME EOF cat < $fyang2 -module augm{ +module augment{ yang-version 1.1; namespace "urn:example:aug"; prefix aug; description "Used as a base for augment"; + container route-config { + description + "Root container for routing models"; + container dynamic { + } + } container route-state { description "Root container for routing models"; @@ -58,7 +70,7 @@ module example{ yang-version 1.1; namespace "urn:example:clixon"; prefix ex; - import aug { + import augment { description "Just for augment"; prefix "aug"; } @@ -86,12 +98,15 @@ module example{ } augment "/aug:route-config/aug:dynamic" { container ospf { - container routers { - container auto-cost { - leaf reference-bandwidth { - type uint32; - } - } + leaf reference-bandwidth { + type uint32; + } + } + } + augment "/aug:route-config/aug:dynamic" { + container ospf { + leaf reference-bandwidth { + type uint32; } } } @@ -147,13 +162,28 @@ if false; then new "restconf POST non-existent (no yang) element" # should be invalid element expectpart "$(curl -is -X POST -H 'Content-Type: application/yang-data+xml' -d "$XML" http://localhost/restconf/data/example:a=23/xxx)" 0 'HTTP/1.1 400 Bad Request' '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Unknown element: ' - - -new "restconf GET multi-namespace path" -# simplify yang -# works for config? -expectpart "$(curl -si -X GET http://localhost/restconf/data/augm:route-state/dynamic/ospf/routers/auto-cost/reference-bandwidth)" 0 'HTTP/1.1 404 Not Found' '{"ietf-restconf:errors":{"error":{"rpc-error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Unknown element: 'xxx'"}}}}' fi + +# Test for multi-module path where an augment stretches across modules +new "restconf POST augment multi-namespace path" +expecteq "$(curl -s -X POST -H 'Content-Type: application/yang-data+xml' -d '23' http://localhost/restconf/data)" 0 '' + +new "restconf GET augment multi-namespace top" +expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config)" 0 'HTTP/1.1 200 OK' '{"augment:route-config":{"dynamic":{"example:ospf":{"reference-bandwidth":23}}}}' + +new "restconf GET augment multi-namespace level 1" +expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config/dynamic)" 0 'HTTP/1.1 200 OK' '{"augment:dynamic":{"example:ospf":{"reference-bandwidth":23}}}' + +new "restconf GET augment multi-namespace cross" +expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config/dynamic/example:ospf)" 0 'HTTP/1.1 200 OK' '{"example:ospf":{"reference-bandwidth":23}}' + +new "restconf GET augment multi-namespace cross level 2" +expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config/dynamic/example:ospf/reference-bandwidth)" 0 'HTTP/1.1 200 OK' '{"example:reference-bandwidth":23}' + +# XXX actually no such element +#new "restconf GET augment multi-namespace, no 2nd module in api-path, fail" +#expectpart "$(curl -si -X GET http://localhost/restconf/data/augment:route-config/dynamic/ospf)" 0 'HTTP/1.1 404 Not Found' '{"ietf-restconf:errors":{"error":{"rpc-error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}}' + new "Kill restconf daemon" stop_restconf