WIP: capture the full raw response for Reply/Notificaiton #63

Closed

nemith wants to merge 1 commit from nemith/full_msg into main

nemith commented

2023-08-29 23:14:56 +00:00

(Migrated from github.com)

While working with an internal netconf service I discovered that OKResp and other structs were not being parsed properly. This is because xml (and thus encoding/xml) is document focused. An XML document has single root. This means the "defered" parsing of the <rpc-reply> body doesn't work as you need a proper root node (not just a value) as well you cannot support multiple nodes.

To fix this, begrudgingly, this change changes the API to to not just expose the Body to the call but the entire <rpc-reply> to be parsed. The same goes for <notification>.

This could mean slightly larger allocations, but really.... it doesn't matter. The majority of the data would be inside the Body value anyway so adding the wrapping <rpc-reply> doesn't add much.

I think there is room in a v2 to explore a full stream based solution here, but not needed for this. It does make the full io.Reader/io.Writer of RFC6242 kinda.. worthless now, but perhaps this will make things like netconf over QUIC possible. We will see.

Closes: #62

Breaking changes

Reply.Body and Notification.Body are now gone. In their place there is a Raw() and String() methods on both structs to return the raw data and a string representation.
OkResp was renamed to OKReply
GetConfigReply was reworked to assume as the base.

I think there is future work to make a <rpc-reply> drop in for structs to enforce the root node, but this works for now.

While working with an internal netconf service I discovered that OKResp and other structs were not being parsed properly. This is because xml (and thus `encoding/xml`) is document focused. An XML document has single root. This means the "defered" parsing of the `<rpc-reply>` body doesn't work as you need a proper root node (not just a value) as well you cannot support multiple nodes. To fix this, begrudgingly, this change changes the API to to not just expose the `Body` to the call but the entire `<rpc-reply>` to be parsed. The same goes for `<notification>`. This could mean slightly larger allocations, but really.... it doesn't matter. The majority of the data would be inside the `Body` value anyway so adding the wrapping `<rpc-reply>` doesn't add much. I think there is room in a v2 to explore a full stream based solution here, but not needed for this. It does make the full io.Reader/io.Writer of RFC6242 kinda.. worthless now, but perhaps this will make things like netconf over QUIC possible. We will see. Closes: #62 ## Breaking changes * Reply.Body and Notification.Body are now gone. In their place there is a `Raw()` and `String()` methods on both structs to return the raw data and a string representation. * OkResp was renamed to OKReply * GetConfigReply was reworked to assume <rpc-reply> as the base. I think there is future work to make a `<rpc-reply>` drop in for structs to enforce the root node, but this works for now.

nemith commented

2025-12-30 01:55:21 +00:00

(Migrated from github.com)

I have a "working" streaming version. Streaming isn't important but allocations do and this allocates a lot.