From 20d28b479691a142b83600914e67facb5e0558bb Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 29 Jan 2020 21:56:50 +0100 Subject: [PATCH] Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. --- CHANGELOG.md | 1 + apps/backend/backend_client.c | 56 ++++++++++++++++++++++++++++++++++- apps/backend/backend_plugin.c | 54 ++++++--------------------------- include/clixon_custom.h | 8 +++++ lib/src/clixon_xpath_eval.c | 4 +-- 5 files changed, 75 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbba6c80..44268286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 4.3.1 (Expected: January 2020) ### Corrected Bugs +* Compile option `VALIDATE_STATE_XML` introduced in `include/custom.h` to control whether code for state data validation is compiled or not. * Fixed: Validation of user state data led to wrong validation, if state relied on config data, eg leafref/must/when etc. * Fixed: No revision in yang module led to errors in validation of state data * Fixed: Leafref validation did not cover case of when the "path" statement is declared within a typedef, only if it was declared in the data part directly under leaf. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 943e9510..100eb162 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -252,6 +252,7 @@ client_get_streams(clicon_handle h, * @param[in] h Clicon handle * @param[in] xpath Xpath selection, not used but may be to filter early * @param[in] nsc XML Namespace context for xpath + * @param[in] content config/state or both * @param[in,out] xret Existing XML tree, merge x into this * @retval -1 Error (fatal) * @retval 0 Statedata callback failed (clicon_err called) @@ -261,6 +262,7 @@ static int client_statedata(clicon_handle h, char *xpath, cvec *nsc, + netconf_content content, cxobj **xret) { int retval = -1; @@ -271,6 +273,9 @@ client_statedata(clicon_handle h, yang_stmt *ymod; int ret; char *namespace; +#ifdef VALIDATE_STATE_XML + cxobj *xerr = NULL; +#endif if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, ENOENT, "No yang spec"); @@ -320,6 +325,51 @@ client_statedata(clicon_handle h, goto done; if (ret == 0) goto fail; + +#ifdef VALIDATE_STATE_XML + /* Check XML from state callback by validating it. return internal + * error with error cause + * Only if config has been read, disable validation on state-only + */ + if (content != CONTENT_NONCONFIG){ + 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){ + cxobj *xe; + cxobj *xb; + +#if 1 + if (debug){ + cbuf *ccc=cbuf_new(); + if (clicon_xml2cbuf(ccc, *xret, 0, 0, -1) < 0) + goto done; + clicon_debug(1, "%s FAIL: %s", __FUNCTION__, cbuf_get(ccc)); + cbuf_free(ccc); + } +#endif + if ((xe = xpath_first(xerr, NULL, "//error-tag")) != NULL && + (xb = xml_body_get(xe))){ + if (xml_value_set(xb, "operation-failed") < 0) + goto done; + } + if ((xe = xpath_first(xerr, NULL, "//error-message")) != NULL && + (xb = xml_body_get(xe))){ + if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) + goto done; + } + if (*xret){ + xml_free(*xret); + *xret = NULL; + } + *xret = xerr; + xerr = NULL; + goto fail; + } + } +#endif /* VALIDATE_STATE_XML */ + /* Code complex to filter out anything that is outside of xpath * Actually this is a safety catch, should really be done in plugins * and modules_state functions. @@ -344,6 +394,10 @@ client_statedata(clicon_handle h, retval = 1; /* OK */ done: clicon_debug(1, "%s %d", __FUNCTION__, retval); +#ifdef VALIDATE_STATE_XML + if (xerr) + xml_free(xerr); +#endif if (xvec) free(xvec); return retval; @@ -955,7 +1009,7 @@ from_client_get(clicon_handle h, if (content != CONTENT_CONFIG){ /* Get state data from plugins as defined by plugin_statedata(), if any */ clicon_err_reset(); - if ((ret = client_statedata(h, xpath, nsc, &xret)) < 0) + if ((ret = client_statedata(h, xpath, nsc, content, &xret)) < 0) goto done; if (ret == 0){ /* Error from callback (error in xret) */ if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0) diff --git a/apps/backend/backend_plugin.c b/apps/backend/backend_plugin.c index 19da9990..9e5c398b 100644 --- a/apps/backend/backend_plugin.c +++ b/apps/backend/backend_plugin.c @@ -111,12 +111,10 @@ clixon_plugin_statedata(clicon_handle h, { int retval = -1; int ret; - cxobj *xerr = NULL; cxobj *x = NULL; clixon_plugin *cp = NULL; plgstatedata_t *fn; /* Plugin statedata fn */ cbuf *cberr = NULL; - int i = 0; while ((cp = clixon_plugin_each(h, cp)) != NULL) { if ((fn = cp->cp_api.ca_statedata) == NULL) @@ -125,7 +123,15 @@ clixon_plugin_statedata(clicon_handle h, goto done; if (fn(h, nsc, xpath, x) < 0) goto fail; /* Dont quit here on user callbacks */ - i++; /* indicates that new state data is added */ +#if 1 + if (debug){ + cbuf *ccc=cbuf_new(); + if (clicon_xml2cbuf(ccc, x, 0, 0, -1) < 0) + goto done; + clicon_debug(1, "%s STATE: %s", __FUNCTION__, cbuf_get(ccc)); + cbuf_free(ccc); + } +#endif if (xml_apply(x, CX_ELMNT, xml_spec_populate, yspec) < 0) goto done; if ((ret = netconf_trymerge(x, yspec, xret)) < 0) @@ -137,54 +143,12 @@ clixon_plugin_statedata(clicon_handle h, x = NULL; } } - if (i){ - /* Check XML from state callback by validating it. return internal - * error with error cause - */ - 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){ - cxobj *xe; - cxobj *xb; - - if ((xe = xpath_first(xerr, NULL, "//error-tag")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_set(xb, "operation-failed") < 0) - goto done; - } - if ((xe = xpath_first(xerr, NULL, "//error-message")) != NULL && - (xb = xml_body_get(xe))){ - if (xml_value_append(xb, " Internal error, state callback returned invalid XML") < 0) - goto done; - } - if (*xret){ - xml_free(*xret); - *xret = NULL; - } - *xret = xerr; - xerr = NULL; - goto fail; - } -#if 1 - if (debug){ - cbuf *ccc=cbuf_new(); - if (clicon_xml2cbuf(ccc, x, 0, 0, -1) < 0) - goto done; - clicon_debug(1, "%s MERGE: %s", __FUNCTION__, cbuf_get(ccc)); - cbuf_free(ccc); - } -#endif - } retval = 1; done: if (cberr) cbuf_free(cberr); if (x) xml_free(x); - if (xerr) - xml_free(xerr); return retval; fail: retval = 0; diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 2da8699c..34ebbe85 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -64,3 +64,11 @@ * binary search. This only works if "y" has proper yang binding and is sorted by system */ #undef XPATH_LIST_OPTIMIZE + +/*! Validate user state callback content + * Use 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. + */ +#define VALIDATE_STATE_XML diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c index 60ee0172..dd3c0965 100644 --- a/lib/src/clixon_xpath_eval.c +++ b/lib/src/clixon_xpath_eval.c @@ -932,7 +932,7 @@ xp_eval(xp_ctx *xc, xp_ctx *xr2 = NULL; int use_xr0 = 0; /* In 2nd child use transitively result of 1st child */ - if (debug) + if (debug > 1) ctx_print(stderr, xc, xpath_tree_int2str(xs->xs_type)); /* Pre-actions before check first child c0 */ @@ -1088,7 +1088,7 @@ xp_eval(xp_ctx *xc, xr0 = NULL; } ok: - if (debug) + if (debug > 1) ctx_print(stderr, *xrp, xpath_tree_int2str(xs->xs_type)); retval = 0; done: