Fixed: [Expansion removes the double quote](#524)

Add escaping in expand_dbvar instead of automatic in cligen
clicon · Nov 8, 2024 · 6ce1df2 · 6ce1df2
1 parent 7b01b02
commit 6ce1df2
Show file tree

Hide file tree

Showing 3 changed files with 170 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,8 @@ Developers may need to change their code
 
 ### Corrected Bugs
 
+* Fixed: [Expansion removes the double quote](https://github.com/clicon/clixon/issues/524)
+  *  Add escaping in expand_dbvar instead of automatic in cligen expand
 * Fixed: [string length validation doesn't work for the entry "" in case it has default value specified](https://github.com/clicon/clixon/issues/563)
 * Fixed: [SNMP: snmpwalk is slow and can timeout](https://github.com/clicon/clixon/issues/404)
 

diff --git a/apps/cli/cli_show.c b/apps/cli/cli_show.c
@@ -185,6 +185,112 @@ xpath_append(cbuf      *cb0,
     return retval;
 }
 
+/*! Insert (escaped) strings into expand coammnds
+ *
+ * Help function to expand_dbvar
+ * Detect duplicates: for ordered-by system assume list is ordered, so you need
+ * just remember previous but for ordered-by system, check the whole list
+ * Also escape strings if necessary
+ * @param[in]  h        Clixon handle
+ * @param[in]  xvec     XML vector
+ * @param[in]  xlen     Length of XML vector
+ * @param[out] commands Vector of function pointers to callback functions
+ * @retval     0        OK
+ * @retval    -1        Error
+ */
+static int
+expand_dbvar_insert(clixon_handle h,
+                    cxobj       **xvec,
+                    size_t        xlen,
+                    cvec         *commands)
+{
+    int        retval = -1;
+    cxobj     *x;
+    char      *body;
+    char      *bodyesc = NULL; /* escaped */
+    char      *bodyprev = NULL; /* previous */
+    cg_var    *cv;
+    yang_stmt *y;
+    yang_stmt *yp = NULL;
+    int        user_ordered_list = 0;
+    cg_obj    *co;
+    int        doesc;
+    int        i;
+    int        ret;
+
+    /* Dont escape if REST, peek on cligen-object */
+    if ((co = cligen_co_match(cli_cligen(h))) != NULL &&
+        ISREST(co)){
+        doesc = 0;
+    }
+    else
+        doesc = 1;
+    for (i = 0; i < xlen; i++) {
+        x = xvec[i];
+        /* First element, check if in ordered-by-user list */
+        if (i == 0){
+            if ((y = xml_spec(xvec[i])) != NULL &&
+                (yp = yang_parent_get(y)) != NULL &&
+                yang_keyword_get(yp) == Y_LIST &&
+                yang_find(yp, Y_ORDERED_BY, "user") != NULL){
+                user_ordered_list = 1;
+            }
+        }
+        if (xml_type(x) == CX_BODY)
+            body = xml_value(x);
+        else
+            body = xml_body(x);
+        if (body == NULL)
+            continue;
+        bodyesc = NULL;
+        if (doesc) {
+            if ((ret = cligen_escape_need(body)) < 0)
+                goto done;
+            if (ret){
+                if ((bodyesc = cligen_escape_do(body)) == NULL)
+                    goto done;
+            }
+        }
+        if (user_ordered_list) {
+            /* Detect duplicates linearly in existing values */
+            cv = NULL;
+            while ((cv = cvec_each(commands, cv)) != NULL)
+                if (strcmp(cv_string_get(cv), bodyesc?bodyesc:body) == 0)
+                    break;
+            if (cv == NULL){
+                if (cvec_add_string(commands, NULL, bodyesc?bodyesc:body) < 0) {
+                    clixon_err(OE_UNIX, errno, "cvec_add_string");
+                    goto done;
+                }
+            }
+        }
+        else{
+            /* Remember previous */
+            if (bodyprev && strcmp(body, bodyprev) == 0){
+                if (bodyesc){
+                    free(bodyesc);
+                    bodyesc = NULL;
+                }
+                continue; /* duplicate, assume sorted */
+            }
+            bodyprev = body;
+            if (cvec_add_string(commands, NULL, bodyesc?bodyesc:body) < 0){
+                clixon_err(OE_UNIX, errno, "cvec_add_string");
+                goto done;
+            }
+        }
+        if (bodyesc){
+            free(bodyesc);
+            bodyesc = NULL;
+        }
+    }
+    retval = 0;
+ done:
+    if (bodyesc)
+        free(bodyesc);
+    return retval;
+}
+
 /*! Completion callback of variable for configured data and automatically generated data model
  *
  * Returns an expand-type list of commands as used by cligen 'expand' 
@@ -227,15 +333,10 @@ expand_dbvar(void   *h,
     cxobj           *xe; /* direct ptr */
     cxobj           *xerr = NULL; /* free */
     size_t           xlen = 0;
-    cxobj           *x;
-    char            *bodystr;
-    int              i;
-    char            *bodystr0 = NULL; /* previous */
     cg_var          *cv;
     cxobj           *xtop = NULL; /* xpath root */
     cxobj           *xbot = NULL; /* xpath, NULL if datastore */
     yang_stmt       *y = NULL; /* yang spec of xpath */
-    yang_stmt       *yp;
     cvec            *nsc = NULL;
     int              ret;
     int              cvvi = 0;
@@ -390,42 +491,9 @@ expand_dbvar(void   *h,
     }
     if (xpath_vec(xt, nsc, "%s", &xvec, &xlen, cbuf_get(cbxpath)) < 0)
         goto done;
-    /* Loop for inserting into commands cvec. 
-     * Detect duplicates: for ordered-by system assume list is ordered, so you need
-     * just remember previous
-     * but for ordered-by system, check the whole list
-     */
-    bodystr0 = NULL;
-    for (i = 0; i < xlen; i++) {
-        x = xvec[i];
-        if (xml_type(x) == CX_BODY)
-            bodystr = xml_value(x);
-        else
-            bodystr = xml_body(x);
-        if (bodystr == NULL)
-            continue; /* no body, cornercase */
-        if ((y = xml_spec(x)) != NULL &&
-            (yp = yang_parent_get(y)) != NULL &&
-            yang_keyword_get(yp) == Y_LIST &&
-            yang_find(yp, Y_ORDERED_BY, "user") != NULL){
-            /* Detect duplicates linearly in existing values */
-            {
-                cg_var *cv = NULL;
-                while ((cv = cvec_each(commands, cv)) != NULL)
-                    if (strcmp(cv_string_get(cv), bodystr) == 0)
-                        break;
-                if (cv == NULL)
-                    cvec_add_string(commands, NULL, bodystr);
-            }
-        }
-        else{
-            if (bodystr0 && strcmp(bodystr, bodystr0) == 0)
-                continue; /* duplicate, assume sorted */
-            bodystr0 = bodystr;
-            /* RFC3986 decode */
-            cvec_add_string(commands, NULL, bodystr);
-        }
-    }
+    /* Loop for inserting into commands cvec. */
+    if (expand_dbvar_insert(h, xvec, xlen, commands) < 0)
+        goto done;
  ok:
     retval = 0;
  done:

diff --git a/test/test_cli_rest.sh b/test/test_cli_rest.sh
@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
 # CLIgen rest and delimiters test
-# Special code for <var:rest> and if the value has delimiters, such as "a b"
+# Special code for <var:rest> and if the value has delimiters and needs to be escaped,
+# such as "a b"
 
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
@@ -37,7 +38,10 @@ module clixon-example{
   yang-version 1.1;
   namespace "urn:example:clixon";
   prefix ex;
-  leaf description{
+  leaf myrest{
+      type string;
+  }
+  leaf mystr{
       type string;
   }
 }
@@ -47,11 +51,20 @@ EOF
 
 cat <<EOF > $clidir/cli1.cli
    CLICON_MODE="example";
-   description
-    ( <desc:rest>
-    | <desc:rest expand_dbvar("candidate","/clixon-example:description")> ),
-        cli_set("/clixon-example:description");
-    show configuration("Show configuration"), cli_show_auto_mode("candidate", "xml", false, false);
+   set {
+      myrest ( <rest> | <rest expand_dbvar("candidate","/clixon-example:myrest")> ),
+         cli_set("/clixon-example:myrest");
+      mystr ( <string> | <string expand_dbvar("candidate","/clixon-example:mystr")> ),
+         cli_set("/clixon-example:mystr");
+   }
+   delete {
+      myrest ( <rest> | <rest expand_dbvar("candidate","/clixon-example:myrest")> ),
+         cli_del("/clixon-example:myrest");
+      mystr ( <string> | <string expand_dbvar("candidate","/clixon-example:mystr")> ),
+         cli_del("/clixon-example:mystr");
+
+   }
+   show configuration("Show configuration"), cli_show_auto_mode("candidate", "xml", true, false);
 EOF
 
 new "test params: -f $cfg"
@@ -69,31 +82,61 @@ new "wait backend"
 wait_backend
 
 new "Add a b"
-expectpart "$($clixon_cli -1 -f $cfg description a b)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg set mystr \"a b\")" 0 "^$"
+
+new "Add a b c"
+expectpart "$($clixon_cli -1 -f $cfg set mystr \"a b c\")" 0 "^$"
+
+new "Show config"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<mystr xmlns=\"urn:example:clixon\">a b c</mystr>$"
+
+new "Re-add a b c"
+expectpart "$($clixon_cli -1 -f $cfg set mystr \"a b c\")" 0 "^$"
+
+new "Show config again"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<mystr xmlns=\"urn:example:clixon\">a b c</mystr>$"
+
+new "Expand <TAB>"
+expectpart "$(echo "set mystr 	" | $clixon_cli -f $cfg 2>&1)" 0 "set mystr \"a b c\""
+
+new "Expand a <TAB>"
+expectpart "$(echo "set mystr \"a 	" | $clixon_cli -f $cfg 2>&1)" 0 "set mystr \"a b c\""
+
+new "Show config again"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<mystr xmlns=\"urn:example:clixon\">a b c</mystr>$"
+
+new "Delete a b c"
+expectpart "$($clixon_cli -1 -f $cfg delete mystr \"a b c\")" 0 "^$"
+
+new "Show config again"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^$"
+
+new "Add a b"
+expectpart "$($clixon_cli -1 -f $cfg set myrest a b)" 0 "^$"
 
 new "Add a b c"
-expectpart "$($clixon_cli -1 -f $cfg description a b c)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg set myrest a b c)" 0 "^$"
 
 new "Show config"
-expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<description xmlns=\"urn:example:clixon\">a b c</description>$"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<myrest xmlns=\"urn:example:clixon\">a b c</myrest>$"
 
 new "Re-add a b c"
-expectpart "$($clixon_cli -1 -f $cfg description a b c)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg set myrest a b c)" 0 "^$"
 
 new "Show config again"
-expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<description xmlns=\"urn:example:clixon\">a b c</description>$"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<myrest xmlns=\"urn:example:clixon\">a b c</myrest>$"
 
 new "Expand <TAB>"
-expectpart "$(echo "description 	" | $clixon_cli -f $cfg 2>&1)" 0 "description a b c"
+expectpart "$(echo "set myrest 	" | $clixon_cli -f $cfg 2>&1)" 0 "set myrest a b c"
 
 new "Expand a <TAB>"
-expectpart "$(echo "description a 	" | $clixon_cli -f $cfg 2>&1)" 0 "description a b c"
+expectpart "$(echo "set myrest a 	" | $clixon_cli -f $cfg 2>&1)" 0 "set myrest a b c"
 
 new "Show config again"
-expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<description xmlns=\"urn:example:clixon\">a b c</description>$"
+expectpart "$($clixon_cli -1 -f $cfg show config)" 0 "^<myrest xmlns=\"urn:example:clixon\">a b c</myrest>$"
 
 new "Add a b"
-expectpart "$($clixon_cli -1 -f $cfg description a b)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg set myrest a b)" 0 "^$"
 
 if [ $BE -ne 0 ]; then
     new "Kill backend"