From 0d53b406607ddbcffc58cbc5b5fedbfe8a2f3cb7 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 13 Feb 2019 18:19:33 +0100 Subject: [PATCH] Did not check for missing list keys in validate. [Key of a list isn't mandatory](https://github.com/clicon/clixon/issues/73) --- CHANGELOG.md | 2 ++ apps/backend/backend_commit.c | 8 ++++---- lib/src/clixon_xml_map.c | 21 +++++++++++++++++++++ lib/src/clixon_xml_sort.c | 14 +++++++++----- test/test_netconf.sh | 9 +++++++++ test/test_restconf2.sh | 8 +++++++- 6 files changed, 52 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a8f08cf..6eafdb67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,8 @@ * New config option: CLICON_CLI_MODEL_TREENAME defining name of generated syntax tree if CLIXON_CLI_GENMODEL is set. ### Corrected Bugs +* Did not check for missing keys in validate. [Key of a list isn't mandatory](https://github.com/clicon/clixon/issues/73) + * Problem here is that you can still add keys via netconf - since validation is a separate step, but in cli or restconf it should not be possible. * Partially corrected: [yang type range statement does not support multiple values](https://github.com/clicon/clixon/issues/59). * Should work for netconf and restconf, but not for CLI. * Fixed: [Range parsing is not RFC 7950 compliant](https://github.com/clicon/clixon/issues/71) diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 453e3e6c..62a7ea9b 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -203,13 +203,13 @@ validate_common(clicon_handle h, xml_flag_set(xn, XML_FLAG_DEL); xml_apply(xn, CX_ELMNT, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_DEL); xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE); - } + } for (i=0; itd_alen; i++){ /* Also down */ xn = td->td_avec[i]; xml_flag_set(xn, XML_FLAG_ADD); xml_apply(xn, CX_ELMNT, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_ADD); xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE); - } + } for (i=0; itd_clen; i++){ /* Also up */ xn = td->td_scvec[i]; xml_flag_set(xn, XML_FLAG_CHANGE); @@ -218,7 +218,6 @@ validate_common(clicon_handle h, xml_flag_set(xn, XML_FLAG_CHANGE); xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE); } - /* 4. Call plugin transaction start callbacks */ if (plugin_transaction_begin(h, td) < 0) goto done; @@ -357,7 +356,8 @@ from_client_commit(clicon_handle h, goto done; goto ok; } - cprintf(cbret, ""); + if (ret == 1) + cprintf(cbret, ""); ok: retval = 0; done: diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 6a410510..219de1a2 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -501,9 +501,27 @@ check_mandatory(cxobj *xt, yang_stmt *y; yang_stmt *yc; yang_node *yp; + cvec *cvk = NULL; /* vector of index keys */ + cg_var *cvi; + char *keyname; for (i=0; iys_len; i++){ yc = yt->ys_stmt[i]; + /* Check if a list does not have mandatory key leafs */ + if (yt->ys_keyword == Y_LIST && + yc->ys_keyword == Y_KEY && + yang_config(yt)){ + cvk = yt->ys_cvec; /* 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(cbret, "application", keyname, "Mandatory key") < 0) + goto done; + goto fail; + } + } + } if (!yang_mandatory(yc)) continue; switch (yc->ys_keyword){ @@ -793,6 +811,9 @@ xml_yang_validate_all(cxobj *xt, } /*! Translate a single xml node to a cligen variable vector. Note not recursive + * @retval 1 Validation OK + * @retval 0 Validation failed (cbret set) + * @retval -1 Error */ int xml_yang_validate_all_top(cxobj *xt, diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index f33901d1..f9828e11 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -252,11 +252,15 @@ xml_cmp(const void* arg1, cvk = y1->ys_cvec; /* Use Y_LIST cache, see ys_populate_list() */ cvi = NULL; while ((cvi = cvec_each(cvk, cvi)) != NULL) { - keyname = cv_string_get(cvi); - b1 = xml_find_body(x1, keyname); - b2 = xml_find_body(x2, keyname); - if ((equal = strcmp(b1,b2)) != 0) - goto done; + keyname = cv_string_get(cvi); /* operational data may have NULL keys*/ + if ((b1 = xml_find_body(x1, keyname)) == NULL) + equal = -1; + else if ((b2 = xml_find_body(x2, keyname)) == NULL) + equal = 1; + else{ + if ((equal = strcmp(b1,b2)) != 0) + goto done; + } } equal = 0; break; diff --git a/test/test_netconf.sh b/test/test_netconf.sh index d113cab5..b28d86c0 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -88,6 +88,15 @@ expecteof "$clixon_netconf -qf $cfg" 0 'eth/0/0ex:ethnone ]]>]]>' '^' +new "Add interface without key" +expecteof "$clixon_netconf -qf $cfg" 0 'ex:ethnone ]]>]]>' "^]]>]]>$" + +new "netconf validate missing key" +expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" '^applicationmissing-elementnameerrorMandatory key]]>]]>$' + +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
]]>]]>' "^]]>]]>$" diff --git a/test/test_restconf2.sh b/test/test_restconf2.sh index 1eda32c2..446a8caa 100755 --- a/test/test_restconf2.sh +++ b/test/test_restconf2.sh @@ -92,6 +92,9 @@ sleep $RCWAIT new "restconf tests" +new "restconf POST tree without key" +expectfn 'curl -s -X POST -d {"example:cont1":{"interface":{"type":"regular"}}} http://localhost/restconf/data' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "name"},"error-severity": "error","error-message": "Mandatory key"}}} ' + new "restconf POST initial tree" expectfn 'curl -s -X POST -d {"example:cont1":{"interface":{"name":"local0","type":"regular"}}} http://localhost/restconf/data' 0 "" @@ -113,7 +116,10 @@ new "restconf GET if-type" expectfn "curl -s -X GET http://localhost/restconf/data/example:cont1/interface=local0/type" 0 '{"example:type": "regular"}' new "restconf POST interface without mandatory type" -expectfn 'curl -s -X POST -d {"interface":{"name":"TEST"}} http://localhost/restconf/data/example:cont1' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "type"},"error-severity": "error","error-message": "Mandatory variable"}}} ' +expectfn 'curl -s -X POST http://localhost/restconf/data/example:cont1 -d {"interface":{"name":"TEST"}} http://localhost/restconf/data/example:cont1' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "type"},"error-severity": "error","error-message": "Mandatory variable"}}} ' + +new "restconf POST interface without mandatory key" +expectfn 'curl -s -X POST http://localhost/restconf/data/example:cont1 -d {"interface":{"type":"regular"}}' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "missing-element","error-info": {"bad-element": "name"},"error-severity": "error","error-message": "Mandatory key"}}} ' new "restconf POST interface" expectfn 'curl -s -X POST -d {"example:interface":{"name":"TEST","type":"eth0"}} http://localhost/restconf/data/example:cont1' 0 ""