From 97316e0bfa833a5dfaf79edacc6d6e49973ca493 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 22 Feb 2022 13:14:30 +0100 Subject: [PATCH] * Follow-up on [restconf GET json response does not encode top level node with namespace as per rfc #303](https://github.com/clicon/clixon/issues/303) * Load/save JSON config file did not work * Added rfc7951 parameter to `clixon_json_parse_string()` and `clixon_json_parse_file()` * If set, honor RFC 7951: JSON Encoding of Data Modeled with YANG, eg it requires module name prefixes * If not set, parse as regular JSON * Test: added test_db.sh for datastore format tests --- CHANGELOG.md | 6 + apps/cli/cli_common.c | 47 ++++-- apps/restconf/restconf_methods.c | 2 +- apps/restconf/restconf_methods_patch.c | 2 +- apps/restconf/restconf_methods_post.c | 4 +- example/main/example_backend.c | 2 +- lib/clixon/clixon_json.h | 4 +- lib/src/clixon_datastore_read.c | 2 +- lib/src/clixon_json.c | 46 ++++-- test/test_db.sh | 203 +++++++++++++++++++++++++ util/clixon_util_json.c | 2 +- util/clixon_util_socket.c | 2 +- util/clixon_util_xml.c | 2 +- 13 files changed, 281 insertions(+), 43 deletions(-) create mode 100755 test/test_db.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ec6d71..f5dd55c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,12 @@ Users may have to change how they access the system * `configure --with-wwwdir=` is removed * Command field of clixon-lib:process-control RPC reply used CDATA encoding but now uses regular XML encoding +### C/CLI-API changes on existing features + +* Added rfc7951 parameter to `clixon_json_parse_string()` and `clixon_json_parse_file()` + * If set, honor RFC 7951: JSON Encoding of Data Modeled with YANG, eg it requires module name prefixes + * If not set, parse as regular JSON + ### Minor features * Backend ignore of SIGPIPE. This occurs if client quits unexpectedly over the UNIX socket. diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index 63c8d68d..bb02a6a3 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -794,25 +794,31 @@ load_config_file(clicon_handle h, cvec *cvv, cvec *argv) { - int ret = -1; - struct stat st; - char *filename = NULL; - int replace; - cg_var *cv; - char *opstr; - char *varstr; - FILE *fp = NULL; - cxobj *xt = NULL; - cxobj *x; - cbuf *cbxml; - char *formatstr = NULL; - enum format_enum format = FORMAT_XML; + int ret = -1; + struct stat st; + char *filename = NULL; + int replace; + cg_var *cv; + char *opstr; + char *varstr; + FILE *fp = NULL; + cxobj *xt = NULL; + cxobj *x; + cbuf *cbxml; + char *formatstr = NULL; + enum format_enum format = FORMAT_XML; + yang_stmt *yspec; + cxobj *xerr = NULL; if (cvec_len(argv) < 2 || cvec_len(argv) > 4){ clicon_err(OE_PLUGIN, EINVAL, "Received %d arguments. Expected: ,[,]", cvec_len(argv)); goto done; } + if ((yspec = clicon_dbspec_yang(h)) == NULL){ + clicon_err(OE_FATAL, 0, "No DB_SPEC"); + goto done; + } if (cvec_len(argv) > 2){ formatstr = cv_string_get(cvec_i(argv, 2)); if ((int)(format = format_str2int(formatstr)) < 0){ @@ -846,12 +852,20 @@ load_config_file(clicon_handle h, } switch (format){ case FORMAT_XML: - if (clixon_xml_parse_file(fp, YB_NONE, NULL, &xt, NULL) < 0) + if ((ret = clixon_xml_parse_file(fp, YB_NONE, yspec, &xt, &xerr)) < 0) goto done; + if (ret == 0){ + clixon_netconf_error(xerr, "Loading", filename); + goto done; + } break; case FORMAT_JSON: - if (clixon_json_parse_file(fp, YB_NONE, NULL, &xt, NULL) < 0) + if ((ret = clixon_json_parse_file(fp, 1, YB_NONE, yspec, &xt, &xerr)) < 0) goto done; + if (ret == 0){ + clixon_netconf_error(xerr, "Loading", filename); + goto done; + } break; default: clicon_err(OE_PLUGIN, 0, "format: %s not implemented", formatstr); @@ -874,9 +888,10 @@ load_config_file(clicon_handle h, cbuf_get(cbxml)) < 0) goto done; cbuf_free(cbxml); - // } ret = 0; done: + if (xerr) + xml_free(xerr); if (xt) xml_free(xt); if (fp) diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index 94bee113..a87b7269 100644 --- a/apps/restconf/restconf_methods.c +++ b/apps/restconf/restconf_methods.c @@ -333,7 +333,7 @@ api_data_write(clicon_handle h, } break; case YANG_DATA_JSON: - if ((ret = clixon_json_parse_string(data, yb, yspec, &xdata0, &xerr)) < 0){ + if ((ret = clixon_json_parse_string(data, 1, yb, yspec, &xdata0, &xerr)) < 0){ if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0) goto done; if (api_return_err0(h, req, xerr, pretty, media_out, 0) < 0) diff --git a/apps/restconf/restconf_methods_patch.c b/apps/restconf/restconf_methods_patch.c index 9dcaa4c0..f63b5fe1 100644 --- a/apps/restconf/restconf_methods_patch.c +++ b/apps/restconf/restconf_methods_patch.c @@ -783,7 +783,7 @@ api_data_yang_patch(clicon_handle h, ret = clixon_xml_parse_string(data, YB_MODULE, yspec, &xpatch, &xerr); break; case YANG_PATCH_JSON: /* RFC 8072 patch */ - ret = clixon_json_parse_string(data, YB_MODULE, yspec, &xpatch, &xerr); + ret = clixon_json_parse_string(data, 1, YB_MODULE, yspec, &xpatch, &xerr); break; default: restconf_unsupported_media(h, req, pretty, media_out); diff --git a/apps/restconf/restconf_methods_post.c b/apps/restconf/restconf_methods_post.c index d0d0cc19..f3c55023 100644 --- a/apps/restconf/restconf_methods_post.c +++ b/apps/restconf/restconf_methods_post.c @@ -250,7 +250,7 @@ api_data_post(clicon_handle h, } break; case YANG_DATA_JSON: - if ((ret = clixon_json_parse_string(data, yb, yspec, &xbot, &xerr)) < 0){ + if ((ret = clixon_json_parse_string(data, 1, yb, yspec, &xbot, &xerr)) < 0){ if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0) goto done; if (api_return_err0(h, req, xerr, pretty, media_out, 0) < 0) @@ -461,7 +461,7 @@ api_operations_post_input(clicon_handle h, case YANG_DATA_JSON: /* XXX: Here data is on the form: {"clixon-example:input":null} and has no proper yang binding * support */ - if ((ret = clixon_json_parse_string(data, YB_NONE, yspec, &xdata, &xerr)) < 0){ + if ((ret = clixon_json_parse_string(data, 1, YB_NONE, yspec, &xdata, &xerr)) < 0){ if (netconf_malformed_message_xml(&xerr, clicon_err_reason) < 0) goto done; if (api_return_err0(h, req, xerr, pretty, media_out, 0) < 0) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index a9044e3f..527f3092 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -544,7 +544,7 @@ example_statefile(clicon_handle h, * @param[in] h Generic handler * @param[in] xpath Registered XPath using canonical prefixes * @param[in] userargs Per-call user arguments - * @param[in] arg Per-path user argument + * @param[in] arg Per-path user argument (at register time) */ int example_pagination(void *h0, diff --git a/lib/clixon/clixon_json.h b/lib/clixon/clixon_json.h index 05742307..5ff09077 100644 --- a/lib/clixon/clixon_json.h +++ b/lib/clixon/clixon_json.h @@ -50,7 +50,7 @@ int xml2json(FILE *f, cxobj *x, int pretty); int xml2json_cb(FILE *f, cxobj *x, int pretty, clicon_output_cb *fn); int json_print(FILE *f, cxobj *x); int xml2json_vec(FILE *f, cxobj **vec, size_t veclen, int pretty); -int clixon_json_parse_string(char *str, yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xret); -int clixon_json_parse_file(FILE *fp, yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xret); +int clixon_json_parse_string(char *str, int rfc7951, yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xret); +int clixon_json_parse_file(FILE *fp, int rfc7951, yang_bind yb, yang_stmt *yspec, cxobj **xt, cxobj **xret); #endif /* _CLIXON_JSON_H */ diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 5b2d5e6a..1034f769 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -482,7 +482,7 @@ xmldb_readfile(clicon_handle h, * * ret == 0 should not happen with YB_NONE. Binding is done later */ if (strcmp(format, "json")==0){ - if (clixon_json_parse_file(fp, YB_NONE, yspec, &x0, xerr) < 0) + if (clixon_json_parse_file(fp, 1, YB_NONE, yspec, &x0, xerr) < 0) goto done; } else { diff --git a/lib/src/clixon_json.c b/lib/src/clixon_json.c index 431b46e5..f1b5770e 100644 --- a/lib/src/clixon_json.c +++ b/lib/src/clixon_json.c @@ -786,8 +786,9 @@ xml2json1_cbuf(cbuf *cb, modname = yang_argument_get(ymod); /* Special case for ietf-netconf -> ietf-restconf translation * A special case is for return data on the form {"data":...} + * See also json_xmlns_translate() */ - if (strcmp(modname, "ietf-netconf")==0) + if (strcmp(modname, "ietf-netconf") == 0) modname = "ietf-restconf"; if (modname0 && strcmp(modname, modname0) == 0) modname=NULL; @@ -1275,6 +1276,12 @@ json_xmlns_translate(yang_stmt *yspec, int ret; if ((modname = xml_prefix(x)) != NULL){ /* prefix is here module name */ + /* Special case for ietf-netconf -> ietf-restconf translation + * A special case is for return data on the form {"data":...} + * See also xml2json1_cbuf + */ + if (strcmp(modname, "ietf-restconf") == 0) + modname = "ietf-netconf"; if ((ymod = yang_find_module_by_name(yspec, modname)) == NULL){ if (xerr && netconf_unknown_namespace_xml(xerr, "application", @@ -1314,8 +1321,9 @@ json_xmlns_translate(yang_stmt *yspec, * are split and interpreted as in RFC7951 * * @param[in] str Input string containing JSON - * @param[in] yb How to bind yang to XML top-level when parsing - * @param[in] yspec If set, also do yang validation + * @param[in] rfc7951 Do sanity checks according to RFC 7951 JSON Encoding of Data Modeled with YANG + * @param[in] yb How to bind yang to XML top-level when parsing (if rfc7951) + * @param[in] yspec Yang specification (if rfc 7951) * @param[out] xt XML top of tree typically w/o children on entry (but created) * @param[out] xerr Reason for invalid returned as netconf err msg * @@ -1328,6 +1336,7 @@ json_xmlns_translate(yang_stmt *yspec, */ static int _json_parse(char *str, + int rfc7951, yang_bind yb, yang_stmt *yspec, cxobj *xt, @@ -1362,17 +1371,18 @@ _json_parse(char *str, /* RFC 7951 Section 4: A namespace-qualified member name MUST be used for all * members of a top-level JSON object */ - if (yspec && xml_prefix(x) == NULL && + if (rfc7951 && xml_prefix(x) == NULL){ /* XXX: For top-level config file: */ - (yb != YB_NONE || strcmp(xml_name(x),DATASTORE_TOP_SYMBOL)!=0)){ - if ((cberr = cbuf_new()) == NULL){ - clicon_err(OE_UNIX, errno, "cbuf_new"); - goto done; + if (yb != YB_NONE || strcmp(xml_name(x),DATASTORE_TOP_SYMBOL)!=0){ + if ((cberr = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cberr, "Top-level JSON object %s is not qualified with namespace which is a MUST according to RFC 7951", xml_name(x)); + if (xerr && netconf_malformed_message_xml(xerr, cbuf_get(cberr)) < 0) + goto done; + goto fail; } - cprintf(cberr, "Top-level JSON object %s is not qualified with namespace which is a MUST according to RFC 7951", xml_name(x)); - if (xerr && netconf_malformed_message_xml(xerr, cbuf_get(cberr)) < 0) - goto done; - goto fail; } /* Names are split into name/prefix, but now add namespace info */ if ((ret = json_xmlns_translate(yspec, x, xerr)) < 0) @@ -1442,6 +1452,7 @@ _json_parse(char *str, /*! Parse string containing JSON and return an XML tree * * @param[in] str String containing JSON + * @param[in] rfc7951 Do sanity checks according to RFC 7951 JSON Encoding of Data Modeled with YANG * @param[in] yb How to bind yang to XML top-level when parsing * @param[in] yspec Yang specification, mandatory to make module->xmlns translation * @param[in,out] xt Top object, if not exists, on success it is created with name 'top' @@ -1452,7 +1463,7 @@ _json_parse(char *str, * * @code * cxobj *x = NULL; - * if (clixon_json_parse_string(str, YB_MODULE, yspec, &x, &xerr) < 0) + * if (clixon_json_parse_string(str, 1, YB_MODULE, yspec, &x, &xerr) < 0) * err; * xml_free(x); * @endcode @@ -1462,6 +1473,7 @@ _json_parse(char *str, */ int clixon_json_parse_string(char *str, + int rfc7951, yang_bind yb, yang_stmt *yspec, cxobj **xt, @@ -1476,7 +1488,7 @@ clixon_json_parse_string(char *str, if ((*xt = xml_new("top", NULL, CX_ELMNT)) == NULL) return -1; } - return _json_parse(str, yb, yspec, *xt, xerr); + return _json_parse(str, rfc7951, yb, yspec, *xt, xerr); } /*! Read a JSON definition from file and parse it into a parse-tree. @@ -1492,13 +1504,14 @@ clixon_json_parse_string(char *str, * But this is not done if yspec=NULL, and is not part of the JSON spec * * @param[in] fp File descriptor to the JSON file (ASCII string) + * @param[in] rfc7951 Do sanity checks according to RFC 7951 JSON Encoding of Data Modeled with YANG * @param[in] yspec Yang specification, or NULL * @param[in,out] xt Pointer to (XML) parse tree. If empty, create. * @param[out] xerr Reason for invalid returned as netconf err msg * * @code * cxobj *xt = NULL; - * if (clixon_json_parse_file(stdin, YB_MODULE, yspec, &xt) < 0) + * if (clixon_json_parse_file(stdin, 1, YB_MODULE, yspec, &xt) < 0) * err; * xml_free(xt); * @endcode @@ -1515,6 +1528,7 @@ clixon_json_parse_string(char *str, */ int clixon_json_parse_file(FILE *fp, + int rfc7951, yang_bind yb, yang_stmt *yspec, cxobj **xt, @@ -1551,7 +1565,7 @@ clixon_json_parse_file(FILE *fp, if ((*xt = xml_new(JSON_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL) goto done; if (len){ - if ((ret = _json_parse(ptr, yb, yspec, *xt, xerr)) < 0) + if ((ret = _json_parse(ptr, rfc7951, yb, yspec, *xt, xerr)) < 0) goto done; if (ret == 0) goto fail; diff --git a/test/test_db.sh b/test/test_db.sh new file mode 100755 index 00000000..0cf9eee7 --- /dev/null +++ b/test/test_db.sh @@ -0,0 +1,203 @@ +#!/usr/bin/env bash +# Datastore tests: +# - XML and JSON +# - save and load config files + +# 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 + +# include err() and new() functions and creates $dir + +cfg=$dir/conf_yang.xml +fyang=$dir/clixon-example.yang +fclispec=$dir/clispec.cli + +# Use yang in example + +cat < $cfg + + $cfg + ${YANG_INSTALLDIR} + $IETFRFC + $fyang + /usr/local/lib/$APPNAME/backend + $APPNAME + /usr/local/lib/$APPNAME/cli + $dir + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + +EOF + +cat < $fyang +module clixon-example{ + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + /* Generic config data */ + container table{ + list parameter{ + key name; + leaf name{ + type string; + } + leaf value{ + type string; + } + } + } +} +EOF + +cat < $fclispec +CLICON_MODE="example"; +CLICON_PROMPT="%U@%H %w> "; +CLICON_PLUGIN="example_cli"; + +# Autocli syntax tree operations +set @datamodel, cli_auto_set(); +merge @datamodel, cli_auto_merge(); +create @datamodel, cli_auto_create(); +delete("Delete a configuration item") @datamodel, cli_auto_del(); +validate("Validate changes"), cli_validate(); +commit("Commit the changes"), cli_commit(); +discard("Discard edits (rollback 0)"), discard_changes(); + +load("Load configuration from XML file") ("Filename (local filename)"){ + xml("Replace candidate with file containing XML"), load_config_file("","filename", "replace", "xml"); + json("Replace candidate with file containing JSON"), load_config_file("","filename", "replace", "json"); +} +save("Save candidate configuration to XML file") ("Filename (local filename)"){ + xml("Save configuration as XML"), save_config_file("candidate","filename", "xml"); + json("Save configuration as JSON"), save_config_file("candidate","filename", "json"); +} +show("Show a particular state of the system") + configuration("Show configuration"), cli_auto_show("datamodel", "candidate", "xml", false, false); +quit("Quit"), cli_quit(); +EOF + +# Restconf test routine with arguments: +# 1. format: xml/json +# 2. pretty: false/true - pretty-printed XMLDB +function testrun() +{ + format=$1 + pretty=$2 + + if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -z -f $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg -o CLICON_XMLDB_FORMAT=$format -o CLICON_XMLDB_PRETTY=$pretty" + start_backend -s init -f $cfg -o CLICON_XMLDB_FORMAT=$format -o CLICON_XMLDB_PRETTY=$pretty + fi + + new "wait backend" + wait_backend + + new "cli configure parameter a" + expectpart "$($clixon_cli -1 -f $cfg set table parameter a value 42)" 0 "^$" + + new "cli show config xml" + expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^a42
$" + + new "Check xmldb $format format" + # permission kludges + sudo chmod 666 $dir/candidate_db + if [ "$format" = xml ]; then + if [ "$pretty" = false ]; then + cat < $dir/expect +<${DATASTORE_TOP}>a42
+EOF + else + cat < $dir/expect +<${DATASTORE_TOP}> + + + a + 42 + +
+ +EOF + fi + else + if [ "$pretty" = false ]; then + cat < $dir/expect +{"$DATASTORE_TOP":{"clixon-example:table":{"parameter":[{"name":"a","value":"42"}]}}} +EOF + else + cat < $dir/expect +{ + "${DATASTORE_TOP}": { + "clixon-example:table": { + "parameter": [ + { + "name": "a", + "value": "42" + } + ] + } + } +} +EOF + fi + fi + + # -w ignore white space + ret=$(diff -w $dir/candidate_db $dir/expect) + if [ $? -ne 0 ]; then + err "$(cat $dir/expect)" "$(cat $dir/candidate_db)" + fi + + new "save config file" + expectpart "$($clixon_cli -1 -f $cfg save $dir/myconfig $format)" 0 "^$" + + new "discard" + expectpart "$($clixon_cli -1 -f $cfg discard)" 0 "^$" + + new "load config file" + expectpart "$($clixon_cli -1 -f $cfg load $dir/myconfig $format)" 0 "^$" + + new "cli show config xml" + expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^a42
$" + + if [ $BE -ne 0 ]; then + 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 + fi +} + +new "test params: -f $cfg" + +new "test db xml" +testrun xml false + +new "test db xml pretty" +testrun xml true + +new "test db json" +testrun json false + +new "test db json pretty" +testrun json true + +rm -rf $dir + +unset format +unset pid +unset ret + +new "endtest" +endtest diff --git a/util/clixon_util_json.c b/util/clixon_util_json.c index 60d26e97..269591b3 100644 --- a/util/clixon_util_json.c +++ b/util/clixon_util_json.c @@ -139,7 +139,7 @@ main(int argc, return -1; } } - if ((ret = clixon_json_parse_file(stdin, yspec?YB_MODULE:YB_NONE, yspec, &xt, &xerr)) < 0) + if ((ret = clixon_json_parse_file(stdin, yspec?1:0, yspec?YB_MODULE:YB_NONE, yspec, &xt, &xerr)) < 0) goto done; if (ret == 0){ xml_print(stderr, xerr); diff --git a/util/clixon_util_socket.c b/util/clixon_util_socket.c index 97a6ff01..943153d0 100644 --- a/util/clixon_util_socket.c +++ b/util/clixon_util_socket.c @@ -149,7 +149,7 @@ main(int argc, } /* 2. Parse data (xml/json) */ if (jsonin){ - if ((ret = clixon_json_parse_file(fp, YB_NONE, NULL, &xt, &xerr)) < 0) + if ((ret = clixon_json_parse_file(fp, 0, YB_NONE, NULL, &xt, &xerr)) < 0) goto done; if (ret == 0){ fprintf(stderr, "Invalid JSON\n"); diff --git a/util/clixon_util_xml.c b/util/clixon_util_xml.c index f169605e..72f8bbb5 100644 --- a/util/clixon_util_xml.c +++ b/util/clixon_util_xml.c @@ -298,7 +298,7 @@ main(int argc, } /* 2. Parse data (xml/json) */ if (jsonin){ - if ((ret = clixon_json_parse_file(fp, top_input_filename?YB_PARENT:YB_MODULE, yspec, &xt, &xerr)) < 0) + if ((ret = clixon_json_parse_file(fp, 1, top_input_filename?YB_PARENT:YB_MODULE, yspec, &xt, &xerr)) < 0) goto done; if (ret == 0){ clixon_netconf_error(xerr, "util_xml", NULL);