Juniper / go-netconf

NETCONF implementation in Go.
Other
253 stars 110 forks source link

Unexpected extra data in XML from devices #103

Open waldner opened 3 years ago

waldner commented 3 years ago

This happens using the master branch.

Running the following code (connection is to a Cisco device, if that matters):

reply, _ := s.Exec(netconf.MethodGetConfig("running"))
fmt.Println("REPLY:", string(reply.RawReply))

I get the following output (excerpts):

REPLY: 
#4104
<?xml version="1.0" encoding="UTF-8"?>
<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="192cfbdd-fc16-4a3c-b496-9a3e9475ef3e"><data>
...
<last>4</last><login><local/></login><transport><input><input>ssh</input></input>
#4102
</transport></vty></line>
...
#4083
<protocol xmlns:oc-pol-types="http://openconfig.net/yang/policy-types">oc-pol-types:STATIC</protocol>
...
</rpc-reply>

I don't understand what those #4104, #4102, #4083 etc. mean.

Inspecting the Exec() method of the session reveals that after the following line: https://github.com/Juniper/go-netconf/blob/a527e68d123f2daae822e61380be51349643f754/netconf/session.go#L48

rawXML already contains the spurious numbers.

Using the stable version (0.1.1), the one you get by default when importing github.com/Juniper/go-netconf/netconf, the output is clean.

nights99 commented 3 years ago

FWIW, I've also seen this, this is due to Netconf 1.1 'chunking' - these numbers are the number of bytes in the chunk.

At the moment I'm working around this by stripping the numbers out after receiving them, but I'd like to do a proper fix at some point. I wonder if the intention was to use the WaitForRegexp transport function for this - that was added but isn't yet used anywhere.

nemith commented 3 years ago

If you know the size of the chunk using Regexp would be the most inefficient way to handle it :)

So chunked transport isn't supported and this library probably needs to be reworked to handle it.

nights99 commented 3 years ago

Yeah, agreed, having thought more, regex is definitely not the right approach.

I have a prototype that mostly works now reading the chunk sizes - but it doesn't really fit nicely with how the code is currently structured. I'll have a bit more of a think about how to do this in a way that is a bit more structured.

nights99 commented 3 years ago

I have ugly but working changes for this on my fork at https://github.com/nights99/go-netconf/tree/chunk_read

The error handling definitely needs improving, but if anyone has suggestions for a nicer structure I should have some more time to work on it again soon.

nemith commented 2 years ago

chunking is in the latest release (however i haven't tested it)

I am working on a rewrite where chunking will be a part of this (tracking in #24). Closing this for now. Please feel free to comment if the new version (v0.3.0) isn't working for you.

nights99 commented 2 years ago

Hi, I think the issue here is still present even in the latest code - although the code supports sending the chunk size, it isn't parsing out the chunk sizes from received messages, leaving them in the xml. Looks like @GiacomoCortesi's fork has a possible solution for this.

Thanks, Jon

nemith commented 2 years ago

Good to know. Open for a fix for v1.

I am working on v2 now and will do stream based w/ proper parsing: https://github.com/nemith/go-netconf/blob/v2/transport/frame.go#L100

GiacomoCortesi commented 2 years ago

Yes, my fork includes some changes for chunked framing support, but credits go to the author of https://github.com/andaru/netconf. I just took a couple of functions from there and made the adaptations needed to make it work. Andaru repo is very well done, unfortunately it looks like he left it unfinished and stopped development on it.

GiacomoCortesi commented 2 years ago

fun fact, someone took part of the code from my fork for chunked framing support and put it here: https://github.com/openshift-telco/go-netconf-client

nemith commented 2 years ago

I'll approve any PR for fixes but my focus will be on the rewrite (which already support chucked reads and writes)