Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPC output not verified by yang #283

Closed
nowaits opened this issue Oct 31, 2021 · 3 comments
Closed

RPC output not verified by yang #283

nowaits opened this issue Oct 31, 2021 · 3 comments

Comments

@nowaits
Copy link
Contributor

nowaits commented Oct 31, 2021

Hi olof,
Currently the netconf RPC output not verified by yang, my test case is below:
RPC define in yang:

    rpc demo {
        output {
            container result {
                leaf v0 {
                    type uint32;
                    mandatory true;
                }
                leaf v1 {
                    type uint32 {
                        range "1..1024";
                    }
                }
            }
        }
    }

code of C callback

int demo(clicon_handle h,            /* Clicon handle */
    cxobj         *xe,          /* Request: <rpc><xe></rpc> */
    cbuf          *cbret,       /* Reply eg <rpc-reply>... */
    void         *arg,          /* client_entry */
    void         *regarg)       /* Argument given at register */
{
    cprintf(cbret,
        "<rpc-reply xmlns=\"%s\" message-id=\"%s\"> \
        <result xmlns=\"http://www.xdja.com/unigw\"> \
            <v1>0</v1> \
        </result> \
        </rpc-reply>",
        NETCONF_BASE_NAMESPACE,
        xml_find_value(xml_parent(xe), "message-id"));

    return 0;
}

RPC calling with restconf and the output is

[dev@localhost net-base]$ curl -X POST -H "Content-Type: application/yang-data+json"  http://localhost/restconf/operations/unigw:demo
{
  "unigw:output": {
    "result": {
      "v1": 0
    }
  }
}

log in backend:

Oct 31 22:00:31: from_client
Oct 31 22:00:31: from_client_msg
Oct 31 22:00:31: clicon_msg_decode <hello username="anonymous" xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="42"><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>
Oct 31 22:00:31: from_client_msg cbret:<hello xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="42"><session-id>1</session-id></hello>
Oct 31 22:00:31: from_client_msg retval:0
Oct 31 22:00:31: from_client retval=0
Oct 31 22:00:31: from_client
Oct 31 22:00:31: from_client_msg
Oct 31 22:00:31: clicon_msg_decode <rpc xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" username="anonymous" message-id="42"><demo xmlns="http://www.xdja.com/unigw"/></rpc>
Oct 31 22:00:31: from_client_msg module:unigw-add-ons rpc:demo
Oct 31 22:00:31: rpc_callback_call retval:1
Oct 31 22:00:31: from_client_msg cbret:<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="42">         <result xmlns="http://www.xdja.com/unigw">             <v1>0</v1>         </result>         </rpc-reply>
Oct 31 22:00:31: from_client_msg retval:0
Oct 31 22:00:31: from_client retval=0

Backend should check the mandatory value result/v0 and the range of result/v1, but both not checked.

@olofhagsand
Copy link
Member

There is code for checking RPC replies but for various reasons it is not enabled.
They are somewhat special in the sense the errors are generated by the server itself, such as plugin code, not the client message. In those parts where it is enabled it has the tag: "internal error", with the actual error in the message.
In a few instances I noted they reduced performance such as CLICON_VALIDATE_STATE_XML in which case it is recommended to enable it during development only.

@olofhagsand
Copy link
Member

I guess I am saying that I support this enhancement, just wanted to give a background/description.

olofhagsand added a commit that referenced this issue Nov 16, 2021
  * Stricter checking of outgoing RPC replies from server
  * See [RPC output not verified by yang](#283)
  * This lead to some corrections of RPC replies in system code
@olofhagsand
Copy link
Member

Fixed, please verify the functionality

@nowaits nowaits closed this as completed Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants