* RESTCONF in Clixon used empty key as "wildchar". But according to R…

…FC 8040 it should mean the "empty string". * Example: `GET restconf/data/x:a=` * Previous meaning (wrong): Return all `a` elements. * New meaning (correct): Return the `a` instance with empty key string: "". * [RESTCONF GET request of single-key list with empty string returns all elements #213](#213) * [RESTCONF GETof lists with empty string keys does not work #214](#214)
clicon · May 5, 2021 · 17e7b25 · 17e7b25
1 parent af04ec9
commit 17e7b25
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,15 @@ Expected: June 2021
 * Yang deviation [deviation statement not yet support #211](https://github.com/clicon/clixon/issues/211)
   * See RFC7950 Sec 5.6.3
 
+### API changes on existing protocol/config features
+
+Users may have to change how they access the system
+
+* RESTCONF in Clixon used empty key as "wildchar". But according to RFC 8040 it should mean the "empty string".
+  * Example: `GET restconf/data/x:a=`
+  * Previous meaning (wrong): Return all `a` elements.
+  * New meaning (correct): Return the `a` instance with empty key string: "".
+
 ### Minor features
 
 * Add default network namespace constant: `RESTCONF_NETNS_DEFAULT` with default value "default".
@@ -46,6 +55,8 @@ Expected: June 2021
 
 ### Corrected Bugs
 
+* [RESTCONF GET request of single-key list with empty string returns all elements #213](https://github.com/clicon/clixon/issues/213)
+* [RESTCONF GETof lists with empty string keys does not work #214](https://github.com/clicon/clixon/issues/214)
 * Fixed: [Multiple http requests in native restconf yields same reply #212](https://github.com/clicon/clixon/issues/212)
 
 ## 5.1.0
@@ -77,6 +88,8 @@ The 5.1 release contains more RESTCONF native mode restructuring, new multi-yang
 
 ### API changes on existing protocol/config features
 
+Users may have to change how they access the system
+
 * Native RESTCONF mode
   * Configure native mode changed to: `configure --with-restconf=native`, NOT `evhtp`
   * Use libevhtp from https://github.com/clixon/clixon-libevhtp.git, NOT from criticalstack

diff --git a/lib/src/clixon_path.c b/lib/src/clixon_path.c
@@ -713,7 +713,7 @@ api_path2xpath_cvv(cvec       *api_path,
 		goto done;
 	}
 	/* Check if has value, means '=' */
-        if (cv2str(cv, NULL, 0) > 0){
+	if (cv_type_get(cv) == CGV_STRING){
 	    /* val is uri percent encoded, eg x%2Cy,z */
             if ((val = cv2str_dup(cv)) == NULL)
                 goto done;

diff --git a/lib/src/clixon_string.c b/lib/src/clixon_string.c
@@ -560,8 +560,9 @@ xml_chardata_cbuf_append(cbuf *cb,
  *   err;
  * @endcode
  *
- * a=b&c=d    ->  [[a,"b"][c="d"]
- * kalle&c=d  ->  [[c="d"]]  # Discard elements with no delim2
+ * a=b&c=d    ->  [[a,"b"][c,"d"]
+ * a&b=       ->  [[a,null][b,""]]  
+ * Note difference between empty (CGV_EMPTY) and empty string (CGV_STRING)
  * XXX differentiate between error and null cvec.
  */
 int
@@ -622,12 +623,11 @@ uri_str2cvec(char  *string,
 	}
 	else{
 	    if (strlen(s)){
-		if ((cv = cvec_add(cvv, CGV_STRING)) == NULL){
+		if ((cv = cvec_add(cvv, CGV_EMPTY)) == NULL){
 		    clicon_err(OE_UNIX, errno, "cvec_add");
 		    goto err;
 		}
 		cv_name_set(cv, s);
-		cv_string_set(cv, "");
 	    }
 	}
 	s = snext;

diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c
@@ -251,7 +251,7 @@ xpath_tree2cbuf(xpath_tree *xs,
 	cprintf(xcb, "/");
 	break;
     case XP_PRIME_STR:
-	cprintf(xcb, "'%s'", xs->xs_s0);
+	cprintf(xcb, "'%s'", xs->xs_s0?xs->xs_s0:"");
 	break;
     case XP_PRIME_NR:
 	cprintf(xcb, "%s", xs->xs_strnr?xs->xs_strnr:"0"); 

diff --git a/lib/src/clixon_xpath_eval.c b/lib/src/clixon_xpath_eval.c
@@ -824,8 +824,12 @@ xp_relop(xp_ctx    *xc1,
 		s1 = xml_body(x);
 		switch(op){
 		case XO_EQ:
-		    if (s1 == NULL || s2 == NULL)
-			xr->xc_bool = (s1==NULL && s2 == NULL);
+		    if (s1 == NULL && s2 == NULL)
+			xr->xc_bool = 1;
+		    else if (s1 == NULL && strlen(s2) == 0)
+			xr->xc_bool = 1;
+		    else if (strlen(s1) == 0 && s2 == NULL)
+			xr->xc_bool = 1;
 		    else
 			xr->xc_bool = (strcmp(s1, s2)==0);
 		    break;

diff --git a/lib/src/clixon_xpath_parse.y b/lib/src/clixon_xpath_parse.y
@@ -441,9 +441,9 @@ predicates  : predicates '[' expr ']' { $$=xp_new(XP_PRED,A_NAN,NULL, NULL, NULL
 primaryexpr : '(' expr ')'         { $$=xp_new(XP_PRI0,A_NAN,NULL, NULL, NULL, $2, NULL); clicon_debug(3,"primaryexpr-> ( expr )"); } 
             | NUMBER               { $$=xp_new(XP_PRIME_NR,A_NAN, $1, NULL, NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> NUMBER(%s)", $1); /*XXX*/} 
             | QUOTE string QUOTE   { $$=xp_new(XP_PRIME_STR,A_NAN,NULL, $2, NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> \" string \""); }
-            | QUOTE QUOTE          { $$=xp_new(XP_PRIME_STR,A_NAN,NULL, NULL, NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> \" \""); } 
+            | QUOTE QUOTE          { $$=xp_new(XP_PRIME_STR,A_NAN,NULL,  strdup(""), NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> \" \""); } 
             | APOST string APOST   { $$=xp_new(XP_PRIME_STR,A_NAN,NULL, $2, NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> ' string '"); }
-            | APOST APOST          { $$=xp_new(XP_PRIME_STR,A_NAN,NULL, NULL, NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> ' '"); } 
+            | APOST APOST          { $$=xp_new(XP_PRIME_STR,A_NAN,NULL, strdup(""), NULL, NULL, NULL);clicon_debug(3,"primaryexpr-> ' '"); } 
             | FUNCTIONNAME ')'      { if (($$ = xp_primary_function(_XPY, $1, NULL)) == NULL) YYERROR; clicon_debug(3,"primaryexpr-> functionname ()"); }
             | FUNCTIONNAME args ')' { if (($$ = xp_primary_function(_XPY, $1, $2)) == NULL) YYERROR;  clicon_debug(3,"primaryexpr-> functionname (arguments)"); } 
             ;

diff --git a/test/test_restconf_listkey.sh b/test/test_restconf_listkey.sh
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 # Testcases for Restconf list and leaf-list keys, check matching keys for RFC8040 4.5:
 # the key values must match in URL and data
+# Empty keys vs comma as a key
 
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@@ -82,7 +83,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
@@ -91,11 +92,11 @@ if [ $RC -ne 0 ]; then
 
     new "start restconf daemon"
     start_restconf -f $cfg
-
-    new "waiting"
-    wait_restconf
 fi
 
+new "wait restconf"
+wait_restconf
+
 new "restconf PUT add whole list entry"
 expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y -d '{"list:a":{"b":"x","c":"y","nonkey":"0"}}')" 0 "HTTP/1.1 201 Created"
 
@@ -112,6 +113,7 @@ expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCP
 new "restconf PUT add whole list entry XML"
 expectpart "$(curl $CURLOPTS -X PUT -H 'Content-Type: application/yang-data+xml' -H 'Accept: application/yang-data+xml' -d '<a xmlns="urn:example:clixon"><b>xx</b><c>xy</c><nonkey>0</nonkey></a>' $RCPROTO://localhost/restconf/data/list:c/a=xx,xy)" 0 "HTTP/1.1 201 Created"
 
+new "restconf GET sub key"
 expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a/b)" 0 'HTTP/1.1 400 Bad Request' '^{"ietf-restconf:errors":{"error":{"error-type":"rpc","error-tag":"malformed-message","error-severity":"error","error-message":"malformed key =a, expected '
 
 new "restconf PUT change whole list entry (same keys)"
@@ -147,6 +149,15 @@ expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json
 new "restconf PUT list-list"
 expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y/e=z -d '{"list:e":{"f":"z","nonkey":"0"}}')" 0 "HTTP/1.1 201 Created"
 
+new "restconf GET e element with empty string key expect empty"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y/e=)" 0 "HTTP/1.1 404 Not Found" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}'
+
+new "restconf PUT list-list empty string"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y/e= -d '{"list:e":{"f":"","nonkey":"42"}}')" 0 "HTTP/1.1 201 Created"
+
+new "restconf GET e element with empty string key"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+xml" $RCPROTO://localhost/restconf/data/list:c/a=x,y/e=)" 0 "HTTP/1.1 200 OK" '<e xmlns="urn:example:clixon"><f/><nonkey>42</nonkey></e>'
+
 new "restconf PUT change list-lst entry (wrong keys)(expect fail)"
 expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y/e=z -d '{"list:e":{"f":"wrong","nonkey":"0"}}')" 0 "HTTP/1.1 412 Precondition Failed" '{"ietf-restconf:errors":{"error":{"error-type":"protocol","error-tag":"operation-failed","error-severity":"error","error-message":"api-path keys do not match data keys"}}}'
 
@@ -174,6 +185,33 @@ expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json
 new "restconf GET check percent-encoded"
 expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x%2Cy,z)" 0 "HTTP/1.1 200 OK" '{"list:a":\[{"b":"x,y","c":"z","nonkey":"foo"}\]}'
 
+new "restconf PUT ,z empty string as first key"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=,z -d '{"list:a":{"b":"","c":"z", "nonkey":"42"}}')" 0 "HTTP/1.1 201 Created"
+
+new "restconf GET ,z empty"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=,z)" 0 "HTTP/1.1 200 OK" '{"list:a":\[{"b":"","c":"z","nonkey":"42"}\]}'
+
+new "restconf PUT x, empty string as second key"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x, -d '{"list:a":{"b":"x","c":"", "nonkey":"43"}}')" 0 "HTTP/1.1 201 Created"
+
+new "restconf GET x, empty"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,)" 0 "HTTP/1.1 200 OK" '{"list:a":\[{"b":"x","c":"","nonkey":"43"}\]}'
+
+new "restconf PUT , empty string as second key"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=, -d '{"list:a":{"b":"","c":"", "nonkey":"44"}}')" 0 "HTTP/1.1 201 Created"
+
+new "restconf GET , empty"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=,)" 0 "HTTP/1.1 200 OK" '{"list:a":\[{"b":"","c":"","nonkey":"44"}\]}'
+
+new "restconf PUT two commas as keys"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=%2C,%2C -d '{"list:a":{"b":",","c":",", "nonkey":"45"}}')" 0 "HTTP/1.1 201 Created"
+
+new "restconf GET two commas"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=%2C,%2C)" 0 "HTTP/1.1 200 OK" '{"list:a":\[{"b":",","c":",","nonkey":"45"}\]}'
+
+new "restconf GET all empty strings"
+expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c)" 0 "HTTP/1.1 200 OK" '{"list:c":{"a":\[{"b":"","c":"","nonkey":"44"},{"b":"","c":"z","nonkey":"42"},{"b":",","c":",","nonkey":"45"},{"b":"x","c":"","nonkey":"43"}'
+
 if [ $RC -ne 0 ]; then
     new "Kill restconf daemon"
     stop_restconf 

diff --git a/test/test_restconf_patch.sh b/test/test_restconf_patch.sh
@@ -111,7 +111,7 @@ if [ $BE -ne 0 ]; then
     start_backend -s startup -f $cfg
 fi
 
-new "waiting"
+new "wait backend"
 wait_backend
 
 if [ $RC -ne 0 ]; then
@@ -120,11 +120,11 @@ if [ $RC -ne 0 ]; then
 
     new "start restconf daemon"
     start_restconf -f $cfg
-
-    new "waiting restconf"
-    wait_restconf
 fi
 
+new "wait restconf"
+wait_restconf
+
 # also in test_restconf.sh
 new "MUST support the PATCH method for a plain patch" 
 expectpart "$(curl -u andy:bar $CURLOPTS -X OPTIONS $RCPROTO://localhost/restconf/data)" 0 "HTTP/1.1 200 OK" "Allow: OPTIONS,HEAD,GET,POST,PUT,PATCH,DELETE" "Accept-Patch: application/yang-data+xml,application/yang-data+json"
@@ -174,7 +174,7 @@ if [ $BE -ne 0 ]; then
     start_backend -s startup -f $cfg
 fi
 
-new "waiting"
+new "wait backend"
 wait_backend
 
 if [ $RC -ne 0 ]; then
@@ -183,11 +183,11 @@ if [ $RC -ne 0 ]; then
 
     new "start restconf daemon"
     start_restconf -f $cfg
-
-    new "waiting"
-    wait_restconf
 fi
 
+new "wait restconf"
+wait_restconf
+
 # 4.6.1.  Plain Patch
 new "Create album London Calling with PUT"
 expectpart "$(curl -u andy:bar $CURLOPTS -X PUT -H 'Content-Type: application/yang-data+json' $RCPROTO://localhost/restconf/data/example-jukebox:jukebox/library/artist=Clash/album=London%20Calling -d '{"example-jukebox:album":{"name":"London Calling"}}')" 0 "HTTP/1.1 201 Created"