From 712d8a6ff1f09e078d17f73a1dd452aeadf0aa65 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 15 May 2019 17:38:52 +0200 Subject: [PATCH] Restconf PUT different keys detected (thanks @dcornejo) and fixed --- CHANGELOG.md | 3 + apps/restconf/restconf_methods.c | 21 +++++-- test/test_restconf_listkey.sh | 97 ++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 4 deletions(-) create mode 100755 test/test_restconf_listkey.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e31306b..e1d8b985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,9 @@ ### Corrected Bugs +* Restconf PUT different keys detected (thanks @dcornejo) and fixed + * This was accepted but shouldn't be: `PUT http://restconf/data/A=hello/B -d '{"B":"goodbye"}'` + * See RFC 8040 Sec 4.5 * Yang Enumeration including space did not generate working CLIgen code, see [Choice with space is not working in CLIgen code](https://github.com/olofhagsand/cligen/issues/24) * Fixed: [Yang submodule import prefix restrictions #60](https://github.com/clicon/clixon/issues/60) * Fixed support for multiple datanodes in a choice/case statement. Only single datanode was supported. diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index c2290331..a84aae4a 100644 --- a/apps/restconf/restconf_methods.c +++ b/apps/restconf/restconf_methods.c @@ -734,6 +734,7 @@ api_data_put(clicon_handle h, cxobj *xparent; cxobj *x; yang_stmt *y = NULL; + yang_stmt *yp; /* yang parent */ yang_stmt *yspec; cxobj *xa; char *api_path; @@ -869,9 +870,20 @@ api_data_put(clicon_handle h, goto done; goto ok; } - /* If list or leaf-list, api-path keys must match data keys */ - if (y && (yang_keyword_get(y) == Y_LIST || yang_keyword_get(y) == Y_LEAF_LIST)){ - if (match_list_keys((yang_stmt*)y, x, xbot) < 0){ + /* If list or leaf-list, api-path keys must match data keys + * There are two cases, either the object is the list element itself, + * eg xpath:obj=a data:b + * or the object is the key element: + * eg xpath:obj=a/key data:b + * That is why the conditional is somewhat hairy + */ + xparent = xml_parent(xbot); + if (y){ + yp = yang_parent_get(y); + if (((yang_keyword_get(y) == Y_LIST || yang_keyword_get(y) == Y_LEAF_LIST) && + match_list_keys(y, x, xbot) < 0) || + (yp && yang_keyword_get(yp) == Y_LIST && + match_list_keys(yp, xml_parent(x), xparent) < 0)){ if (netconf_operation_failed_xml(&xerr, "protocol", "api-path keys do not match data keys") < 0) goto done; if ((xe = xpath_first(xerr, "rpc-error")) == NULL){ @@ -881,9 +893,10 @@ api_data_put(clicon_handle h, if (api_return_err(h, r, xe, pretty, use_xml) < 0) goto done; goto ok; + } } - xparent = xml_parent(xbot); + xml_purge(xbot); if (xml_addsub(xparent, x) < 0) goto done; diff --git a/test/test_restconf_listkey.sh b/test/test_restconf_listkey.sh new file mode 100755 index 00000000..9759e6e6 --- /dev/null +++ b/test/test_restconf_listkey.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# Testcases for lists, key operations for netconf and restconf + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + +APPNAME=example + +cfg=$dir/conf.xml +fyang=$dir/list.yang + +# example +cat < $cfg + + $cfg + /usr/local/var + /usr/local/share/clixon + $IETFRFC + $fyang + false + /usr/local/var/$APPNAME/$APPNAME.sock + $dir/restconf.pidfile + /usr/local/var/$APPNAME + +EOF + +cat < $fyang +module list{ + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + container c{ + list a{ + key b; + leaf b{ + type string; + } + leaf c{ + type string; + } + } + } +} +EOF + +new "test params: -f $cfg" + +if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg" + start_backend -s init -f $cfg +fi + +new "kill old restconf daemon" +sudo pkill -u www-data -f "/www-data/clixon_restconf" + +new "start restconf daemon" +start_restconf -f $cfg + +new "waiting" +wait_backend +wait_restconf + +new "restconf PUT add entry" +expectfn 'curl -s -X PUT http://localhost/restconf/data/list:c/a=x -d {"list:a":{"b":"x","c":"0"}}' 0 '' + +new "restconf PUT change regular entry" +expectfn 'curl -s -X PUT http://localhost/restconf/data/list:c/a=x -d {"list:a":{"b":"x","c":"z"}}' 0 '' + +new "restconf PUT change key entry" +expectfn 'curl -s -X PUT http://localhost/restconf/data/list:c/a=x -d {"list:a":{"b":"y"}}' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "protocol","error-tag": "operation-failed","error-severity": "error","error-message": "api-path keys do not match data keys"}}}' + +new "restconf PUT change actual key entry" +expectfn 'curl -s -X PUT http://localhost/restconf/data/list:c/a=x/b -d {"b":"y"}' 0 '{"ietf-restconf:errors" : {"error": {"error-type": "protocol","error-tag": "operation-failed","error-severity": "error","error-message": "api-path keys do not match data keys"}}}' +exit + +new "Kill restconf daemon" +stop_restconf + +if [ $BE -eq 0 ]; then + exit # BE +fi + +new "Kill backend" +# Check if premature kill +pid=`pgrep -u root -f clixon_backend` +if [ -z "$pid" ]; then + err "backend already dead" +fi +# kill backend +stop_backend -f $cfg + +rm -rf $dir