From cdcffa768f7f3af0d8809cb3653df9e5956fe5c0 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 4 Mar 2020 11:46:30 +0100 Subject: [PATCH] * Replaced compile option `VALIDATE_STATE_XML` with runtime option `CLICON_VALIDATE_STATE_XML`. * XML childvec Compile error --- CHANGELOG.md | 2 + apps/backend/backend_client.c | 82 +++++++++++------------ include/clixon_custom.h | 11 --- lib/src/clixon_xml.c | 25 ++++--- yang/clixon/clixon-config@2020-02-22.yang | 17 ++++- 5 files changed, 72 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be38bd76..3200e624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Expected: Early March 2020 * Added `clixon-stats` state for clixon XML and memory statistics. * Added: CLICON_CLI_BUF_START and CLICON_CLI_BUF_THRESHOLD so you can change the start and threshold of quadratic and linear growth of CLIgen buffers (cbuf:s) + * Added: CLICON_VALIDATE_STATE_XML for controling validation of user state XML * JSON parse error messages change from ` on line x: syntax error,..` to `json_parse: line x: syntax error` * Unknown-element error message is more descriptive, eg from `namespace is: urn:example:clixon` to: `Failed to find YANG spec of XML node: x with parent: xp in namespace urn:example:clixon`. * C-API parse and validation API more capable @@ -66,6 +67,7 @@ Expected: Early March 2020 ### Minor changes +* Replaced compile option `VALIDATE_STATE_XML` with runtime option `CLICON_VALIDATE_STATE_XML`. * Memory footprint * Do not autopopulate namespace cache, instead use on-demand, see `xml2ns()`. * Set CBUF start level to 256 (`CLICON_CLI_BUF_START` option) diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index cd3fddd5..53103363 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1027,11 +1027,9 @@ from_client_get(clicon_handle h, int32_t depth = -1; /* Nr of levels to print, -1 is all, 0 is none */ yang_stmt *yspec; int i; -#ifdef VALIDATE_STATE_XML cxobj *xerr = NULL; cxobj *xr; cxobj *xb; -#endif username = clicon_username_get(h); if ((yspec = clicon_dbspec_yang(h)) == NULL){ @@ -1081,21 +1079,22 @@ from_client_get(clicon_handle h, /* If not only-state, then read running config * Note xret can be pruned by nacm below and change name and * merged with state data, so zero-copy cant be used - * Also, must use external namespace context here due to stmt */ -#ifdef VALIDATE_STATE_XML - if (xmldb_get0(h, "running", nsc, NULL, 1, &xret, NULL) < 0) { - if (netconf_operation_failed(cbret, "application", "read registry")< 0) - goto done; - goto ok; + if (clicon_option_bool(h, "CLICON_VALIDATE_STATE_XML")){ + if (xmldb_get0(h, "running", nsc, NULL, 1, &xret, NULL) < 0) { + if (netconf_operation_failed(cbret, "application", "read registry")< 0) + goto done; + goto ok; + } } -#else - if (xmldb_get0(h, "running", nsc, xpath, 1, &xret, NULL) < 0) { - if (netconf_operation_failed(cbret, "application", "read registry")< 0) - goto done; - goto ok; + else{ + if (xmldb_get0(h, "running", nsc, xpath, 1, &xret, NULL) < 0) { + if (netconf_operation_failed(cbret, "application", "read registry")< 0) + goto done; + goto ok; + } } -#endif /* If not only config, * get state data from plugins as defined by plugin_statedata(), if any */ @@ -1107,33 +1106,34 @@ from_client_get(clicon_handle h, goto done; goto ok; } -#ifdef VALIDATE_STATE_XML - /* Check XML by validating it. return internal error with error cause - * Primarily intended for user-supplied state-data. - * The whole config tree must be present in case the state data references config data - */ - if ((ret = xml_yang_validate_all_top(h, xret, &xerr)) < 0) - goto done; - if (ret > 0 && (ret = xml_yang_validate_add(h, xret, &xerr)) < 0) - goto done; - if (ret == 0){ - if (debug) - clicon_log_xml(LOG_DEBUG, xret, "VALIDATE_STATE"); - if ((xr = xpath_first(xerr, NULL, "//error-tag")) != NULL && - (xb = xml_body_get(xr))){ - if (xml_value_set(xb, "operation-failed") < 0) - goto done; - } - if ((xr = xpath_first(xerr, NULL, "//error-message")) != NULL && - (xb = xml_body_get(xr))){ - if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) - goto done; - } - if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0) + if (clicon_option_bool(h, "CLICON_VALIDATE_STATE_XML")){ + /* Check XML by validating it. return internal error with error cause + * Primarily intended for user-supplied state-data. + * The whole config tree must be present in case the state data references config data + */ + if ((ret = xml_yang_validate_all_top(h, xret, &xerr)) < 0) goto done; - goto ok; - } -#endif /* VALIDATE_STATE_XML */ + if (ret > 0 && (ret = xml_yang_validate_add(h, xret, &xerr)) < 0) + goto done; + if (ret == 0){ + if (debug) + clicon_log_xml(LOG_DEBUG, xret, "VALIDATE_STATE"); + if ((xr = xpath_first(xerr, NULL, "//error-tag")) != NULL && + (xb = xml_body_get(xr))){ + if (xml_value_set(xb, "operation-failed") < 0) + goto done; + } + if ((xr = xpath_first(xerr, NULL, "//error-message")) != NULL && + (xb = xml_body_get(xr))){ + if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) + goto done; + } + if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0) + goto done; + goto ok; + } + } /* CLICON_VALIDATE_STATE_XML */ + if (content == CONTENT_NONCONFIG){ /* state only, all config should be removed now */ /* Keep state data only, remove everything that is not config. Note that state data * may be a sub-part in a config tree, we need to traverse to find all @@ -1197,10 +1197,8 @@ from_client_get(clicon_handle h, retval = 0; done: clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); -#ifdef VALIDATE_STATE_XML if (xerr) xml_free(xerr); -#endif if (xpath) free(xpath); if (xnacm) diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 409ca911..2405b8a1 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -78,17 +78,6 @@ */ #define XML_EXPLICIT_INDEX -/*! Validate user state callback content - * Users may register state callbacks using ca_statedata callback - * When this option is set, the XML returned from the callback is validated after merging with the running - * db. If it fails, an internal error is returned to the originating user. - * If the option is not set, the XML returned by the user is not validated. - * Note that enabling this option causes a large performance overhead for large lists, therefore it - * is recommended to enable it during development and debugging but disable it in production, until - * this has been resolved. - */ -#define VALIDATE_STATE_XML - /*! Treat specially in a xmldb datastore. * config is treated as a "neutral" tag that does not have a yang spec. * In particular when binding xml to yang, if is encountered as top-of-tree, do not diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 8491e16c..061abca5 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -859,24 +859,24 @@ xml_child_each(cxobj *xparent, * @note does not do anything with child, you may need to set its parent, etc */ static int -xml_child_append(cxobj *x, +xml_child_append(cxobj *xp, cxobj *xc) { - if (!is_element(x)) + if (!is_element(xp)) return 0; - x->x_childvec_len++; - if (x->x_childvec_len > x->x_childvec_max){ - if (x->x_childvec_len < XML_CHILDVEC_SIZE_THRESHOLD) - x->x_childvec_max = x->x_childvec_max?2*x->x_childvec_max:XML_CHILDVEC_SIZE_START; + xp->x_childvec_len++; + if (xp->x_childvec_len > xp->x_childvec_max){ + if (xp->x_childvec_len < XML_CHILDVEC_SIZE_THRESHOLD) + xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:XML_CHILDVEC_SIZE_START; else - x->x_childvec_max += XML_CHILDVEC_SIZE_THRESHOLD; - x->x_childvec = realloc(x->x_childvec, x->x_childvec_max*sizeof(cxobj*)); - if (x->x_childvec == NULL){ + xp->x_childvec_max += XML_CHILDVEC_SIZE_THRESHOLD; + xp->x_childvec = realloc(xp->x_childvec, xp->x_childvec_max*sizeof(cxobj*)); + if (xp->x_childvec == NULL){ clicon_err(OE_XML, errno, "realloc"); return -1; } } - x->x_childvec[x->x_childvec_len-1] = xc; + xp->x_childvec[xp->x_childvec_len-1] = xc; return 0; } @@ -896,7 +896,10 @@ xml_child_insert_pos(cxobj *xp, return 0; xp->x_childvec_len++; if (xp->x_childvec_len > xp->x_childvec_max){ - xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:XML_CHILDVEC_MAX_DEFAULT; + if (xp->x_childvec_len < XML_CHILDVEC_SIZE_THRESHOLD) + xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:XML_CHILDVEC_SIZE_START; + else + xp->x_childvec_max += XML_CHILDVEC_SIZE_THRESHOLD; xp->x_childvec = realloc(xp->x_childvec, xp->x_childvec_max*sizeof(cxobj*)); if (xp->x_childvec == NULL){ clicon_err(OE_XML, errno, "realloc"); diff --git a/yang/clixon/clixon-config@2020-02-22.yang b/yang/clixon/clixon-config@2020-02-22.yang index 49a41d60..1ba45dfe 100644 --- a/yang/clixon/clixon-config@2020-02-22.yang +++ b/yang/clixon/clixon-config@2020-02-22.yang @@ -45,7 +45,8 @@ module clixon-config { "Added: search index extension, Added: clixon-stats state for clixon XML and memory statistics. Added: CLICON_CLI_BUF_START and CLICON_CLI_BUF_THRESHOLD for quadratic and linear - growth of CLIgen buffers (cbuf:s)"; + growth of CLIgen buffers (cbuf:s) + Added: CLICON_VALIDATE_STATE_XML for controling validation of user state XML"; } revision 2019-09-11 { description @@ -575,6 +576,20 @@ module clixon-config { If CLICON_XML_CHANGELOG is true, Clixon reads the module changelog from this file."; } + leaf CLICON_VALIDATE_STATE_XML { + type boolean; + default false; + description + "Validate user state callback content. + Users may register state callbacks using ca_statedata callback + When set, the XML returned from the callback is validated after merging with + the running db. If it fails, an internal error is returned to the originating + user. + If the option is not set, the XML returned by the user is not validated. + Note that enabling currently causes a large performance overhead for large + lists, therefore it is recommended to enable it during development and debugging + but disable it in production, until this has been resolved."; + } leaf CLICON_STARTUP_MODE { type startup_mode; description "Which method to boot/start clicon backend";