Unmarshal(): Reader is modified but access to bufio.Reader is lost

[LaurentLousky]

I noticed that bencode.Unmarshal() creates a bufio.Reader from the passed-in reader. After bencode.Unmarshal() returns, the passed-in reader is left modified with the new index that the bufio.reader used to fill its buffer. This leaves the caller of bencode.Unmarshal() with a reader that is likely to be at the incorrect index from where the bufio.Reader left off.

Some solutions I can think of off the top of my head:

• Force bencode.Unmarshal() to take in a bufio.Reader not just any io.reader
• Return the bufio.Reader that was created from the passed-in reader (from this point on, you must use only the buffered reader to do all the reads from the underlying reader)

[jackpal]

Ugg, you're right. Thanks for the bug report.

I think the best thing to do is change the Unmarshal() API to take a bufio.Reader.

That would be a breaking API change, even though it is also a bugfix.

I just now tagged the existing API as v1.0.0

Would you like to propose a patch that updates the Unmarshal() API to take a bufio.Reader?

I think you'd need to update the tests as well.

This new code would be v2.0.0

[jackpal]

I guess another approach would be to have a whole separate (probably slower) code path for io.Readers that didn't read ahead more than the minimum necessary to decode.

The advantage of that approach is that it doesn't change the current API at all.

[LaurentLousky]

Hmm yeah breaking the API would be annoying. Do you think there's a way to simply store the passed-in reader's initial index and then use that at the end of Unmarshal() to calculate the passed-in reader initial index + bufio.reader index, then set that value to the reader using Seek()? Kind of hacky, but could be a simple solution

[jackpal]

That's a good idea, it would fix the issue when the reader was an io.ReadSeeker. But it doesn't fix the issue for clients that don't implement io.Seeker.

For a pure io.Reader we'd probably need to bring back the old way of parsing, that worked purely on the io.Reader interface. Maybe that wouldn't be too difficult.