Skip to content

Commit

Permalink
Fixed: [if choice is declared with multiple elements or leaf-list wit…
Browse files Browse the repository at this point in the history 
…h in a case scope , addition or updation is not happening as expected](#327)

  * This includes several choice/case adjustments to follow RFC 7950 Sec 7.9 better
  • Loading branch information
@olofhagsand
olofhagsand committed Apr 29, 2022
1 parent 28bd146 commit ef302f0
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ Users may have to change how they access the system

### Corrected Bugs

* Fixed: [if choice is declared with multiple elements or leaf-list with in a case scope , addition or updation is not happening as expected](https://github.com/clicon/clixon/issues/327)
* This includes several choice/case adjustments to follow RFC 7950 Sec 7.9 better
* Fixed: HTTP/1 parse error for '/' path
* Fixed: YANG if-feature in config file of disables feature did not work, was always on
* This does not apply to the datastore, only the config file itself.
Expand Down
91 changes: 84 additions & 7 deletions lib/src/clixon_datastore_write.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,86 @@ check_when_condition(cxobj *x0p,
goto done;
}

/*! Check if choice nodes and implicitly remove all other cases.
*
* Special case is if yc parent (yp) is choice/case
* then find x0 child with same yc even though it does not match lexically
* However this will give another y0c != yc
* @param[in] x0 Base tree node
* @param[in] y1c Yang spec of tree child. If null revert to linear search.
* @retval 0 OK
* @retval -1 Error
*
* From RFC 7950 Sec 7.9
* Since only one of the choice's cases can be valid at any time in the
* data tree, the creation of a node from one case implicitly deletes
* all nodes from all other cases. If a request creates a node from a
* case, the server will delete any existing nodes that are defined in
* other cases inside the choice.
*/
static int
check_delete_existing_case(cxobj *x0,
yang_stmt *y1c)
{
int retval = -1;
yang_stmt *yp;
yang_stmt *y0case;
yang_stmt *y1case;
yang_stmt *y0choice;
yang_stmt *y1choice;
yang_stmt *y0c;
cxobj *x0c;
cxobj *x0prev;

if ((yp = yang_parent_get(y1c)) == NULL)
goto ok;
if (yang_keyword_get(yp) == Y_CASE){
y1case = yp;
y1choice = yang_parent_get(y1case);
}
else if (yang_keyword_get(yp) == Y_CHOICE){
y1case = NULL;
y1choice = yp;
}
else
goto ok;
x0prev = NULL;
x0c = NULL;
while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) {
if ((y0c = xml_spec(x0c)) == NULL ||
(yp = yang_parent_get(y0c)) == NULL){
x0prev = x0c;
continue;
}
if (yang_keyword_get(yp) == Y_CASE){
y0case = yp;
y0choice = yang_parent_get(y0case);
}
else if (yang_keyword_get(yp) == Y_CHOICE){
y0case = NULL;
y0choice = yp;
}
else{
x0prev = x0c;
continue;
}
if (y0choice == y1choice){
if (y0case == NULL ||
y0case != y1case){
if (xml_purge(x0c) < 0)
goto done;
x0c = x0prev;
continue;
}
}
x0prev = x0c;
}
ok:
retval = 0;
done:
return retval;
}

/*! Modify a base tree x0 with x1 with yang spec y according to operation op
* @param[in] h Clicon handle
* @param[in] x0 Base xml tree (can be NULL in add scenarios)
Expand Down Expand Up @@ -695,7 +775,7 @@ text_modify(clicon_handle h,
}
x1c = NULL;
i = 0;
while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) {
while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) {
x1cname = xml_name(x1c);
/* Get yang spec of the child by child matching */
yc = yang_find_datanode(y0, x1cname);
Expand Down Expand Up @@ -724,16 +804,13 @@ text_modify(clicon_handle h,
x1cname, yang_find_mynamespace(y0));
goto done;
}
/* Check if existing case should be deleted */
if (check_delete_existing_case(x0, yc) < 0)
goto done;
/* See if there is a corresponding node in the base tree */
x0c = NULL;
if (match_base_child(x0, x1c, yc, &x0c) < 0)
goto done;
if (x0c && (yc != xml_spec(x0c))){
/* There is a match but is should be replaced (choice)*/
if (xml_purge(x0c) < 0)
goto done;
x0c = NULL;
}
x0vec[i++] = x0c; /* != NULL if x0c is matching x1c */
}
/* Second pass: Loop through children of the x1 modification tree again
Expand Down
2 changes: 1 addition & 1 deletion lib/src/clixon_validate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ check_list_unique_minmax(cxobj *xt,
ych = y;
keyw = yang_keyword_get(y);
if (keyw != Y_LIST && keyw != Y_LEAF_LIST){
if (yprev != NULL && y == yprev && yang_choice(y)==NULL){
if (yprev != NULL && y == yprev){
/* Only lists and leaf-lists are allowed to be many
* This checks duplicate container and leafs
*/
Expand Down
19 changes: 0 additions & 19 deletions lib/src/clixon_xml_sort.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,10 +1148,6 @@ match_base_child(cxobj *x0,
cg_var *cvi;
cxobj *xb;
char *keyname;
cxobj *x0c = NULL;
yang_stmt *y0c;
yang_stmt *y0p;
yang_stmt *yp; /* yang parent */
clixon_xvec *xvec = NULL;

*x0cp = NULL; /* init return value */
Expand All @@ -1160,21 +1156,6 @@ match_base_child(cxobj *x0,
*x0cp = xml_find(x0, xml_name(x1c));
goto ok;
}
/* Special case is if yc parent (yp) is choice/case
* then find x0 child with same yc even though it does not match lexically
* However this will give another y0c != yc
*/
if ((yp = yang_choice(yc)) != NULL){
x0c = NULL;
while ((x0c = xml_child_each(x0, x0c, CX_ELMNT)) != NULL) {
if ((y0c = xml_spec(x0c)) != NULL &&
(y0p = yang_choice(y0c)) != NULL &&
y0p == yp)
break; /* x0c will have a value */
}
*x0cp = x0c;
goto ok; /* What to do if not found? */
}
switch (yang_keyword_get(yc)){
case Y_CONTAINER: /* Equal regardless */
case Y_LEAF: /* Equal regardless */
Expand Down
40 changes: 32 additions & 8 deletions test/test_choice.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ if [ $BE -ne 0 ]; then
start_backend -s init -f $cfg
fi

new "waiting"
new "wait backend"
wait_backend

if [ $RC -ne 0 ]; then
Expand All @@ -158,10 +158,11 @@ if [ $RC -ne 0 ]; then
new "start restconf daemon"
start_restconf -f $cfg

new "waiting"
wait_restconf
fi

new "wait restconf"
wait_restconf

# First vanilla (protocol) case
new "netconf validate empty"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
Expand Down Expand Up @@ -283,26 +284,32 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
# Choice with multiple items

new "netconf choice multiple items first"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" op=\"replace\">
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" nc:operation=\"replace\" xmlns:nc=\"$BASENS\">
<ma>0</ma>
<ma>1</ma>
<mb>2</mb>
</system></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf get multiple items"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><system xmlns=\"urn:example:config\"><ma>0</ma><ma>1</ma><mb>2</mb></system>" ""

new "netconf validate multiple ok"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf choice multiple items second"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" op=\"replace\">
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" nc:operation=\"replace\" xmlns:nc=\"$BASENS\">
<mb>0</mb>
<mb>1</mb>
</system></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf validate multiple ok"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
new "netconf get items second"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><system xmlns=\"urn:example:config\"><mb>0</mb><mb>1</mb></system>" ""

new "netconf validate items second expect fail"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>protocol</error-type><error-tag>operation-failed</error-tag><error-app-tag>too-many-elements</error-app-tag><error-severity>error</error-severity><error-path>/system/mb</error-path></rpc-error></rpc-reply>"

new "netconf choice multiple items mix"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" op=\"replace\">
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" nc:operation=\"replace\" xmlns:nc=\"$BASENS\">
<ma>0</ma>
<mb>2</mb>
<mc>2</mc>
Expand All @@ -311,6 +318,23 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS>
new "netconf validate multiple error"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>bad-element</error-tag><error-info><bad-element>mc</bad-element></error-info><error-severity>error</error-severity><error-message>Element in choice statement already exists</error-message></rpc-error></rpc-reply>"

# Double merge
new "netconf choice single item"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" nc:operation=\"replace\" xmlns:nc=\"$BASENS\">
<ma>12</ma>
</system></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf choice merge second item"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><system xmlns=\"urn:example:config\" nc:operation=\"merge\" xmlns:nc=\"$BASENS\">
<mb>99</mb>
</system></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

new "netconf get both items"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>" "<rpc-reply $DEFAULTNS><data><system xmlns=\"urn:example:config\"><ma>12</ma><mb>99</mb></system>" ""

new "netconf validate ok"
expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"

if [ $RC -ne 0 ]; then
new "Kill restconf daemon"
stop_restconf
Expand Down

0 comments on commit ef302f0

Please sign in to comment.