Compile option VALIDATE_STATE_XML introduced in include/custom.h to control whether code for state data validation is compiled or not.

This commit is contained in:
Olof hagsand 2020-01-29 21:56:50 +01:00
parent f5209b1fab
commit 20d28b4796
5 changed files with 75 additions and 48 deletions

View file

@ -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.

View file

@ -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)

View file

@ -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;

View file

@ -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

View file

@ -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: