diff --git a/CHANGELOG.md b/CHANGELOG.md index 4629f4c2..4b652624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,14 +21,20 @@ * Note that this adds bytes to your configs ### API changes on existing features (you may need to change your code) -* Removed obsolete `CLICON_CLI_MODEL_TREENAME_PATCH` +* Strict XML Prefixed namespace check. This means all XML namespaces must always be declared by default or prefixed attribute name. There were some cases where this was not enforced. Example, `y` must be declared: +``` + --> +``` ### Minor changes +* Removed obsolete `CLICON_CLI_MODEL_TREENAME_PATCH` * Added specific clixon_suberrno code: XMLPARSE_ERRNO to identify XML parse errors. * Removed all dependency on strverscmp * Added libgen.h for baseline() ### Corrected Bugs +* Yang augment created multiple augmented children (no side-effect) +* XML prefixed attribute names were not copied into the datastore * [yang type range statement does not support multiple values](https://github.com/clicon/clixon/issues/59) * Remaining problem was mainly CLIgen feature. Strengthened test cases in [test/test_type.sh]. * Also in: [Multiple ranges support](https://github.com/clicon/clixon/issues/78) diff --git a/datastore/clixon_xmldb_text.c b/datastore/clixon_xmldb_text.c index 91b11f60..4090c97d 100644 --- a/datastore/clixon_xmldb_text.c +++ b/datastore/clixon_xmldb_text.c @@ -886,6 +886,7 @@ text_modify(struct text_handle *th, cxobj *x0c; /* base child */ cxobj *x0b; /* base body */ cxobj *x1c; /* mod child */ + char *xns; /* namespace */ char *x0bstr; /* mod body string */ char *x1bstr; /* mod body string */ yang_stmt *yc; /* yang child */ @@ -923,13 +924,18 @@ text_modify(struct text_handle *th, // int iamkey=0; if ((x0 = xml_new(x1name, x0p, (yang_stmt*)y0)) == NULL) goto done; - /* Copy xmlns attributes */ - if ((x1a = xml_find_type(x1, NULL, "xmlns", CX_ATTR)) != 0){ - if ((x0a = xml_dup(x1a)) == NULL) - goto done; - if (xml_addsub(x0, x0a) < 0) - goto done; - } + + /* Copy xmlns attributes */ + x1a = NULL; + while ((x1a = xml_child_each(x1, x1a, CX_ATTR)) != NULL) + if (strcmp(xml_name(x1a),"xmlns")==0 || + ((xns = xml_prefix(x1a)) && strcmp(xns, "xmlns")==0)){ + if ((x0a = xml_dup(x1a)) == NULL) + goto done; + if (xml_addsub(x0, x0a) < 0) + goto done; + } + #if 0 /* If it is key I dont want to mark it */ if ((iamkey=yang_key_match(y0->yn_parent, x1name)) < 0) @@ -1042,13 +1048,16 @@ text_modify(struct text_handle *th, } if ((x0 = xml_new(x1name, x0p, (yang_stmt*)y0)) == NULL) goto done; - /* Copy xmlns attributes */ - if ((x1a = xml_find_type(x1, NULL, "xmlns", CX_ATTR)) != 0){ - if ((x0a = xml_dup(x1a)) == NULL) - goto done; - if (xml_addsub(x0, x0a) < 0) - goto done; - } + /* Copy xmlns attributes */ + x1a = NULL; + while ((x1a = xml_child_each(x1, x1a, CX_ATTR)) != NULL) + if (strcmp(xml_name(x1a),"xmlns")==0 || + ((xns = xml_prefix(x1a)) && strcmp(xns, "xmlns")==0)){ + if ((x0a = xml_dup(x1a)) == NULL) + goto done; + if (xml_addsub(x0, x0a) < 0) + goto done; + } if (op==OP_NONE) xml_flag_set(x0, XML_FLAG_NONE); /* Mark for potential deletion */ } diff --git a/include/clixon_custom.h b/include/clixon_custom.h index ee8bd231..d06a9e50 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -41,6 +41,4 @@ */ #undef RPC_USERNAME_ASSERT -/* Full xmlns validation check is made only if XML has associated YANG spec -*/ -#define XMLNS_YANG_ONLY 1 + diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 2c99afb7..3ae3cae0 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -362,19 +362,12 @@ xml_localname_check(cxobj *xn, return 0; xn = xp; } -#ifdef XMLNS_YANG_ONLY - if (ys == NULL) - return 0; /* If no yang spec */ - else -#endif - { - /* Check if my namespace */ - if ((n = yang_find_myprefix(ys)) != NULL && strcmp(nsn,n)==0) - return 0; - /* Check if any imported module */ - if (yang_find_module_by_prefix(ys, nsn) != NULL) - return 0; - } + /* Check if my namespace */ + if ((n = yang_find_myprefix(ys)) != NULL && strcmp(nsn,n)==0) + return 0; + /* Check if any imported module */ + if (yang_find_module_by_prefix(ys, nsn) != NULL) + return 0; /* Not found, error */ clicon_err(OE_XML, ENOENT, "Namespace name %s in %s:%s not found", nsn, nsn, xml_name(xn)); diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 89de12ff..0eb54de4 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -358,7 +358,9 @@ validate_identityref(cxobj *xt, goto done; goto fail; } - /* Here check if node is in the derived node list of the base identity */ + /* Here check if node is in the derived node list of the base identity + * The derived node list is a cvec computed XXX + */ if (cvec_find(ybaseid->ys_cvec, node) == NULL){ cbuf_reset(cb); cprintf(cb, "Identityref validation failed, %s not derived from %s", @@ -767,7 +769,8 @@ xml_yang_validate_all(cxobj *xt, default: break; } - /* must sub-node RFC 7950 Sec 7.5.3. Can be several. */ + /* must sub-node RFC 7950 Sec 7.5.3. Can be several. + * XXX. use yang path instead? */ yc = NULL; while ((yc = yn_each((yang_node*)ys, yc)) != NULL) { if (yc->ys_keyword != Y_MUST) diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index dda790db..d1869665 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -1620,10 +1620,16 @@ ys_grouping_resolve(yang_stmt *ys, } /*! This is an augment node, augment the original datamodel. - The target node MUST be either a container, list, choice, case, input, - output, or notification node. - If the "augment" statement is on the top level the absolute form MUST be used. - @note Destructively changing a datamodel may affect outlying loop? + * @param[in] ys The augment statement + * @param[in] yspec Yang specification + * @see RFC7950 Sec 7.17 + * The target node MUST be either a container, list, choice, case, input, + * output, or notification node. + * If the "augment" statement is on the top level the absolute form MUST be + * used. + * All data nodes defined in the "augment" statement are defined as XML + * elements in the XML namespace of the module where the "augment" is + * specified. */ static int yang_augment_node(yang_stmt *ys, @@ -1631,25 +1637,25 @@ yang_augment_node(yang_stmt *ys, { int retval = -1; char *schema_nodeid; - yang_stmt *yss = NULL; + yang_stmt *ytarget = NULL; yang_stmt *yc; int i; schema_nodeid = ys->ys_argument; clicon_debug(2, "%s %s", __FUNCTION__, schema_nodeid); /* Find the target */ - if (yang_abs_schema_nodeid(ysp, ys, schema_nodeid, -1, &yss) < 0) + if (yang_abs_schema_nodeid(ysp, ys, schema_nodeid, -1, &ytarget) < 0) goto done; - if (yss == NULL) + if (ytarget == NULL) goto ok; - /* Extend yss with ys' children - * First enlarge yss vector + /* Extend ytarget with ys' children + * First enlarge ytarget vector */ for (i=0; iys_len; i++){ if ((yc = ys_dup(ys->ys_stmt[i])) == NULL) goto done; /* XXX: use prefix of origin */ - if (yn_insert((yang_node*)yss, yc) < 0) + if (yn_insert((yang_node*)ytarget, yc) < 0) goto done; } ok: @@ -1660,7 +1666,8 @@ yang_augment_node(yang_stmt *ys, /*! Find all top-level augments and change original datamodels. */ static int -yang_augment_spec(yang_spec *ysp) +yang_augment_spec(yang_spec *ysp, + int modnr) { int retval = -1; yang_stmt *ym; @@ -1668,7 +1675,7 @@ yang_augment_spec(yang_spec *ysp) int i; int j; - i = 0; + i = modnr; while (iyp_len){ /* Loop through modules and sub-modules */ ym = ysp->yp_stmt[i++]; j = 0; @@ -2320,8 +2327,9 @@ yang_parse_post(clicon_handle h, int retval = -1; int i; - /* 1: Parse from text to yang parse-tree. */ - /* Iterate through modules */ + /* 1: Parse from text to yang parse-tree. + * Iterate through modules and detect module/submodules to parse + * - note the list may grow on each iteration */ for (i=modnr; iyp_len; i++) if (yang_parse_recurse(h, yspec->yp_stmt[i], yspec) < 0) goto done; @@ -2382,7 +2390,7 @@ yang_parse_post(clicon_handle h, } /* 8: Top-level augmentation of all modules XXX: only new modules? */ - if (yang_augment_spec(yspec) < 0) + if (yang_augment_spec(yspec, modnr) < 0) goto done; /* 9: sanity check of schemanode references, need more here */ diff --git a/test/test_augment.yang b/test/test_augment.yang new file mode 100755 index 00000000..bc2fe89e --- /dev/null +++ b/test/test_augment.yang @@ -0,0 +1,140 @@ +#!/bin/bash +# yang augment and identityref tests in different modules +# See RFC7950 Sec 7.17 +# This test defines an example-augment module which augments an interface +# defined in ietf-interface module. The interface then consists of identities +# both defined in the basic ietf-interfaces module (type) as well as the main +# module through the augmented module () +# The ietf-interfaces is very restricted (not original). +# Work-in-progress + +# 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_yang.xml +fyang=$dir/main.yang +fyang2=$dir/ietf-interfaces@2019-03-04.yang + +cat < $cfg + + $cfg + a:test + $dir + /usr/local/share/clixon + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + 1 + /usr/local/var/$APPNAME + /usr/local/lib/xmldb/text.so + true + +EOF + +# Stub ietf-interfaces for test +cat < $fyang2 +module ietf-interfaces { + yang-version 1.1; + namespace "urn:ietf:params:xml:ns:yang:ietf-interfaces"; + revision "2019-03-04"; + prefix if; + identity interface-type { + description + "Base identity from which specific interface types are + derived."; + } + identity eth { + base interface-type; + } + identity fddi { + base interface-type; + } + container interfaces { + description "Interface parameters."; + list interface { + key "name"; + leaf name { + type string; + } + leaf type { + type identityref { + base interface-type; + } + mandatory true; + } + } + } +} +EOF + +# From rfc7950 sec 7.17 +# Note "when" is not present +cat < $fyang +module example-augment { + yang-version 1.1; + namespace "urn:example:augment"; + prefix mymod; + revision "2019-03-04"; + import ietf-interfaces { + prefix if; + } + identity some-new-iftype { + base if:interface-type; + } + augment "/if:interfaces/if:interface" { +/* when 'derived-from-or-self(if:type, "mymod:some-new-iftype")'; */ + leaf mandatory-leaf { + mandatory true; + type string; + } + } +} +EOF + +new "test params: -f $cfg -y $fyang" + +if [ $BE -ne 0 ]; then + new "kill old backend" + sudo clixon_backend -zf $cfg -y $fyang + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg -y $fyang" + start_backend -s init -f $cfg -y $fyang + + new "waiting" + sleep $RCWAIT +fi + +# mandatory-leaf See RFC7950 Sec 7.17 +# Error1: the xml should have xmlns for "mymod" +# XMLNS_YANG_ONLY must be undeffed +new "netconf set new if my w augmented type" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 'mymymod:some-new-iftypetrue]]>]]>' "^]]>]]>$" + +new "netconf validate ok" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 "]]>]]>" '^]]>]]>$' + +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 "]]>]]>" "^]]>]]>$" + +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 +sudo pkill -u root -f clixon_backend + + + +rm -rf $dir diff --git a/test/test_compat.sh b/test/test_compat.sh deleted file mode 100755 index 92e9254d..00000000 --- a/test/test_compat.sh +++ /dev/null @@ -1,39 +0,0 @@ -#!/bin/bash -# Test of backward compatibility, from last release to newer. -# - -# 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_startup.xml - -# Use yang in example - -cat < $cfg - - $cfg - /usr/local/share/clixon - $IETFRFC - clixon-example - $APPNAME - /usr/local/lib/$APPNAME/backend - /usr/local/lib/$APPNAME/netconf - /usr/local/lib/$APPNAME/restconf - /usr/local/lib/$APPNAME/cli - /usr/local/lib/$APPNAME/clispec - /usr/local/var/$APPNAME/$APPNAME.sock - /usr/local/var/$APPNAME/$APPNAME.pidfile - 1 - /usr/local/var/$APPNAME - /usr/local/lib/xmldb/text.so - 0 - init - - -EOF - -# Nothing - -rm -rf $dir diff --git a/test/test_xpath.sh b/test/test_xpath.sh index 5cf902a9..318ef5c2 100755 --- a/test/test_xpath.sh +++ b/test/test_xpath.sh @@ -27,7 +27,7 @@ cat < $xml EOF cat < $xml2 - + e0 @@ -35,9 +35,9 @@ cat < $xml2 -e0 +e0 myfamily - + v6ur:ipv6-unicast foo