* Fixed: [comma in yang list name will lead to cli setting error #186](…

…#186)

CHANGELOG.md

@@ -50,6 +50,7 @@ Developers may need to change their code
 
 ### Corrected Bugs
 
+* Fixed: [comma in yang list name will lead to cli setting error #186](https://github.com/clicon/clixon/issues/186)
 * Reverted blocked signal behavior introduced in 5.0.
 
 ## 5.0.0

apps/restconf/restconf_lib.c

@@ -214,7 +214,7 @@ get_user_cookie(char  *cookiestr,
     cvec  *cvv = NULL;
     char  *c;
 
-    if (str2cvec(cookiestr, ';', '=', &cvv) < 0)
+    if (uri_str2cvec(cookiestr, ';', '=', 1, &cvv) < 0)
 	goto done;
     if ((c = cvec_find_str(cvv, attribute)) != NULL){
 	if ((*val = strdup(c)) == NULL)

apps/restconf/restconf_main_evhtp.c

@@ -363,7 +363,7 @@ evhtp_params_set(clicon_handle    h,
 	    goto done;
 	/* SSL subject fields, eg CN (Common Name) , can add more here? */
 	if ((subject = (char*)htp_sslutil_subject_tostr(req->conn->ssl)) != NULL){
-	    if (str2cvec(subject, '/', '=', &cvv) < 0)
+	    if (uri_str2cvec(subject, '/', '=', 1, &cvv) < 0)
 		goto done;
 	    if ((cn = cvec_find_str(cvv, "CN")) != NULL){
 		if (restconf_param_set(h, "SSL_CN", cn) < 0)

apps/restconf/restconf_main_fcgi.c

@@ -537,7 +537,7 @@ main(int    argc,
 		cvec  *qvec = NULL;
 		query = restconf_param_get(h, "QUERY_STRING");
 		if (query != NULL && strlen(query))
-		    if (str2cvec(query, '&', '=', &qvec) < 0)
+		    if (uri_str2cvec(query, '&', '=', 1, &qvec) < 0)
 			goto done;
 		api_root_restconf(h, req, qvec); /* This is the function */
 		if (qvec){
@@ -550,7 +550,7 @@ main(int    argc,
 		cvec  *qvec = NULL;
 		query = restconf_param_get(h, "QUERY_STRING");
 		if (query != NULL && strlen(query))
-		    if (str2cvec(query, '&', '=', &qvec) < 0)
+		    if (uri_str2cvec(query, '&', '=', 1, &qvec) < 0)
 			goto done;
 		api_stream(h, req, qvec, stream_path, &finish); 
 		if (qvec){

apps/restconf/restconf_root.c

@@ -452,7 +452,7 @@ api_root_restconf(clicon_handle        h,
 	goto done;
     }
     clicon_debug(1, "%s: api_resource=%s", __FUNCTION__, api_resource);
-    if (str2cvec(path, '/', '=', &pcvec) < 0) /* rest url eg /album=ricky/foo */
+    if (uri_str2cvec(path, '/', '=', 1, &pcvec) < 0) /* rest url eg /album=ricky/foo */
       goto done;
     /* data */
     if ((cb = restconf_get_indata(req)) == NULL) /* XXX NYI ACTUALLY not always needed, do this later? */

apps/restconf/restconf_stream_fcgi.c

@@ -418,7 +418,7 @@ api_stream(clicon_handle h,
     }
     clicon_debug(1, "%s: method=%s", __FUNCTION__, method);
 
-    if (str2cvec(path, '/', '=', &pcvec) < 0) /* rest url eg /album=ricky/foo */
+    if (uri_str2cvec(path, '/', '=', 1, &pcvec) < 0) /* rest url eg /album=ricky/foo */
 	goto done;
     /* data */
     if ((cb = restconf_get_indata(req)) == NULL)

lib/clixon/clixon_string.h

@@ -88,25 +88,25 @@ static inline char * strdup4(char *str)
  * Prototypes
  */ 
 char **clicon_strsep(char *string, char *delim, int  *nvec0);
-char *clicon_strjoin (int argc, char **argv, char *delim);
-int clixon_strsplit(char *nodeid, const int delim, char **prefix, char **id);
-int str2cvec(char *string, char delim1, char delim2, cvec **cvp);
+char  *clicon_strjoin (int argc, char **argv, char *delim);
+int    clixon_strsplit(char *nodeid, const int delim, char **prefix, char **id);
+int    uri_str2cvec(char *string, char delim1, char delim2, int decode, cvec **cvp);
 #if defined(__GNUC__) && __GNUC__ >= 3
-int uri_percent_encode(char **encp, const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
-int xml_chardata_encode(char **escp, const char *fmt, ... ) __attribute__ ((format (printf, 2, 3)));
+int    uri_percent_encode(char **encp, const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
+int    xml_chardata_encode(char **escp, const char *fmt, ... ) __attribute__ ((format (printf, 2, 3)));
 #else
-int uri_percent_encode(char **encp, const char *str, ...);
-int xml_chardata_encode(char **escp, const char *fmt, ...);
+int    uri_percent_encode(char **encp, const char *str, ...);
+int    xml_chardata_encode(char **escp, const char *fmt, ...);
 #endif
-int xml_chardata_cbuf_append(cbuf *cb, char *str);
-int uri_percent_decode(char *enc, char **str);
+int    xml_chardata_cbuf_append(cbuf *cb, char *str);
+int    uri_percent_decode(char *enc, char **str);
 const char *clicon_int2str(const map_str2int *mstab, int i);
-int clicon_str2int(const map_str2int *mstab, char *str);
-int clicon_str2int_search(const map_str2int *mstab, char *str, int upper);
-int nodeid_split(char *nodeid,	char **prefix, char **id);
-char *clixon_trim(char *str);
-char *clixon_trim2(char *str, char *trims);
-int clicon_strcmp(char *s1, char *s2);
+int    clicon_str2int(const map_str2int *mstab, char *str);
+int    clicon_str2int_search(const map_str2int *mstab, char *str, int upper);
+int    nodeid_split(char *nodeid,	char **prefix, char **id);
+char  *clixon_trim(char *str);
+char  *clixon_trim2(char *str, char *trims);
+int   clicon_strcmp(char *s1, char *s2);
 
 #ifndef HAVE_STRNDUP
 char *clicon_strndup (const char *, size_t);

lib/src/clixon_path.c

@@ -607,7 +607,7 @@ api_path_fmt2xpath(char  *api_path_fmt,
  *   cbuf *xpath = cbuf_new();
  *   cvec *cvv = NULL; 
  *   cvec *nsc = NULL;
- *   if (str2cvec("www.foo.com/restconf/a/b=c", '/', '=', &cvv) < 0)
+ *   if (uri_str2cvec("www.foo.com/restconf/a/b=c", '/', '=', 0, &cvv) < 0)
  *      err;
  *   if ((ret = api_path2xpath_cvv(yspec, cvv, 0, cxpath, &nsc, NULL)) < 0)
  *      err;
@@ -653,6 +653,8 @@ api_path2xpath_cvv(cvec       *api_path,
     cbuf      *cberr = NULL;
     char      *namespace = NULL;
     cvec      *nsc = NULL;
+    char      *val1;
+    char      *decval;
 
     cprintf(xpath, "/");
     /* Initialize namespace context */
@@ -712,6 +714,7 @@ api_path2xpath_cvv(cvec       *api_path,
 	}
 	/* Check if has value, means '=' */
         if (cv2str(cv, NULL, 0) > 0){
+	    /* val is uri percent encoded, eg x%2Cy,z */
             if ((val = cv2str_dup(cv)) == NULL)
                 goto done;
 	    switch (yang_keyword_get(y)){
@@ -737,7 +740,15 @@ api_path2xpath_cvv(cvec       *api_path,
 		    cprintf(xpath, "[");
 		    if (xprefix)
 			cprintf(xpath, "%s:", xprefix);
-		    cprintf(xpath, "%s='%s']", cv_string_get(cvi), valvec[vi++]);
+		    val1 = valvec[vi++];
+		    /* valvec is uri encoded, needs decoding */
+		    if (uri_percent_decode(val1, &decval) < 0)
+			goto done;
+		    cprintf(xpath, "%s='%s']", cv_string_get(cvi), decval);
+		    if (decval){
+			free(decval);
+			decval = NULL;
+		    }
 		}
 		break;
 	    case Y_LEAF_LIST: /* XXX: LOOP? */
@@ -837,8 +848,9 @@ api_path2xpath(char       *api_path,
 	clicon_err(OE_XML, EINVAL, "api_path is NULL");
 	goto done;
     }
-    /* Split api-path into cligen variable vector */
-    if (str2cvec(api_path, '/', '=', &cvv) < 0)
+    /* Split api-path into cligen variable vector, 
+     * dont decode since api_path2xpath_cvv takes uri encode as input */
+    if (uri_str2cvec(api_path, '/', '=', 0, &cvv) < 0)
 	goto done;
     if ((xpath = cbuf_new()) == NULL)
         goto done;
@@ -896,8 +908,7 @@ api_path2xml_vec(char      **vec,
     char      *nodeid;
     char      *name = NULL;
     char      *prefix = NULL;
-    char      *restval = NULL;
-    char      *restval_enc;
+    char      *restval;
     cxobj     *xn = NULL; /* new */
     cxobj     *xb;        /* body */
     cvec      *cvk = NULL; /* vector of index keys */
@@ -912,6 +923,7 @@ api_path2xml_vec(char      **vec,
     yang_stmt *ykey;
     char      *namespace = NULL;
     cbuf      *cberr = NULL;
+    char      *val = NULL;
 
     if ((nodeid = vec[0]) == NULL || strlen(nodeid)==0){
 	if (xbotp)
@@ -925,11 +937,9 @@ api_path2xml_vec(char      **vec,
 	goto done;
     }
     /* restval is RFC 3896 encoded */
-    if ((restval_enc = index(nodeid, '=')) != NULL){
-	*restval_enc = '\0';
-	restval_enc++;
-	if (uri_percent_decode(restval_enc, &restval) < 0)
-	    goto done;
+    if ((restval = index(nodeid, '=')) != NULL){
+	*restval = '\0';
+	restval++;
     }
     /* Split into prefix and localname */
     if (nodeid_split(nodeid, &prefix, &name) < 0)
@@ -986,7 +996,11 @@ api_path2xml_vec(char      **vec,
 	xml_spec_set(x, y);
 	if ((xb = xml_new("body", x, CX_BODY)) == NULL)
 	    goto done; 
-	if (restval && xml_value_set(xb, restval) < 0)
+	if (restval){
+	    if (uri_percent_decode(restval, &val) < 0)
+		goto done;
+	}
+	if (val && xml_value_set(xb, val) < 0)
 	    goto done;
 	break;
     case Y_LIST:
@@ -1007,6 +1021,7 @@ api_path2xml_vec(char      **vec,
 	else{
 	    /* Transform restval "a,b,c" to "a" "b" "c" (nvalvec=3) 
 	     * Note that vnr can be < length of cvk, due to empty or unset values
+	     * Note also that valvec entries are encoded
 	     */
 	    if ((valvec = clicon_strsep(restval, ",", &nvalvec)) == NULL)
 		goto done;
@@ -1044,8 +1059,16 @@ api_path2xml_vec(char      **vec,
 		if ((xb = xml_new("body", xn, CX_BODY)) == NULL)
 		    goto done;
 		if (vi++ < nvalvec){
-		    if (xml_value_set(xb, valvec[vi-1]) < 0)
+		    /* Here assign and decode key values */
+		    val = NULL;
+		    if (uri_percent_decode(valvec[vi-1], &val) < 0)
+			goto done;
+		    if (xml_value_set(xb, val) < 0)
 			goto done;
+		    if (val){
+			free(val);
+			val = NULL;
+		    }
 		}
 	    }
 	}
@@ -1078,10 +1101,10 @@ api_path2xml_vec(char      **vec,
 	free(prefix);
     if (name)
 	free(name);
-    if (restval)
-	free(restval);
     if (valvec)
 	free(valvec);
+    if (val)
+	free(val);
     return retval;
  fail:
     retval = 0; /* invalid api-path or XML */

lib/src/clixon_string.c

@@ -314,6 +314,10 @@ uri_percent_decode(char  *enc,
     int   len;
     char *ptr;
 
+    if (enc == NULL){
+	clicon_err(OE_UNIX, EINVAL, "enc is NULL");
+	goto done;
+    }
     /* This is max */
     len = strlen(enc)+1;
     if ((str = malloc(len)) == NULL){
@@ -538,18 +542,21 @@ xml_chardata_cbuf_append(cbuf *cb,
     return retval;
 }
 
-/*! Split a string into a cligen variable vector using 1st and 2nd delimiter 
- * Split a string first into elements delimited by delim1, then into
- * pairs delimited by delim2.
+/*! Split a string into a cligen variable vector using 1st and 2nd delimiter
+ * 
+ * (1) Split a string into elements delimited by delim1, 
+ * (2) split elements into pairs delimited by delim2, 
+ * (3) Optionally URI-decode values
  * @param[in]  string String to split
  * @param[in]  delim1 First delimiter char that delimits between elements
  * @param[in]  delim2 Second delimiter char for pairs within an element
+ * @param[in]  decode If set, URI decode values. The caller may want to decode, or add another level
  * @param[out] cvp    Created cligen variable vector, deallocate w cvec_free
  * @retval     0      OK
  * @retval    -1      error
  * @code
  * cvec  *cvv = NULL;
- * if (str2cvec("a=b&c=d", ';', '=', &cvv) < 0)
+ * if (uri_str2cvec("a=b&c=d", ';', '=', 1, &cvv) < 0)
  *   err;
  * @endcode
  *
@@ -558,10 +565,11 @@ xml_chardata_cbuf_append(cbuf *cb,
  * XXX differentiate between error and null cvec.
  */
 int
-str2cvec(char  *string, 
-	 char   delim1, 
-	 char   delim2, 
-	 cvec **cvp)
+uri_str2cvec(char  *string, 
+	     char   delim1, 
+	     char   delim2, 
+	     int    decode,
+	     cvec **cvp)
 {
     int     retval = -1;
     char   *s;
@@ -593,8 +601,15 @@ str2cvec(char  *string,
 	    *(snext++) = '\0';
 	if ((val = index(s, delim2)) != NULL){
 	    *(val++) = '\0';
-	    if (uri_percent_decode(val, &valu) < 0)
-		goto err;
+	    if (decode){
+		if (uri_percent_decode(val, &valu) < 0)
+		    goto err;
+	    }
+	    else
+		if ((valu = strdup(val)) == NULL){
+		    clicon_err(OE_UNIX, errno, "strdup");
+		    goto err;
+		}
 	    if ((cv = cvec_add(cvv, CGV_STRING)) == NULL){
 		clicon_err(OE_UNIX, errno, "cvec_add");
 		goto err;

lib/src/clixon_yang.c

@@ -2666,7 +2666,7 @@ yang_abs_schema_nodeid(yang_stmt    *yn,
     }
     /* Split nodeid on the form /p0:i0/p1:i1 to a cvec with [name:p0 value:i0][...]
      */
-    if (str2cvec(schema_nodeid, '/', ':', &nodeid_cvv) < 0)
+    if (uri_str2cvec(schema_nodeid, '/', ':', 1, &nodeid_cvv) < 0)
 	goto done;
     if (cvec_len(nodeid_cvv) == 0)
 	goto ok;
@@ -2749,7 +2749,7 @@ yang_desc_schema_nodeid(yang_stmt    *yn,
     }
     /* Split nodeid on the form /p0:i0/p1:i1 to a cvec with [name:p0 value:i0][...]
      */
-    if (str2cvec(schema_nodeid, '/', ':', &nodeid_cvv) < 0)
+    if (uri_str2cvec(schema_nodeid, '/', ':', 1, &nodeid_cvv) < 0)
 	goto done;
     if (cvec_len(nodeid_cvv) == 0)
 	goto ok;

test/test_restconf_listkey.sh

@@ -23,6 +23,7 @@ cat <<EOF > $cfg
   <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
   <CLICON_BACKEND_PIDFILE>$dir/restconf.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
+  <CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
   $RESTCONFIG
 </clixon-config>
 EOF
@@ -166,6 +167,12 @@ expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json
 new "restconf PUT change list+leaf-list entry (expect fail)"
 expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x,y/f=u -d '{"list:f":"w"}')" 0 '{"ietf-restconf:errors":{"error":{"error-type":"protocol","error-tag":"operation-failed","error-severity":"error","error-message":"api-path keys do not match data keys"}}}'
 
+new "restconf PUT (x,y),z -> x%2Cy,z percent-encoded"
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/list:c/a=x%2Cy,z/nonkey -d '{"list:nonkey":"foo"}')" 0 "HTTP/1.1 201 Created"
+
+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"}\]}'
+
 if [ $RC -ne 0 ]; then
     new "Kill restconf daemon"
     stop_restconf