From 558abb1f9b02926223c6cc7e296b810d782cfbea Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 27 Aug 2020 15:52:31 +0200 Subject: [PATCH] Corrected error message for list min/max-value to comply to RFC 7950: a proper path is now returned, peviously only the list symbol. it is also exposed in the CLI correctly. * Example: `/c/a1` --- CHANGELOG.md | 2 ++ lib/clixon/clixon_netconf_lib.h | 2 +- lib/src/clixon_netconf_lib.c | 31 ++++++++++++++++++++++++++----- lib/src/clixon_validate.c | 28 +++++++++++++--------------- test/test_minmax.sh | 14 +++++++------- 5 files changed, 49 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb97f02c..408c0be4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ Users may have to change how they access the system ### Corrected Bugs +* Corrected error message for list min/max-value to comply to RFC 7950: a proper path is now returned, peviously only the list symbol. it is also exposed in the CLI correctly. + * Example: `/c/a1` * Fixed: [Yang modules skipped if the name is a proper prefix of other module name](https://github.com/clicon/clixon/issues/130) * Fixed an error in global default values. Global default values were not written to datastore after startup, but AFTER an edit/commit. * Fixed: [Type / Endianism problem in yang_parse_file #128](https://github.com/clicon/clixon/issues/128) diff --git a/lib/clixon/clixon_netconf_lib.h b/lib/clixon/clixon_netconf_lib.h index 68970c04..2dcd59ef 100644 --- a/lib/clixon/clixon_netconf_lib.h +++ b/lib/clixon/clixon_netconf_lib.h @@ -97,7 +97,7 @@ int netconf_operation_failed_xml(cxobj **xret, char *type, char *message); int netconf_malformed_message(cbuf *cb, char *message); int netconf_malformed_message_xml(cxobj **xret, char *message); int netconf_data_not_unique_xml(cxobj **xret, cxobj *x, cvec *cvk); -int netconf_minmax_elements_xml(cxobj **xret, cxobj *x, int max); +int netconf_minmax_elements_xml(cxobj **xret, cxobj *xp, char *name, int max); int netconf_trymerge(cxobj *x, yang_stmt *yspec, cxobj **xret); int netconf_module_features(clicon_handle h); int netconf_module_load(clicon_handle h); diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index e23615dc..8492a89e 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -1173,19 +1173,23 @@ netconf_data_not_unique_xml(cxobj **xret, /*! Create Netconf too-many/few-elements err msg according to RFC 7950 15.2/15.3 * * A NETCONF operation would result in configuration data where a - list or a leaf-list would have too many entries, the following error - * @param[out] xret Error XML tree. Free with xml_free after use - * @param[in] x List element containing duplicate + * list or a leaf-list would have too many entries, the following error + * @param[out] xret Error XML tree. Free with xml_free after use + * @param[in] xp XML parent node (for error) + * @param[in] name Name of list (for error) * @param[in] max If set, return too-many, otherwise too-few * @see RFC7950 Sec 15.1 */ int netconf_minmax_elements_xml(cxobj **xret, - cxobj *x, + cxobj *xp, + char *name, int max) { int retval = -1; cxobj *xerr; + char *path = NULL; + cbuf *cb = NULL; if (*xret == NULL){ if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL) @@ -1195,16 +1199,31 @@ netconf_minmax_elements_xml(cxobj **xret, goto done; if ((xerr = xml_new("rpc-error", *xret, CX_ELMNT)) == NULL) goto done; + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + if (xml_parent(xp)){ /* Dont include root, eg */ + if (xml2xpath(xp, &path) < 0) + goto done; + if (path) + cprintf(cb, "%s", path); + } + cprintf(cb, "/%s", name); if (clixon_xml_parse_va(YB_NONE, NULL, &xerr, NULL, "protocol" "operation-failed" "too-%s-elements" "error" "%s", max?"many":"few", - xml_name(x)) < 0) /* XXX should be xml2xpath */ + cbuf_get(cb)) < 0) goto done; retval = 0; done: + if (path) + free(path); + if (cb) + cbuf_free(cb); return retval; } @@ -1381,6 +1400,8 @@ netconf_err2cb(cxobj *xerr, clicon_xml2cbuf(cberr, xml_child_i(x,0), 0, 0, -1); if ((x=xpath_first(xerr, NULL, "//error-app-tag"))!=NULL) cprintf(cberr, ": %s ", xml_body(x)); + if ((x=xpath_first(xerr, NULL, "//error-path"))!=NULL) + cprintf(cberr, ": %s ", xml_body(x)); retval = 0; return retval; } diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index e3d652e8..c041ca26 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -694,17 +694,17 @@ check_unique_list(cxobj *x, } /*! Given a list, check if any min/max-elemants constraints apply - * @param[in] x One x (the last) of a specific lis - * @param[in] y Yang spec of x - * @param[in] nr Number of elements (like x) in thlist - * @param[out] xret Error XML tree. Free with xml_free after use + * @param[in] xp Parent of the xml list there are too few/many + * @param[in] y Yang spec of the failing list + * @param[in] nr Number of elements (like x) in thlist + * @param[out] xret Error XML tree. Free with xml_free after use * @retval 1 Validation OK * @retval 0 Validation failed (cbret set) * @retval -1 Error * @see RFC7950 7.7.5 */ static int -check_min_max(cxobj *x, +check_min_max(cxobj *xp, yang_stmt *y, int nr, cxobj **xret) @@ -717,7 +717,7 @@ check_min_max(cxobj *x, if ((ymin = yang_find(y, Y_MIN_ELEMENTS, NULL)) != NULL){ cv = yang_cv_get(ymin); if (nr < cv_uint32_get(cv)){ - if (netconf_minmax_elements_xml(xret, x, 0) < 0) + if (netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 0) < 0) goto done; goto fail; } @@ -726,7 +726,7 @@ check_min_max(cxobj *x, cv = yang_cv_get(ymax); if (cv_uint32_get(cv) > 0 && /* 0 means unbounded */ nr > cv_uint32_get(cv)){ - if (netconf_minmax_elements_xml(xret, x, 1) < 0) + if (netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 1) < 0) goto done; goto fail; } @@ -787,10 +787,9 @@ check_list_unique_minmax(cxobj *xt, yang_stmt *y; yang_stmt *yt; yang_stmt *yprev = NULL; /* previous in list */ - yang_stmt *ye = NULL; /* yang each list to catch emtpy */ - yang_stmt *ych; /* y:s parent node (if choice that ye can compare to) */ - cxobj *xp = NULL; /* previous in list */ - yang_stmt *yu; /* yang unique */ + yang_stmt *ye = NULL; /* yang each list to catch emtpy */ + yang_stmt *ych; /* y:s parent node (if choice that ye can compare to) */ + yang_stmt *yu; /* yang unique */ int ret; int nr=0; /* Nr of list elements for min/max check */ enum rfc_6020 keyw; @@ -818,7 +817,7 @@ check_list_unique_minmax(cxobj *xt, /* Only lists and leaf-lists are allowed to be many * This checks duplicate container and leafs */ - if (netconf_minmax_elements_xml(xret, x, 1) < 0) + if (netconf_minmax_elements_xml(xret, xt, xml_name(x), 1) < 0) goto done; goto fail; } @@ -832,14 +831,13 @@ check_list_unique_minmax(cxobj *xt, } else { /* Check if the list length violates min/max */ - if ((ret = check_min_max(xp, yprev, nr, xret)) < 0) + if ((ret = check_min_max(xt, yprev, nr, xret)) < 0) goto done; if (ret == 0) goto fail; } } yprev = y; /* Restart min/max count */ - xp = x; /* Need a reference to the XML as well */ nr = 1; /* Gap analysis: Check if there is any empty list between y and yprev * Note, does not detect empty choice list (too complicated) @@ -884,7 +882,7 @@ check_list_unique_minmax(cxobj *xt, */ if (yprev){ /* Check if the list length violates min/max */ - if ((ret = check_min_max(xp, yprev, nr, xret)) < 0) + if ((ret = check_min_max(xt, yprev, nr, xret)) < 0) goto done; if (ret == 0) goto fail; diff --git a/test/test_minmax.sh b/test/test_minmax.sh index eee5da2c..e74eb5b5 100755 --- a/test/test_minmax.sh +++ b/test/test_minmax.sh @@ -137,31 +137,31 @@ expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' |$clixon_netconf -qf $cfg new "minmax: validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserrorc]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserror/c/a1]]>]]>$' new "minmax: no list" expecteof "$clixon_netconf -qf $cfg" 0 'replace0]]>]]>' '^]]>]]>$' new "minmax: validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserrorc]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserror/c/a1]]>]]>$' new "minmax: no leaf-list" expecteof "$clixon_netconf -qf $cfg" 0 'replace0]]>]]>' '^]]>]]>$' new "minmax: validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserrorc]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-few-elementserror/c/b1]]>]]>$' new "minmax: Too large list" expecteof "$clixon_netconf -qf $cfg" 0 'replace0120]]>]]>' '^]]>]]>$' new "minmax: validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserrora1]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserror/c/a1]]>]]>$' new "minmax: Too large leaf-list" expecteof "$clixon_netconf -qf $cfg" 0 'replace0012]]>]]>' '^]]>]]>$' new "minmax: validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserrorb1]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserror/c/b1]]>]]>$' new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -196,7 +196,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^]]>]]>$" new "minmax choice: too many validate should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserrora1]]>]]>$' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserror/c2/a1]]>]]>$' new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" @@ -219,7 +219,7 @@ expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^]]>]]>$" new "minmax top level too many should fail" -expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserrorb3]]>]]>' +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^protocoloperation-failedtoo-many-elementserror/b3]]>]]>' new "netconf discard-changes" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$"