scrapli / scrapligo

scrapli, but in go!
MIT License
244 stars 35 forks source link

Fix netconf 1.1 chunk parse #181

Closed netixx closed 2 months ago

netixx commented 2 months ago

I stumbled upon an issue with the current netconf 1.1 parser.

My payload contained ######## characters on a single line, which matched the pattern defined in the regex erroneously.

I added a test for it, and proposed a new implementation of the chunk matching mechanism using a basic for loop.

Since the size of the chunk is known in advance, we only need to detect the chunk marker, and consume all data up to the length of the chunk and repeat.

As a bonus, we don't need to do expensive regex on very large payloads, so we should achieve faster speeds and less memory usage.

carlmontanari commented 2 months ago

ah very nice. I have some fear about this due to the newline shenangengins that it looks like you ran into as well :) will take a closer peak this weekend to test it out!

carlmontanari commented 2 months ago

went through all that to make sure I understood it all! very nice solution @netixx ! I made some minor changes -- mostly just tweaking things so it fit my brain a bit better.

only meaningful change it think is that that the max chunk length check. I think we only need to check chunk lengths for up to 10 chars (that giant number from the rfc but the actual char positions in that number) -- so I changed things a bit to only check up to 10 chars. if the chunkSizeStr is empty we know we didn't find our new line and can bail out.

let me know if you're ok with these tweaks and then we can get it merged and cut another minor release!

netixx commented 2 months ago

It's all good to me, the way I was calculating the max chunk length was a bit silly indeed :) I could test that it still works fine on my subset of devices so it ll good to go on my side!