rlp: handle case of normal EOF in Stream.readFull()

[oneeman]

@fjl (since you appear a lot in the rlp git history): This fixes a bug resulting from the combination of three factors:

1. encReader.Read() returning EOF when a successful read (i.e. where len(b) bytes were read) reaches the end. This, in itself, is not a bug, since the docs for io.Reader say returning EOF in such cases is allowed (but not required). See

   go-ethereum/rlp/encode.go

   Lines 271 to 280 in 1489c3f

   	if r.piece = r.next(); r.piece == nil {
   	// Put the encode buffer back into the pool at EOF when it
   	// is first encountered. Subsequent calls still return EOF
   	// as the error but the buffer is no longer valid.
   	if r.buf != nil {
   	encbufPool.Put(r.buf)
   	r.buf = nil
   	}
   	return n, io.EOF
   	}

2. Stream.readFull() giving ErrUnexpectedEOF when it gets an EOF, even if the read was successful (i.e. if len(buff) bytes were read). See

   go-ethereum/rlp/decode.go

   Lines 954 to 956 in 1489c3f

   	if err == io.EOF {
   	err = io.ErrUnexpectedEOF
   	}

3. An inconsistency in bufio.Reader.Read() in whether or not it passes on the error from its underlying reader if the number of bytes read is non-zero. Namely, if the buffer is empty and the amount to read is greater than the buffer size (4096 bytes by default), the resulting EOF will be passed on, but otherwise the read will return nil for its error and the EOF from the underlying reader would only be returned on the next call to .Read(). See https://github.com/golang/go/blob/5faf941df067b33485edb9cd2e880869e7feb6a3/src/bufio/bufio.go#L221 and https://github.com/golang/go/blob/5faf941df067b33485edb9cd2e880869e7feb6a3/src/bufio/bufio.go#L231-L242.

The result of this is that if you encode a large value using rlp.EncodeToReader, you can then get an error leading to failure in decoding. I've added a test in this PR which fails without the fix. Specifically, it fails when the message is 8192 bytes or greater, but passes if it's less (which has to do with the specific reads done during decoding). If you'd rather use a different sort of test, I'd be happy to change it.

The PR fixes this bug by making Stream.readFull() tolerate the EOF error if we read the number of bytes we were supposed to, bringing its behavior more in line with what is done in io.ReadFull() (https://github.com/golang/go/blob/5faf941df067b33485edb9cd2e880869e7feb6a3/src/io/io.go#L314-L348).

A different way to fix this would have been to modify encReader() to not return EOF in such cases.

[holiman]

LGTM, but I'll let Felix take care of this one

[fjl]

Very nice catch, thanks!