Skip to content

Conversation

@nemith
Copy link
Owner

@nemith nemith commented Aug 29, 2023

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.

@nemith nemith added the breaking-change For PRs that bust the API. To be used for cutting releases label Aug 29, 2023
@nemith nemith force-pushed the nemith/full_msg branch 4 times, most recently from ab4ae13 to 7d95be1 Compare August 31, 2023 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change For PRs that bust the API. To be used for cutting releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reply deferred Body parsing doesn't work with multiple elements under the root.

2 participants