io/varint.go: binary.Uvarint can return negative lenLen about protobuf HOT 6 CLOSED

From awalterschulze on June 23, 2014 11:10:54

I agree that ReadMsg should return an error given lenLen <= 0, but my first instinct is that, that is all that needs to change, other than also adding a test.

Maybe I just need to see an example of such a sticky error in the go standard library?

Status: Accepted

from protobuf.

From [email protected] on June 23, 2014 12:10:27

For example http://golang.org/src/pkg/bufio/scan.go#L30 The point of the sticky error is to guard against a buggy caller, that might e.g. log the error, but continue the reading loop. For gogoprotobuf/io, that would mean a stream of gibberish messages, with errors interspersed every now and then.

You could say that the caller must stop reading after the first error, and just leave it at that. I'm ok with that. This bug is really about the fact that you can make varintReader.ReadMsg panic with bad input.

from protobuf.

From awalterschulze on June 24, 2014 01:12:09

I fixed the main issue for now https://code.google.com/p/gogoprotobuf/source/detail?r=d228c1a206c3a756d7ec6cc3579d92d00c35a161 We can revisit the sticky error when I come back from holiday

from protobuf.

From awalterschulze on August 04, 2014 08:02:52

I am happy with the non sticky error.
If you are unhappy please reopen the issue.

Thank you for reporting the issue.

from protobuf.

From awalterschulze on August 04, 2014 08:03:10

Status: Fixed

from protobuf.

From [email protected] on August 04, 2014 08:04:29

I'm fine with this.

from protobuf.