diff --git a/CHANGELOG.md b/CHANGELOG.md index d31c6a28c..e2754cf81 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index c83bbd6d4..b778f479a 100644 --- a/apps/restconf/restconf_lib.c +++ b/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) diff --git a/apps/restconf/restconf_main_evhtp.c b/apps/restconf/restconf_main_evhtp.c index b74a805f1..a814c06ab 100644 --- a/apps/restconf/restconf_main_evhtp.c +++ b/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) diff --git a/apps/restconf/restconf_main_fcgi.c b/apps/restconf/restconf_main_fcgi.c index 95397ad97..630ef9ad9 100644 --- a/apps/restconf/restconf_main_fcgi.c +++ b/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){ diff --git a/apps/restconf/restconf_root.c b/apps/restconf/restconf_root.c index 9dfe652f7..a8123d0c8 100644 --- a/apps/restconf/restconf_root.c +++ b/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? */ diff --git a/apps/restconf/restconf_stream_fcgi.c b/apps/restconf/restconf_stream_fcgi.c index 72fa1305b..2df4613f9 100644 --- a/apps/restconf/restconf_stream_fcgi.c +++ b/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) diff --git a/lib/clixon/clixon_string.h b/lib/clixon/clixon_string.h index 467d80b2e..fea7fd8a3 100644 --- a/lib/clixon/clixon_string.h +++ b/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); diff --git a/lib/src/clixon_path.c b/lib/src/clixon_path.c index eefc261af..33101948d 100644 --- a/lib/src/clixon_path.c +++ b/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 */ diff --git a/lib/src/clixon_string.c b/lib/src/clixon_string.c index f09cdf354..736489c74 100644 --- a/lib/src/clixon_string.c +++ b/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; diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 51dc15a74..46346e6bc 100644 --- a/lib/src/clixon_yang.c +++ b/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; diff --git a/test/test_restconf_listkey.sh b/test/test_restconf_listkey.sh index a2bb068a3..a95e30eda 100755 --- a/test/test_restconf_listkey.sh +++ b/test/test_restconf_listkey.sh @@ -23,6 +23,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.sock $dir/restconf.pidfile /usr/local/var/$APPNAME + false $RESTCONFIG 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