diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index ccdb7f70..54606f5d 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -640,7 +640,7 @@ from_client_edit_config(clicon_handle h, goto done; goto ok; } - /* Maybe validate xml here as in text_modify_top? */ + /* Mark all nodes that are not configure data and set return */ if (xml_apply(xc, CX_ELMNT, xml_non_config_data, &non_config) < 0) goto done; if (non_config){ @@ -649,7 +649,7 @@ from_client_edit_config(clicon_handle h, goto ok; } /* xmldb_put (difflist handling) requires list keys */ - if ((ret = xml_yang_validate_list_key_only(h, xc, &xret)) < 0) + if ((ret = xml_yang_validate_list_key_only(xc, &xret)) < 0) goto done; if (ret == 0){ if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0) diff --git a/lib/clixon/clixon_validate.h b/lib/clixon/clixon_validate.h index cfc7bd50..08f4bc98 100644 --- a/lib/clixon/clixon_validate.h +++ b/lib/clixon/clixon_validate.h @@ -43,7 +43,7 @@ */ int xml_yang_validate_rpc(clicon_handle h, cxobj *xrpc, cxobj **xret); int xml_yang_validate_add(clicon_handle h, cxobj *xt, cxobj **xret); -int xml_yang_validate_list_key_only(clicon_handle h, cxobj *xt, cxobj **xret); +int xml_yang_validate_list_key_only(cxobj *xt, cxobj **xret); int xml_yang_validate_all(clicon_handle h, cxobj *xt, cxobj **xret); int xml_yang_validate_all_top(clicon_handle h, cxobj *xt, cxobj **xret); diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index 662e6897..d86d239b 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -436,6 +436,59 @@ check_choice(cxobj *xt, goto done; } +/*! Check that an XML tree of type list has valid keys + * @param[in] xt XML tree + * @param[in] yt Yang spec of xt of type LIST which is a config true node. + * @param[out] xret Error XML tree. Free with xml_free after use + * @retval 1 Validation OK + * @retval 0 Validation failed (xret set) + * @retval -1 Error + * This ensures that there are keys in XML corresponding to the Yang + * This function is broken out from generic validation since it needs specially checked in incoming + * edit-config + * XXX: It does not check double (more than one) key + * @see xml_yang_validate_list_key_only + */ +static int +check_list_key(cxobj *xt, + yang_stmt *yt, + cxobj **xret) + +{ + int retval = -1; + yang_stmt *yc; + cvec *cvk = NULL; /* vector of index keys */ + cg_var *cvi; + char *keyname; + + if (yt == NULL || !yang_config(yt) || yang_keyword_get(yt) == Y_LIST){ + clicon_err(OE_YANG, EINVAL, "yt is not a config true list node"); + goto done; + } + yc = NULL; + while ((yc = yn_each(yt, yc)) != NULL) { + if (yang_keyword_get(yc) != Y_KEY) + continue; + /* Check if a list does not have mandatory key leafs */ + cvk = yang_cvec_get(yt); /* Use Y_LIST cache, see ys_populate_list() */ + cvi = NULL; + while ((cvi = cvec_each(cvk, cvi)) != NULL) { + keyname = cv_string_get(cvi); + if (xml_find_type(xt, NULL, keyname, CX_ELMNT) == NULL){ + if (netconf_missing_element_xml(xret, "application", keyname, "Mandatory key") < 0) + goto done; + goto fail; + } + } + } + retval = 1; + done: + return retval; + fail: + retval = 0; + goto done; +} + /*! Check if an xml node lacks mandatory children * @param[in] xt XML node to be validated * @param[in] yt xt:s yang statement @@ -455,28 +508,21 @@ check_mandatory(cxobj *xt, yang_stmt *y; yang_stmt *yc; yang_stmt *yp; - cvec *cvk = NULL; /* vector of index keys */ - cg_var *cvi; - char *keyname; cbuf *cb = NULL; + int ret; + if (yt == NULL || !yang_config(yt)){ + clicon_err(OE_YANG, EINVAL, "yt is not config true"); + goto done; + } + if (yang_keyword_get(yt) == Y_LIST){ + if ((ret = check_list_key(xt, yt, xret)) < 0) + goto done; + if (ret == 0) + goto fail; + } yc = NULL; while ((yc = yn_each(yt, yc)) != NULL) { - /* Check if a list does not have mandatory key leafs */ - if (yang_keyword_get(yt) == Y_LIST && - yang_keyword_get(yc) == Y_KEY && - yang_config(yt)){ - cvk = yang_cvec_get(yt); /* Use Y_LIST cache, see ys_populate_list() */ - cvi = NULL; - while ((cvi = cvec_each(cvk, cvi)) != NULL) { - keyname = cv_string_get(cvi); - if (xml_find_type(xt, NULL, keyname, CX_ELMNT) == NULL){ - if (netconf_missing_element_xml(xret, "application", keyname, "Mandatory key") < 0) - goto done; - goto fail; - } - } - } if (!yang_mandatory(yc)) continue; switch (yang_keyword_get(yc)){ @@ -535,47 +581,6 @@ check_mandatory(cxobj *xt, goto done; } -/*! - * @param[out] xret Error XML tree. Free with xml_free after use - */ -static int -check_list_key(cxobj *xt, - yang_stmt *yt, - cxobj **xret) - -{ - int retval = -1; - yang_stmt *yc; - cvec *cvk = NULL; /* vector of index keys */ - cg_var *cvi; - char *keyname; - - yc = NULL; - while ((yc = yn_each(yt, yc)) != NULL) { - /* Check if a list does not have mandatory key leafs */ - if (yang_keyword_get(yt) == Y_LIST && - yang_keyword_get(yc) == Y_KEY && - yang_config(yt)){ - cvk = yang_cvec_get(yt); /* Use Y_LIST cache, see ys_populate_list() */ - cvi = NULL; - while ((cvi = cvec_each(cvk, cvi)) != NULL) { - keyname = cv_string_get(cvi); - if (xml_find_type(xt, NULL, keyname, CX_ELMNT) == NULL){ - if (netconf_missing_element_xml(xret, "application", keyname, "Mandatory key") < 0) - goto done; - goto fail; - } - } - } - } - retval = 1; - done: - return retval; - fail: - retval = 0; - goto done; -} - /*! New element last in list, check if already exists if sp return -1 * @param[in] vec Vector of existing entries (new is last) * @param[in] i1 The new entry is placed at vec[i1] @@ -723,7 +728,7 @@ check_min_max(cxobj *xp, /*! Detect unique constraint for duplicates from parent node and minmax * @param[in] xt XML parent (may have lists w unique constraints as child) - * @param[out] xret Error XML tree. Free with xml_free after use + * @param[out] xret Error XML tree. Free with xml_free after use * @retval 1 Validation OK * @retval 0 Validation failed (xret set) * @retval -1 Error @@ -753,7 +758,7 @@ check_min_max(cxobj *xp, * gaps in the xml nodes given the ancestor Yang structure. * But no gap analysis is done if the yang spec of the top-level xml is unknown * Example: - * Yang structure:y1, y2, y3, + * Yang structure: y1, y2, y3, * XML structure: [x1, x1], [x3, x3] where [x2] list is missing * @note min-element constraints on empty lists are not detected on top-level. * Or more specifically, if no yang spec if associated with the top-level @@ -820,6 +825,7 @@ check_list_unique_minmax(cxobj *xt, goto fail; } } + /* Here is only new list / leaf-list */ yprev = y; /* Restart min/max count */ nr = 1; /* Gap analysis: Check if there is any empty list between y and yprev @@ -868,8 +874,7 @@ check_list_unique_minmax(cxobj *xt, if (ret == 0) goto fail; } - } - + } /* while x */ /* yprev if set, is a list that has been traversed * This check is made in the loop as well - this is for the last list @@ -1010,11 +1015,11 @@ xml_yang_validate_add(clicon_handle h, } /*! Some checks done only at edit_config, eg keys in lists - * @param[out] xret Error XML tree. Free with xml_free after use + * @param[in] xt XML tree + * @param[out] xret Error XML tree. Free with xml_free after use */ int -xml_yang_validate_list_key_only(clicon_handle h, - cxobj *xt, +xml_yang_validate_list_key_only(cxobj *xt, cxobj **xret) { int retval = -1; @@ -1024,7 +1029,9 @@ xml_yang_validate_list_key_only(clicon_handle h, /* if not given by argument (overide) use default link and !Node has a config sub-statement and it is false */ - if ((yt = xml_spec(xt)) != NULL && yang_config(yt) != 0){ + if ((yt = xml_spec(xt)) != NULL && + yang_config(yt) != 0 && + yang_keyword_get(yt) == Y_LIST){ if ((ret = check_list_key(xt, yt, xret)) < 0) goto done; if (ret == 0) @@ -1032,7 +1039,7 @@ xml_yang_validate_list_key_only(clicon_handle h, } x = NULL; while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) { - if ((ret = xml_yang_validate_list_key_only(h, x, xret)) < 0) + if ((ret = xml_yang_validate_list_key_only(x, xret)) < 0) goto done; if (ret == 0) goto fail; diff --git a/test/test_restconf2.sh b/test/test_restconf2.sh index ece54ac1..ea7ccd5f 100755 --- a/test/test_restconf2.sh +++ b/test/test_restconf2.sh @@ -132,14 +132,17 @@ new "restconf POST interface without namespace" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"interface":{"name":"TEST2","type":"eth0"}}' $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"rpc","error-tag":"malformed-message","error-severity":"error","error-message":"Top-level JSON object interface is not qualified with namespace which is a MUST according to RFC 7951"}}}' new "restconf POST again" -expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"example:interface":{"name":"TEST","type":"eth0"}}' $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/1.1 409 Conflict" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"data-exists","error-severity":"error","error-message":"Data already exists; cannot create new resource"}}}' +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/example:cont1 -d '{"example:interface":{"name":"TEST","type":"eth0"}}')" 0 "HTTP/1.1 409 Conflict" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"data-exists","error-severity":"error","error-message":"Data already exists; cannot create new resource"}}}' new "restconf POST from top" -expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"example:cont1":{"interface":{"name":"TEST","type":"eth0"}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/1.1 409 Conflict" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"data-exists","error-severity":"error","error-message":"Data already exists; cannot create new resource"}}}' +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data -d '{"example:cont1":{"interface":{"name":"TEST","type":"eth0"}}}')" 0 "HTTP/1.1 409 Conflict" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"data-exists","error-severity":"error","error-message":"Data already exists; cannot create new resource"}}}' new "restconf DELETE" expectpart "$(curl $CURLOPTS -X DELETE $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/1.1 204 No Content" +new "restconf POST from top containing duplicate keys expect error" +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data -d '{"example:cont1":{"interface":[{"name":"TEST","type":"eth0"},{"name":"TEST","type":"eth0"}]}}')" 0 "HTTP/1.1 412 Precondition Failed" '{"ietf-restconf:errors":{"error":{"error-type":"protocol","error-tag":"operation-failed","error-app-tag":"data-not-unique","error-severity":"error","error-info":{"non-unique":{"name":"TEST"}}}}}' + new "restconf GET null datastore" expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/1.1 404 Not Found" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}' diff --git a/yang/clixon/clixon-config@2020-10-01.yang b/yang/clixon/clixon-config@2020-10-01.yang index 13f9e12f..f2dde0ea 100644 --- a/yang/clixon/clixon-config@2020-10-01.yang +++ b/yang/clixon/clixon-config@2020-10-01.yang @@ -348,7 +348,7 @@ module clixon-config { default true; description "If false, skip Yang list check sanity checks from RFC 7950, Sec 7.8.2: - The 'key' statement, which MUST be present if the list represents configuration. + The 'key' statement, which MUST be present if the list represents configuration. Some yang specs seem not to fulfil this. However, if you reset this, there may be follow-up errors due to code that assumes a configuration list has keys"; }