xml bind yang error in xml_bind_yang_rpc_reply

[nowaits]

When using clicon_rpc_netconf_xml process rpc message, I found there is some error with xml_bind_yang_rpc_reply.
And the following two line need replace by

clixon/lib/src/clixon_xml_bind.c

Lines 665 to 666 in 1f0147f

	xml_spec_set(xrpc, yo);
	if ((ret = xml_bind_yang(xrpc, YB_MODULE, yspec, xerr)) < 0)

xml_spec_set(x, yo); 
if ((ret = xml_bind_yang(x, YB_PARENT, NULL, xerr)) < 0)

[olofhagsand]

Yes, there is something strange with this function. But I would need some more input on the context for when this is a problem? Such as the xml (xrpc) and corresponding yang.
I am not sure that the diff above solves it, since the while loop preceding it makes an effort to identify the "yo" object.
Can you please submit such an example subset to illustrate the problem?

[nowaits]

Yes, there is something strange with this function. But I would need some more input on the context for when this is a problem? Such as the xml (xrpc) and corresponding yang.
I am not sure that the diff above solves it, since the while loop preceding it makes an effort to identify the "yo" object.
Can you please submit such an example subset to illustrate the problem?

My yang file:

module unigw {
    yang-version 1.1;
    namespace "http://www.xdja.com/unigw";
    prefix unigw;
    description
	"TOMO";
   
   typedef daemon-type {
        type enumeration {
            enum vpp {
                description
                    "vpp service";
            }
            enum backend {
                description
                    "clixon backend service";
            }
            enum restconf {
                description
                    "restconf service";
            }
        }
        description
            "unigw daemon type";
    }

	rpc version {
		description "Get vpp/clixon detail version";
        input {
            leaf type {
            description "type";
            type daemon-type;
            mandatory true;
            }
        }

        output {
            leaf type {
            description "type";
            type daemon-type;
            mandatory true;
            }
            leaf build-version {
            description "version";
            type string;
            }
            leaf build-directory {
            description "directory";
            type string;
            }
            leaf build-date {
            description "date";
            type string;
            }
        }
    }
}

backend rpc function:

static int
show_vpp_version(clicon_handle h,            /* Clicon handle */
    cxobj        *xe,           /* Request: <rpc><xn></rpc> */
    cbuf         *cbret,        /* Reply eg <rpc-reply>... */
    void         *arg,          /* client_entry */
    void         *regarg)       /* Argument given at register */
{
    int    retval = -1;
    cxobj *xp;
    char  *namespace;
    char  *msgid;
    vpp_info_t *vinfo;
    char *type;

    if ((namespace = xml_find_type_value(xe, NULL, "xmlns", CX_ATTR)) == NULL) {
        clicon_err(OE_XML, ENOENT, "No namespace given in rpc %s", xml_name(xe));
        goto done;
    }
    cprintf(cbret, "<rpc-reply xmlns=\"%s\"", NETCONF_BASE_NAMESPACE);
    if ((xp = xml_parent(xe)) != NULL &&
        (msgid = xml_find_value(xp, "message-id"))) {
        cprintf(cbret, " message-id=\"%s\">", msgid);
    }

    type = xml_find_body(xe, "type");

    cprintf(cbret, "><version xmlns=\"http://www.xdja.com/unigw\">");

    vinfo = clixon_value(h, "vpp_info");

    cprintf(cbret, "<type>%s</type>", type);
    cprintf(cbret, "<build-version>%s</build-version>", vinfo->version);
    cprintf(cbret, "<build-directory>%s</build-directory>", vinfo->build_directory);
    cprintf(cbret, "<build-date>%s</build-date>", vinfo->build_date);

    cprintf(cbret, "</version></rpc-reply>");
    retval = 0;
done:
    return retval;
}

cli.c function

int
show_version(clicon_handle h,
    cvec         *cvv,
    cvec         *argv)
{
    int        retval = -1;
    char *str;

    cxobj     *xtop = NULL;
    cxobj     *xrpc;
    cxobj     *xrpc_replay;
    cxobj     *xret = NULL;
    cxobj     *xerr = NULL;

    cli_info_t *ci;

    ci = clixon_value(h, "ci_info");

    assert(cvec_len(argv) == 1);

    str = cv_string_get(cvec_i(argv, 0));

    if (clixon_xml_parse_va(YB_NONE, NULL, &xtop, NULL,
            "<rpc xmlns=\"%s\" message-id=\"%d\">"
            "<version xmlns=\"http://www.xdja.com/unigw\"><type>%s</type></version></rpc>",
            NETCONF_BASE_NAMESPACE, ci->message_id_generator++, str) < 0) {
        goto done;
    }

    /* Skip top-level */
    xrpc = xml_child_i(xtop, 0);

    /* Send to backend */
    if (clicon_rpc_netconf_xml(h, xrpc, &xret, NULL, &xerr /* I extend the function with xerr param */) < 0) {
        goto done;
    }

    if (xerr) {
        cligen_output(stdout, "rpc relay xml format error!");
        clicon_xml2file_cb(stdout, xerr, 0, 1, cligen_output);
        goto done;
    }

    xrpc_replay = xml_child_i(xret, 0);

    if ((xerr = xpath_first(xrpc_replay, NULL, "//rpc-error")) != NULL) {
        clixon_netconf_error(xerr, "Get configuration", NULL);
        goto done;
    }

    xerr = NULL;

    if (xml_yang_validate_add(h, xrpc_replay, &xerr) < 0 || xerr != NULL) {
        clixon_netconf_error(xerr, "Get configuration", NULL);
        goto done;
    }

    fprintf(stdout, "\n");
    xml2txt_cb(stdout, xrpc_replay, cligen_output);

    retval = 0;
done:
    if (xret) {
        xml_free(xret);
    }
    if (xtop) {
        xml_free(xtop);
    }
    return retval;
}

[olofhagsand]

See patch dee4e87
Please verify.
The xml is checked for yang errors and are reported as an internal error but with all error details in the error-message.
BTW the top-level <version xmlns="http://www.xdja.com/unigw"> should not be in the rpc reply, just place the <type> etc tags directly under rpc-reply, as defined in RFC 6241.
A sketch on an updated show_version is as follows:

int
show_version(clicon_handle h,
    cvec         *cvv,
    cvec         *argv)
{
    int        retval = -1;
    char      *str;
    cxobj     *xtop = NULL;
    cxobj     *xrpc;
    cxobj     *xrpc_replay;
    cxobj     *xret = NULL;
    static int message_id_generator = 0;

    str = cv_string_get(cvec_i(argv, 0));

    if (clixon_xml_parse_va(YB_NONE, NULL, &xtop, NULL,
            "<rpc xmlns=\"%s\" message-id=\"%d\">"
            "<version xmlns=\"http://www.xdja.com/unigw\"><type>%s</type></version></rpc>",
            NETCONF_BASE_NAMESPACE, message_id_generator++, str) < 0) {
        goto done;
    }

    /* Skip top-level */
    xrpc = xml_child_i(xtop, 0);

    /* Send to backend */
    if (clicon_rpc_netconf_xml(h, xrpc, &xret, NULL /* I extend the function with xerr param */) < 0) {
        goto done;
    }
    if (xpath_first(xret, NULL, "rpc-reply/rpc-error") != NULL){
        cligen_output(stdout, "rpc relay xml format error!");
        clicon_xml2file_cb(stdout, xret, 0, 1, cligen_output);
        goto done;
    }

    xrpc_replay = xml_child_i(xret, 0);

#if 0
    /* No need to validate? */
    if (xml_yang_validate_add(h, xrpc_replay, &xerr) < 0 || xerr != NULL) {
        clixon_netconf_error(xerr, "Get configuration", NULL);
        goto done;
    }
#endif
    fprintf(stdout, "\n");
    xml2txt_cb(stdout, xrpc_replay, cligen_output);

    retval = 0;
done:
    if (xret) {
        xml_free(xret);
    }
    if (xtop) {
        xml_free(xtop);
    }
    return retval;
}

[nowaits]

Hi olof,
Xml bind yang error still exist. Since yo value depends x, i think x should xml_spec_set with yo, not xrpc in xml_bind_yang_rpc_reply.c:675.

[email protected] /> show version vpp

Breakpoint 1, populate_self_parent (xt=0x83ffa0, xsibling=0x0, xerr=0x7fffffffdb28) at clixon_xml_bind.c:185
185             cprintf(cb, "Failed to find YANG spec of XML node: %s", name);
Missing separate debuginfos, use: debuginfo-install fcgi-2.4.0-25.el7.x86_64 libgcc-4.8.5-44.el7.x86_64 libstdc++-4.8.5-44.el7.x86_64
(gdb) bt
#0  populate_self_parent (xt=0x83ffa0, xsibling=0x0, xerr=0x7fffffffdb28) at clixon_xml_bind.c:185
#1  0x00007ffff7965874 in xml_bind_yang0 (xt=0x83ffa0, yb=YB_PARENT, yspec=0x0, xerr=0x7fffffffdb28) at clixon_xml_bind.c:477
#2  0x00007ffff7965553 in xml_bind_yang (xt=0x834ab0, yb=YB_PARENT, yspec=0x0, xerr=0x7fffffffdb28) at clixon_xml_bind.c:363
#3  0x00007ffff7965e7f in xml_bind_yang_rpc_reply (xrpc=0x834ab0, name=0x915ee0 "version", yspec=0x6503c0, xerr=0x7fffffffdba8) at clixon_xml_bind.c:677
#4  0x00007ffff7983e37 in clicon_rpc_netconf_xml (h=0x607010, xml=0x833e80, xret=0x7fffffffdc90, sp=0x0) at clixon_proto_client.c:370
#5  0x00007ffff7fdb0bc in show_version (h=0x607010, cvv=0x8347f0, argv=0x83f7c0) at /home/dev/code/net-base/management/cli.c:40
#6  0x00007ffff72fa41e in cligen_eval (h=0x60d110, co=0x83cf20, cvv=0x8347f0) at cligen_read.c:861
#7  0x00007ffff7bd2737 in clicon_eval (h=0x607010, cmd=0x60d1e0 "show version vpp", match_obj=0x83cf20, cvv=0x8347f0) at cli_plugin.c:531
#8  0x00007ffff7bd2a9c in clicon_parse (h=0x607010, cmd=0x60d1e0 "show version vpp", modenamep=0x7fffffffde58, result=0x7fffffffde54, evalres=0x0) at cli_plugin.c:632
#9  0x0000000000402dd0 in cli_interactive (h=0x607010) at cli_main.c:231
#10 0x0000000000403f05 in main (argc=0, argv=0x7fffffffe060) at cli_main.c:746
(gdb) p xt[0]
$1 = {
  x_type = CX_ELMNT, 
  x_name = 0x91e790 "version", 
  x_prefix = 0x0, 
  x_flags = 0, 
  x_up = 0x834ab0, 
  _x_vector_i = 2, 
  _x_i = 3, 
  x_value_cb = 0x0, 
  x_childvec = 0x82bf10, 
  x_childvec_len = 5, 
  x_childvec_max = 8, 
  x_ns_cache = 0x60d570, 
  x_spec = 0x0, 
  x_cv = 0x0, 
  x_search_index = 0x0
}
(gdb) p yparent[0]
$4 = {
  ys_len = 4, 
  ys_stmt = 0x73efe0, 
  ys_parent = 0x73e1f0, 
  ys_keyword = Y_OUTPUT, 
  ys_argument = 0x0, 
  ys_flags = 0, 
  ys_mymodule = 0x0, 
  ys_cv = 0x0, 
  ys_cvec = 0x73e420, 
  ys_typecache = 0x0, 
  ys_when_xpath = 0x0, 
  ys_when_nsc = 0x0, 
  _ys_vector_i = 2
}
(gdb) up 3
#3  0x00007ffff7965e7f in xml_bind_yang_rpc_reply (xrpc=0x834ab0, name=0x915ee0 "version", yspec=0x6503c0, xerr=0x7fffffffdba8) at clixon_xml_bind.c:677
677             if ((ret = xml_bind_yang(xrpc, YB_PARENT, NULL, &xerr1)) < 0)
(gdb) p yo[0]
$8 = {
  ys_len = 4, 
  ys_stmt = 0x73efe0, 
  ys_parent = 0x73e1f0, 
  ys_keyword = Y_OUTPUT, 
  ys_argument = 0x0, 
  ys_flags = 0, 
  ys_mymodule = 0x0, 
  ys_cv = 0x0, 
  ys_cvec = 0x73e420, 
  ys_typecache = 0x0, 
  ys_when_xpath = 0x0, 
  ys_when_nsc = 0x0, 
  _ys_vector_i = 2
}
(gdb) p xrpc[0]
$9 = {
  x_type = CX_ELMNT, 
  x_name = 0x902ee0 "rpc-reply", 
  x_prefix = 0x0, 
  x_flags = 0, 
  x_up = 0x83e310, 
  _x_vector_i = 0, 
  _x_i = 0, 
  x_value_cb = 0x0, 
  x_childvec = 0x951380, 
  x_childvec_len = 3, 
  x_childvec_max = 4, 
  x_ns_cache = 0x0, 
  x_spec = 0x73e760, 
  x_cv = 0x0, 
  x_search_index = 0x0
}
(gdb) p x[0]
$10 = {
  x_type = CX_ELMNT, 
  x_name = 0x91e790 "version", 
  x_prefix = 0x0, 
  x_flags = 0, 
  x_up = 0x834ab0, 
  _x_vector_i = 2, 
  _x_i = 3, 
  x_value_cb = 0x0, 
  x_childvec = 0x82bf10, 
  x_childvec_len = 5, 
  x_childvec_max = 8, 
  x_ns_cache = 0x60d570, 
  x_spec = 0x0, 
  x_cv = 0x0, 
  x_search_index = 0x0
}

[nowaits]

int
show_version(clicon_handle h,
...
    /* No need to validate? */
    if (xml_yang_validate_add(h, xrpc_replay, &xerr) < 0 || xerr != NULL) {

validate for this is necessary, clicon_rpc_netconf_xml only do the yang bind work, and never validate the xml.
I have to validate myself. For example, If backend reply without type which is mandatory, clicon_rpc_netconf_xml return ok, and xml_yang_validate_add will return
Feb 18 10:04:45: Get configuration: application missing-element Mandatory variable of version in module unigw <bad-element>type</bad-element>
Maybe we should let clicon_rpc_netconf_xml do validate work for rpc-reply.

    clixon_netconf_error(xerr, "Get configuration", NULL);
    goto done;
}

...
}

[olofhagsand]

Note the rpc-reply should not have the method name, ie, in your code show_vpp_version returns:

   <rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
      <version xmlns="http://www.xdja.com/unigw">
         <type>x</type>
         <build-version>y</build-version>
         <build-directory>z</build-directory>
         <build-date>w</build-date>
      </version>
   </rpc-reply>

but should be:

   <rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
         <type xmlns="http://www.xdja.com/unigw">x</type>
         <build-version xmlns="http://www.xdja.com/unigw">y</build-version>
         <build-directory xmlns="http://www.xdja.com/unigw">z</build-directory>
         <build-date xmlns="http://www.xdja.com/unigw">w</build-date>
   </rpc-reply>

See eg rfc 7950 4.2.9