From c35f4fd0bd93bcab01ba7704fc144514a2cc7a1b Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 23 Mar 2015 15:02:55 +0100 Subject: [PATCH 01/13] rlp: check top-level value sizes against input limit This is a preliminary fix for #420 (SEC-18 RLP decoder unsafe allocation). If a sane input limit is set on the rlp.Stream, it should no longer be possible to cause huge []byte allocations. --- rlp/decode.go | 109 ++++++++++++++++++++++++++++++++++------- rlp/decode_test.go | 118 ++++++++++++++++++++++++++++++--------------- 2 files changed, 171 insertions(+), 56 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 3b5617475b..ca9252575b 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -9,6 +9,7 @@ import ( "io" "math/big" "reflect" + "strings" ) var ( @@ -70,14 +71,22 @@ type Decoder interface { // Non-empty interface types are not supported, nor are booleans, // signed integers, floating point numbers, maps, channels and // functions. +// +// Note that Decode does not set an input limit for all readers +// and may be vulnerable to panics cause by huge value sizes. If +// you need an input limit, use +// +// NewStream(r, limit).Decode(val) func Decode(r io.Reader, val interface{}) error { - return NewStream(r).Decode(val) + // TODO: this could use a Stream from a pool. + return NewStream(r, 0).Decode(val) } // DecodeBytes parses RLP data from b into val. // Please see the documentation of Decode for the decoding rules. func DecodeBytes(b []byte, val interface{}) error { - return NewStream(bytes.NewReader(b)).Decode(val) + // TODO: this could use a Stream from a pool. + return NewStream(bytes.NewReader(b), uint64(len(b))).Decode(val) } type decodeError struct { @@ -470,10 +479,12 @@ var ( ErrExpectedList = errors.New("rlp: expected List") ErrCanonInt = errors.New("rlp: expected Int") ErrElemTooLarge = errors.New("rlp: element is larger than containing list") + ErrValueTooLarge = errors.New("rlp: value size exceeds available input length") // internal errors - errNotInList = errors.New("rlp: call of ListEnd outside of any list") - errNotAtEOL = errors.New("rlp: call of ListEnd not positioned at EOL") + errNotInList = errors.New("rlp: call of ListEnd outside of any list") + errNotAtEOL = errors.New("rlp: call of ListEnd not positioned at EOL") + errUintOverflow = errors.New("rlp: uint overflow") ) // ByteReader must be implemented by any input reader for a Stream. It @@ -496,7 +507,13 @@ type ByteReader interface { // // Stream is not safe for concurrent use. type Stream struct { - r ByteReader + r ByteReader + + // number of bytes remaining to be read from r. + remaining uint64 + limited bool + + // auxiliary buffer for integer decoding uintbuf []byte kind Kind // kind of value ahead @@ -507,12 +524,26 @@ type Stream struct { type listpos struct{ pos, size uint64 } -// NewStream creates a new stream reading from r. -// If r does not implement ByteReader, the Stream will -// introduce its own buffering. -func NewStream(r io.Reader) *Stream { +// NewStream creates a new decoding stream reading from r. +// +// If r implements the ByteReader interface, Stream will +// not introduce any buffering. +// +// For non-toplevel values, Stream returns ErrElemTooLarge +// for values that do not fit into the enclosing list. +// +// Stream supports an optional input limit. If a limit is set, the +// size of any toplevel value will be checked against the remaining +// input length. Stream operations that encounter a value exceeding +// the remaining input length will return ErrValueTooLarge. The limit +// can be set by passing a non-zero value for inputLimit. +// +// If r is a bytes.Reader or strings.Reader, the input limit is set to +// the length of r's underlying data unless an explicit limit is +// provided. +func NewStream(r io.Reader, inputLimit uint64) *Stream { s := new(Stream) - s.Reset(r) + s.Reset(r, inputLimit) return s } @@ -520,7 +551,7 @@ func NewStream(r io.Reader) *Stream { // at an encoded list of the given length. func NewListStream(r io.Reader, len uint64) *Stream { s := new(Stream) - s.Reset(r) + s.Reset(r, len) s.kind = List s.size = len return s @@ -574,8 +605,6 @@ func (s *Stream) Raw() ([]byte, error) { return buf, nil } -var errUintOverflow = errors.New("rlp: uint overflow") - // Uint reads an RLP string of up to 8 bytes and returns its contents // as an unsigned integer. If the input does not contain an RLP string, the // returned error will be ErrExpectedString. @@ -667,14 +696,36 @@ func (s *Stream) Decode(val interface{}) error { } // Reset discards any information about the current decoding context -// and starts reading from r. If r does not also implement ByteReader, -// Stream will do its own buffering. -func (s *Stream) Reset(r io.Reader) { +// and starts reading from r. This method is meant to facilitate reuse +// of a preallocated Stream across many decoding operations. +// +// If r does not also implement ByteReader, Stream will do its own +// buffering. +func (s *Stream) Reset(r io.Reader, inputLimit uint64) { + if inputLimit > 0 { + s.remaining = inputLimit + s.limited = true + } else { + // Attempt to automatically discover + // the limit when reading from a byte slice. + switch br := r.(type) { + case *bytes.Reader: + s.remaining = uint64(br.Len()) + s.limited = true + case *strings.Reader: + s.remaining = uint64(br.Len()) + s.limited = true + default: + s.limited = false + } + } + // Wrap r with a buffer if it doesn't have one. bufr, ok := r.(ByteReader) if !ok { bufr = bufio.NewReader(r) } s.r = bufr + // Reset the decoding context. s.stack = s.stack[:0] s.size = 0 s.kind = -1 @@ -700,6 +751,8 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) { tos = &s.stack[len(s.stack)-1] } if s.kind < 0 { + // don't read further if we're at the end of the + // innermost list. if tos != nil && tos.pos == tos.size { return 0, 0, EOL } @@ -709,8 +762,19 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) { } s.kind, s.size = kind, size } - if tos != nil && tos.pos+s.size > tos.size { - return 0, 0, ErrElemTooLarge + // Make sure size is reasonable. This is done always + // so Kind returns the same error when called multiple times. + if tos == nil { + // At toplevel, check that the value is smaller + // than the remaining input length. + if s.limited && s.size > s.remaining { + return 0, 0, ErrValueTooLarge + } + } else { + // Inside a list, check that the value doesn't overflow the list. + if tos.pos+s.size > tos.size { + return 0, 0, ErrElemTooLarge + } } return s.kind, s.size, nil } @@ -778,6 +842,9 @@ func (s *Stream) readUint(size byte) (uint64, error) { } func (s *Stream) readFull(buf []byte) (err error) { + if s.limited && s.remaining < uint64(len(buf)) { + return ErrValueTooLarge + } s.willRead(uint64(len(buf))) var nn, n int for n < len(buf) && err == nil { @@ -791,6 +858,9 @@ func (s *Stream) readFull(buf []byte) (err error) { } func (s *Stream) readByte() (byte, error) { + if s.limited && s.remaining == 0 { + return 0, io.EOF + } s.willRead(1) b, err := s.r.ReadByte() if len(s.stack) > 0 && err == io.EOF { @@ -801,6 +871,9 @@ func (s *Stream) readByte() (byte, error) { func (s *Stream) willRead(n uint64) { s.kind = -1 // rearm Kind + if s.limited { + s.remaining -= n + } if len(s.stack) > 0 { s.stack[len(s.stack)-1].pos += n } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 73a31c67f5..6b37ab0ad7 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -36,7 +36,8 @@ func TestStreamKind(t *testing.T) { } for i, test := range tests { - s := NewStream(bytes.NewReader(unhex(test.input))) + // using plainReader to inhibit input limit errors. + s := NewStream(newPlainReader(unhex(test.input)), 0) kind, len, err := s.Kind() if err != nil { t.Errorf("test %d: Kind returned error: %v", i, err) @@ -70,29 +71,63 @@ func TestNewListStream(t *testing.T) { } func TestStreamErrors(t *testing.T) { + withoutInputLimit := func(b []byte) *Stream { + return NewStream(newPlainReader(b), 0) + } + withCustomInputLimit := func(limit uint64) func([]byte) *Stream { + return func(b []byte) *Stream { + return NewStream(bytes.NewReader(b), limit) + } + } + type calls []string tests := []struct { string calls + newStream func([]byte) *Stream // uses bytes.Reader if nil error }{ - {"", calls{"Kind"}, io.EOF}, - {"", calls{"List"}, io.EOF}, - {"", calls{"Uint"}, io.EOF}, - {"C0", calls{"Bytes"}, ErrExpectedString}, - {"C0", calls{"Uint"}, ErrExpectedString}, - {"81", calls{"Bytes"}, io.ErrUnexpectedEOF}, - {"81", calls{"Uint"}, io.ErrUnexpectedEOF}, - {"BFFFFFFFFFFFFFFF", calls{"Bytes"}, io.ErrUnexpectedEOF}, - {"89000000000000000001", calls{"Uint"}, errUintOverflow}, - {"00", calls{"List"}, ErrExpectedList}, - {"80", calls{"List"}, ErrExpectedList}, - {"C0", calls{"List", "Uint"}, EOL}, - {"C801", calls{"List", "Uint", "Uint"}, io.ErrUnexpectedEOF}, - {"C8C9", calls{"List", "Kind"}, ErrElemTooLarge}, - {"C3C2010201", calls{"List", "List", "Uint", "Uint", "ListEnd", "Uint"}, EOL}, - {"00", calls{"ListEnd"}, errNotInList}, - {"C40102", calls{"List", "Uint", "ListEnd"}, errNotAtEOL}, + {"C0", calls{"Bytes"}, nil, ErrExpectedString}, + {"C0", calls{"Uint"}, nil, ErrExpectedString}, + {"89000000000000000001", calls{"Uint"}, nil, errUintOverflow}, + {"00", calls{"List"}, nil, ErrExpectedList}, + {"80", calls{"List"}, nil, ErrExpectedList}, + {"C0", calls{"List", "Uint"}, nil, EOL}, + {"C8C9010101010101010101", calls{"List", "Kind"}, nil, ErrElemTooLarge}, + {"C3C2010201", calls{"List", "List", "Uint", "Uint", "ListEnd", "Uint"}, nil, EOL}, + {"00", calls{"ListEnd"}, nil, errNotInList}, + {"C401020304", calls{"List", "Uint", "ListEnd"}, nil, errNotAtEOL}, + + // Expected EOF + {"", calls{"Kind"}, nil, io.EOF}, + {"", calls{"Uint"}, nil, io.EOF}, + {"", calls{"List"}, nil, io.EOF}, + {"8105", calls{"Uint", "Uint"}, nil, io.EOF}, + {"C0", calls{"List", "ListEnd", "List"}, nil, io.EOF}, + + // Input limit errors. + {"81", calls{"Bytes"}, nil, ErrValueTooLarge}, + {"81", calls{"Uint"}, nil, ErrValueTooLarge}, + {"81", calls{"Raw"}, nil, ErrValueTooLarge}, + {"BFFFFFFFFFFFFFFFFFFF", calls{"Bytes"}, nil, ErrValueTooLarge}, + {"C801", calls{"List"}, nil, ErrValueTooLarge}, + + // Test for input limit overflow. Since we are counting the limit + // down toward zero in Stream.remaining, reading too far can overflow + // remaining to a large value, effectively disabling the limit. + {"C40102030401", calls{"Raw", "Uint"}, withCustomInputLimit(5), io.EOF}, + {"C4010203048102", calls{"Raw", "Uint"}, withCustomInputLimit(6), ErrValueTooLarge}, + + // Check that the same calls are fine without a limit. + {"C40102030401", calls{"Raw", "Uint"}, withoutInputLimit, nil}, + {"C4010203048102", calls{"Raw", "Uint"}, withoutInputLimit, nil}, + + // Unexpected EOF. This only happens when there is + // no input limit, so the reader needs to be 'dumbed down'. + {"81", calls{"Bytes"}, withoutInputLimit, io.ErrUnexpectedEOF}, + {"81", calls{"Uint"}, withoutInputLimit, io.ErrUnexpectedEOF}, + {"BFFFFFFFFFFFFFFF", calls{"Bytes"}, withoutInputLimit, io.ErrUnexpectedEOF}, + {"C801", calls{"List", "Uint", "Uint"}, withoutInputLimit, io.ErrUnexpectedEOF}, // This test verifies that the input position is advanced // correctly when calling Bytes for empty strings. Kind can be called @@ -109,12 +144,15 @@ func TestStreamErrors(t *testing.T) { "Bytes", // past final element "Bytes", // this one should fail - }, EOL}, + }, nil, EOL}, } testfor: for i, test := range tests { - s := NewStream(bytes.NewReader(unhex(test.string))) + if test.newStream == nil { + test.newStream = func(b []byte) *Stream { return NewStream(bytes.NewReader(b), 0) } + } + s := test.newStream(unhex(test.string)) rs := reflect.ValueOf(s) for j, call := range test.calls { fval := rs.MethodByName(call) @@ -124,8 +162,12 @@ testfor: err = lastret.(error).Error() } if j == len(test.calls)-1 { - if err != test.error.Error() { - t.Errorf("test %d: last call (%s) error mismatch\ngot: %s\nwant: %v", + want := "" + if test.error != nil { + want = test.error.Error() + } + if err != want { + t.Errorf("test %d: last call (%s) error mismatch\ngot: %s\nwant: %s", i, call, err, test.error) } } else if err != "" { @@ -137,7 +179,7 @@ testfor: } func TestStreamList(t *testing.T) { - s := NewStream(bytes.NewReader(unhex("C80102030405060708"))) + s := NewStream(bytes.NewReader(unhex("C80102030405060708")), 0) len, err := s.List() if err != nil { @@ -166,7 +208,7 @@ func TestStreamList(t *testing.T) { } func TestStreamRaw(t *testing.T) { - s := NewStream(bytes.NewReader(unhex("C58401010101"))) + s := NewStream(bytes.NewReader(unhex("C58401010101")), 0) s.List() want := unhex("8401010101") @@ -284,11 +326,6 @@ var decodeTests = []decodeTest{ ptr: new([5]byte), error: "rlp: input string too long for [5]uint8", }, - { - input: "850101", - ptr: new([5]byte), - error: io.ErrUnexpectedEOF.Error(), - }, // byte array reuse (should be zeroed) {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, @@ -401,11 +438,17 @@ func TestDecodeWithByteReader(t *testing.T) { }) } -// dumbReader reads from a byte slice but does not -// implement ReadByte. -type dumbReader []byte +// plainReader reads from a byte slice but does not +// implement ReadByte. It is also not recognized by the +// size validation. This is useful to test how the decoder +// behaves on a non-buffered input stream. +type plainReader []byte -func (r *dumbReader) Read(buf []byte) (n int, err error) { +func newPlainReader(b []byte) io.Reader { + return (*plainReader)(&b) +} + +func (r *plainReader) Read(buf []byte) (n int, err error) { if len(*r) == 0 { return 0, io.EOF } @@ -416,15 +459,14 @@ func (r *dumbReader) Read(buf []byte) (n int, err error) { func TestDecodeWithNonByteReader(t *testing.T) { runTests(t, func(input []byte, into interface{}) error { - r := dumbReader(input) - return Decode(&r, into) + return Decode(newPlainReader(input), into) }) } func TestDecodeStreamReset(t *testing.T) { - s := NewStream(nil) + s := NewStream(nil, 0) runTests(t, func(input []byte, into interface{}) error { - s.Reset(bytes.NewReader(input)) + s.Reset(bytes.NewReader(input), 0) return s.Decode(into) }) } @@ -518,7 +560,7 @@ func ExampleDecode() { func ExampleStream() { input, _ := hex.DecodeString("C90A1486666F6F626172") - s := NewStream(bytes.NewReader(input)) + s := NewStream(bytes.NewReader(input), 0) // Check what kind of value lies ahead kind, size, _ := s.Kind() From 56a48101dc3dd96587915a5d7882f9d46ecc6ae9 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Mon, 23 Mar 2015 15:08:29 +0100 Subject: [PATCH 02/13] cmd/rlpdump, cmd/utils, eth, p2p, whisper: use rlp input limit --- cmd/rlpdump/main.go | 2 +- cmd/utils/cmd.go | 2 +- eth/protocol.go | 6 +++--- p2p/message.go | 3 ++- whisper/peer.go | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cmd/rlpdump/main.go b/cmd/rlpdump/main.go index 8567dcff83..528ccc6bd6 100644 --- a/cmd/rlpdump/main.go +++ b/cmd/rlpdump/main.go @@ -78,7 +78,7 @@ func main() { os.Exit(2) } - s := rlp.NewStream(r) + s := rlp.NewStream(r, 0) for { if err := dump(s, 0); err != nil { if err != io.EOF { diff --git a/cmd/utils/cmd.go b/cmd/utils/cmd.go index 7286f5c5e4..64faf6ad1b 100644 --- a/cmd/utils/cmd.go +++ b/cmd/utils/cmd.go @@ -154,7 +154,7 @@ func ImportChain(chainmgr *core.ChainManager, fn string) error { defer fh.Close() chainmgr.Reset() - stream := rlp.NewStream(fh) + stream := rlp.NewStream(fh, 0) var i, n int batchSize := 2500 diff --git a/eth/protocol.go b/eth/protocol.go index 1a19307dba..6b566f31b8 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -210,7 +210,7 @@ func (self *ethProtocol) handle() error { return p2p.Send(self.rw, BlockHashesMsg, hashes) case BlockHashesMsg: - msgStream := rlp.NewStream(msg.Payload) + msgStream := rlp.NewStream(msg.Payload, uint64(msg.Size)) if _, err := msgStream.List(); err != nil { return err } @@ -231,7 +231,7 @@ func (self *ethProtocol) handle() error { self.blockPool.AddBlockHashes(iter, self.id) case GetBlocksMsg: - msgStream := rlp.NewStream(msg.Payload) + msgStream := rlp.NewStream(msg.Payload, uint64(msg.Size)) if _, err := msgStream.List(); err != nil { return err } @@ -259,7 +259,7 @@ func (self *ethProtocol) handle() error { return p2p.Send(self.rw, BlocksMsg, blocks) case BlocksMsg: - msgStream := rlp.NewStream(msg.Payload) + msgStream := rlp.NewStream(msg.Payload, uint64(msg.Size)) if _, err := msgStream.List(); err != nil { return err } diff --git a/p2p/message.go b/p2p/message.go index b42acbe3cf..be6405d6f0 100644 --- a/p2p/message.go +++ b/p2p/message.go @@ -32,7 +32,8 @@ type Msg struct { // // For the decoding rules, please see package rlp. func (msg Msg) Decode(val interface{}) error { - if err := rlp.Decode(msg.Payload, val); err != nil { + s := rlp.NewStream(msg.Payload, uint64(msg.Size)) + if err := s.Decode(val); err != nil { return newPeerError(errInvalidMsg, "(code %x) (size %d) %v", msg.Code, msg.Size, err) } return nil diff --git a/whisper/peer.go b/whisper/peer.go index e4301f37c3..28abf42605 100644 --- a/whisper/peer.go +++ b/whisper/peer.go @@ -66,7 +66,7 @@ func (self *peer) handshake() error { if packet.Code != statusCode { return fmt.Errorf("peer sent %x before status packet", packet.Code) } - s := rlp.NewStream(packet.Payload) + s := rlp.NewStream(packet.Payload, uint64(packet.Size)) if _, err := s.List(); err != nil { return fmt.Errorf("bad status message: %v", err) } From 2750ec47b7e7ff864eaed72255581e11080907d7 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Apr 2015 00:54:12 +0200 Subject: [PATCH 03/13] rlp: fix integer overflow in list element size validation It is not safe to add anything to s.size. --- rlp/decode.go | 4 ++-- rlp/decode_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index ca9252575b..1e39054e64 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -751,7 +751,7 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) { tos = &s.stack[len(s.stack)-1] } if s.kind < 0 { - // don't read further if we're at the end of the + // Don't read further if we're at the end of the // innermost list. if tos != nil && tos.pos == tos.size { return 0, 0, EOL @@ -772,7 +772,7 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) { } } else { // Inside a list, check that the value doesn't overflow the list. - if tos.pos+s.size > tos.size { + if s.size > tos.size-tos.pos { return 0, 0, ErrElemTooLarge } } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 6b37ab0ad7..a64bfe3fdb 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -112,6 +112,9 @@ func TestStreamErrors(t *testing.T) { {"BFFFFFFFFFFFFFFFFFFF", calls{"Bytes"}, nil, ErrValueTooLarge}, {"C801", calls{"List"}, nil, ErrValueTooLarge}, + // Test for list element size check overflow. + {"CD04040404FFFFFFFFFFFFFFFFFF0303", calls{"List", "Uint", "Uint", "Uint", "Uint", "List"}, nil, ErrElemTooLarge}, + // Test for input limit overflow. Since we are counting the limit // down toward zero in Stream.remaining, reading too far can overflow // remaining to a large value, effectively disabling the limit. From eedbb1ee9a2164cd58e9fd305bc719a4c643f1a2 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Apr 2015 12:02:23 +0200 Subject: [PATCH 04/13] p2p/discover: use rlp.DecodeBytes --- p2p/discover/udp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index 61a0abed99..07a1a739ce 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -413,7 +413,7 @@ func decodePacket(buf []byte) (packet, NodeID, []byte, error) { default: return nil, fromID, hash, fmt.Errorf("unknown type: %d", ptype) } - err = rlp.Decode(bytes.NewReader(sigdata[1:]), req) + err = rlp.DecodeBytes(sigdata[1:], req) return req, fromID, hash, err } From 509d0a8d78236562d9444a6fe851aec3cee5bb5e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Apr 2015 12:05:36 +0200 Subject: [PATCH 05/13] whisper: fix comment for rlpenv --- whisper/envelope.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/whisper/envelope.go b/whisper/envelope.go index 0a817e26ef..07762c3004 100644 --- a/whisper/envelope.go +++ b/whisper/envelope.go @@ -109,16 +109,17 @@ func (self *Envelope) Hash() common.Hash { return self.hash } -// rlpenv is an Envelope but is not an rlp.Decoder. -// It is used for decoding because we need to -type rlpenv Envelope - // DecodeRLP decodes an Envelope from an RLP data stream. func (self *Envelope) DecodeRLP(s *rlp.Stream) error { raw, err := s.Raw() if err != nil { return err } + // The decoding of Envelope uses the struct fields but also needs + // to compute the hash of the whole RLP-encoded envelope. This + // type has the same structure as Envelope but is not an + // rlp.Decoder so we can reuse the Envelope struct definition. + type rlpenv Envelope if err := rlp.DecodeBytes(raw, (*rlpenv)(self)); err != nil { return err } From 6788f955c2414b025a4ea44efaf51caf50aa97f0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Apr 2015 12:28:19 +0200 Subject: [PATCH 06/13] rlp: fix handling of single byte zero when decoding into a pointer A single zero byte carries information and should not set the pointer to nil. This is arguably a corner case. While here, fix the comment to explain pointer reuse. --- rlp/decode.go | 10 +++++----- rlp/decode_test.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 1e39054e64..42be31a2d0 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -37,9 +37,9 @@ type Decoder interface { // DecodeRLP. // // To decode into a pointer, Decode will set the pointer to nil if the -// input has size zero or the input is a single byte with value zero. -// If the input has nonzero size, Decode will allocate a new value of -// the type being pointed to. +// input has size zero. If the input has nonzero size, Decode will +// parse the input data into a value of the type being pointed to. +// If the pointer is non-nil, the existing value will reused. // // To decode into a struct, Decode expects the input to be an RLP // list. The decoded elements of the list are assigned to each public @@ -382,8 +382,8 @@ func makePtrDecoder(typ reflect.Type) (decoder, error) { return nil, err } dec := func(s *Stream, val reflect.Value) (err error) { - _, size, err := s.Kind() - if err != nil || size == 0 && s.byteval == 0 { + kind, size, err := s.Kind() + if err != nil || size == 0 && kind != Byte { // rearm s.Kind. This is important because the input // position must advance to the next value even though // we don't read anything. diff --git a/rlp/decode_test.go b/rlp/decode_test.go index a64bfe3fdb..e5c7f3761f 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -378,7 +378,7 @@ var decodeTests = []decodeTest{ }, // pointers - {input: "00", ptr: new(*uint), value: (*uint)(nil)}, + {input: "00", ptr: new(*uint), value: uintp(0)}, {input: "80", ptr: new(*uint), value: (*uint)(nil)}, {input: "C0", ptr: new(*uint), value: (*uint)(nil)}, {input: "07", ptr: new(*uint), value: uintp(7)}, From 6e9f8035a1a49ac096bc2eae6ec4637b48e29048 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 14 Apr 2015 16:09:06 +0200 Subject: [PATCH 07/13] rlp: stricter validation of canonical integer format All integers (including size information in type tags) need to be encoded using the smallest possible encoding. This commit expands the stricter validation introduced for *big.Int in commit 59597d23a5ee268 to all integer types and size tags. --- rlp/decode.go | 97 ++++++++++++++++++++++++++++++---------------- rlp/decode_test.go | 38 ++++++++++++++---- 2 files changed, 93 insertions(+), 42 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 42be31a2d0..6bd13aa8fb 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -109,7 +109,9 @@ func (err *decodeError) Error() string { func wrapStreamError(err error, typ reflect.Type) error { switch err { case ErrCanonInt: - return &decodeError{msg: "canon int error appends zero's", typ: typ} + return &decodeError{msg: "non-canonical integer (leading zero bytes)", typ: typ} + case ErrCanonSize: + return &decodeError{msg: "non-canonical size information", typ: typ} case ErrExpectedList: return &decodeError{msg: "expected input list", typ: typ} case ErrExpectedString: @@ -195,12 +197,10 @@ func decodeBigInt(s *Stream, val reflect.Value) error { i = new(big.Int) val.Set(reflect.ValueOf(i)) } - - // Reject big integers which are zero appended + // Reject leading zero bytes if len(b) > 0 && b[0] == 0 { return wrapStreamError(ErrCanonInt, val.Type()) } - i.SetBytes(b) return nil } @@ -270,7 +270,7 @@ func decodeListSlice(s *Stream, val reflect.Value, elemdec decoder) error { func decodeListArray(s *Stream, val reflect.Value, elemdec decoder) error { size, err := s.List() if err != nil { - return err + return wrapStreamError(err, val.Type()) } if size == 0 { zero(val, 0) @@ -474,10 +474,11 @@ var ( // has been reached during streaming. EOL = errors.New("rlp: end of list") - // Other errors + // Actual Errors ErrExpectedString = errors.New("rlp: expected String or Byte") ErrExpectedList = errors.New("rlp: expected List") - ErrCanonInt = errors.New("rlp: expected Int") + ErrCanonInt = errors.New("rlp: non-canonical (leading zero bytes) integer") + ErrCanonSize = errors.New("rlp: non-canonical size information") ErrElemTooLarge = errors.New("rlp: element is larger than containing list") ErrValueTooLarge = errors.New("rlp: value size exceeds available input length") @@ -519,6 +520,7 @@ type Stream struct { kind Kind // kind of value ahead size uint64 // size of value ahead byteval byte // value of single byte in type tag + kinderr error // error from last readKind stack []listpos } @@ -619,13 +621,21 @@ func (s *Stream) uint(maxbits int) (uint64, error) { } switch kind { case Byte: + if s.byteval == 0 { + return 0, ErrCanonInt + } s.kind = -1 // rearm Kind return uint64(s.byteval), nil case String: if size > uint64(maxbits/8) { return 0, errUintOverflow } - return s.readUint(byte(size)) + v, err := s.readUint(byte(size)) + if err == ErrCanonSize { + // Adjust error because we're not reading a size right now. + err = ErrCanonInt + } + return v, err default: return 0, ErrExpectedString } @@ -729,6 +739,7 @@ func (s *Stream) Reset(r io.Reader, inputLimit uint64) { s.stack = s.stack[:0] s.size = 0 s.kind = -1 + s.kinderr = nil if s.uintbuf == nil { s.uintbuf = make([]byte, 8) } @@ -751,32 +762,31 @@ func (s *Stream) Kind() (kind Kind, size uint64, err error) { tos = &s.stack[len(s.stack)-1] } if s.kind < 0 { + s.kinderr = nil // Don't read further if we're at the end of the // innermost list. if tos != nil && tos.pos == tos.size { return 0, 0, EOL } - kind, size, err = s.readKind() - if err != nil { - return 0, 0, err - } - s.kind, s.size = kind, size - } - // Make sure size is reasonable. This is done always - // so Kind returns the same error when called multiple times. - if tos == nil { - // At toplevel, check that the value is smaller - // than the remaining input length. - if s.limited && s.size > s.remaining { - return 0, 0, ErrValueTooLarge - } - } else { - // Inside a list, check that the value doesn't overflow the list. - if s.size > tos.size-tos.pos { - return 0, 0, ErrElemTooLarge + s.kind, s.size, s.kinderr = s.readKind() + if s.kinderr == nil { + if tos == nil { + // At toplevel, check that the value is smaller + // than the remaining input length. + if s.limited && s.size > s.remaining { + s.kinderr = ErrValueTooLarge + } + } else { + // Inside a list, check that the value doesn't overflow the list. + if s.size > tos.size-tos.pos { + s.kinderr = ErrElemTooLarge + } + } } } - return s.kind, s.size, nil + // Note: this might return a sticky error generated + // by an earlier call to readKind. + return s.kind, s.size, s.kinderr } func (s *Stream) readKind() (kind Kind, size uint64, err error) { @@ -805,6 +815,9 @@ func (s *Stream) readKind() (kind Kind, size uint64, err error) { // would be encoded as 0xB90400 followed by the string. The range of // the first byte is thus [0xB8, 0xBF]. size, err = s.readUint(b - 0xB7) + if err == nil && size < 56 { + err = ErrCanonSize + } return String, size, err case b < 0xF8: // If the total payload of a list @@ -821,24 +834,40 @@ func (s *Stream) readKind() (kind Kind, size uint64, err error) { // the concatenation of the RLP encodings of the items. The // range of the first byte is thus [0xF8, 0xFF]. size, err = s.readUint(b - 0xF7) + if err == nil && size < 56 { + err = ErrCanonSize + } return List, size, err } } func (s *Stream) readUint(size byte) (uint64, error) { - if size == 1 { + switch size { + case 0: + s.kind = -1 // rearm Kind + return 0, nil + case 1: b, err := s.readByte() if err == io.EOF { err = io.ErrUnexpectedEOF } return uint64(b), err + default: + start := int(8 - size) + for i := 0; i < start; i++ { + s.uintbuf[i] = 0 + } + if err := s.readFull(s.uintbuf[start:]); err != nil { + return 0, err + } + if s.uintbuf[start] == 0 { + // Note: readUint is also used to decode integer + // values. The error needs to be adjusted to become + // ErrCanonInt in this case. + return 0, ErrCanonSize + } + return binary.BigEndian.Uint64(s.uintbuf), nil } - start := int(8 - size) - for i := 0; i < start; i++ { - s.uintbuf[i] = 0 - } - err := s.readFull(s.uintbuf[start:]) - return binary.BigEndian.Uint64(s.uintbuf), err } func (s *Stream) readFull(buf []byte) (err error) { diff --git a/rlp/decode_test.go b/rlp/decode_test.go index e5c7f3761f..aa410de921 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -21,16 +21,11 @@ func TestStreamKind(t *testing.T) { {"7F", Byte, 0}, {"80", String, 0}, {"B7", String, 55}, - {"B800", String, 0}, {"B90400", String, 1024}, - {"BA000400", String, 1024}, - {"BB00000400", String, 1024}, {"BFFFFFFFFFFFFFFFFF", String, ^uint64(0)}, {"C0", List, 0}, {"C8", List, 8}, {"F7", List, 55}, - {"F800", List, 0}, - {"F804", List, 4}, {"F90400", List, 1024}, {"FFFFFFFFFFFFFFFFFF", List, ^uint64(0)}, } @@ -85,7 +80,7 @@ func TestStreamErrors(t *testing.T) { string calls newStream func([]byte) *Stream // uses bytes.Reader if nil - error + error error }{ {"C0", calls{"Bytes"}, nil, ErrExpectedString}, {"C0", calls{"Uint"}, nil, ErrExpectedString}, @@ -98,6 +93,21 @@ func TestStreamErrors(t *testing.T) { {"00", calls{"ListEnd"}, nil, errNotInList}, {"C401020304", calls{"List", "Uint", "ListEnd"}, nil, errNotAtEOL}, + // Leading zero bytes are rejected when reading integers. + {"00", calls{"Uint"}, nil, ErrCanonInt}, + {"820002", calls{"Uint"}, nil, ErrCanonInt}, + + // Size tags must use the smallest possible encoding. + // Leading zero bytes in the size tag are also rejected. + {"B800", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"B90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"B90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"BA0002FFFF", calls{"Bytes"}, withoutInputLimit, ErrCanonSize}, + {"F800", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"F90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"F90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, + {"FA0002FFFF", calls{"List"}, withoutInputLimit, ErrCanonSize}, + // Expected EOF {"", calls{"Kind"}, nil, io.EOF}, {"", calls{"Uint"}, nil, io.EOF}, @@ -170,10 +180,12 @@ testfor: want = test.error.Error() } if err != want { + t.Log(test) t.Errorf("test %d: last call (%s) error mismatch\ngot: %s\nwant: %s", i, call, err, test.error) } } else if err != "" { + t.Log(test) t.Errorf("test %d: call %d (%s) unexpected error: %q", i, j, call, err) continue testfor } @@ -289,15 +301,20 @@ var decodeTests = []decodeTest{ {input: "8405050505", ptr: new(uint32), value: uint32(0x05050505)}, {input: "850505050505", ptr: new(uint32), error: "rlp: input string too long for uint32"}, {input: "C0", ptr: new(uint32), error: "rlp: expected input string or byte for uint32"}, + {input: "00", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"}, + {input: "820004", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"}, + {input: "B8020004", ptr: new(uint32), error: "rlp: non-canonical size information for uint32"}, // slices {input: "C0", ptr: new([]uint), value: []uint{}}, {input: "C80102030405060708", ptr: new([]uint), value: []uint{1, 2, 3, 4, 5, 6, 7, 8}}, + {input: "F8020004", ptr: new([]uint), error: "rlp: non-canonical size information for []uint"}, // arrays {input: "C0", ptr: new([5]uint), value: [5]uint{}}, {input: "C50102030405", ptr: new([5]uint), value: [5]uint{1, 2, 3, 4, 5}}, {input: "C6010203040506", ptr: new([5]uint), error: "rlp: input list has too many elements for [5]uint"}, + {input: "F8020004", ptr: new([5]uint), error: "rlp: non-canonical size information for [5]uint"}, // byte slices {input: "01", ptr: new([]byte), value: []byte{1}}, @@ -352,7 +369,7 @@ var decodeTests = []decodeTest{ // big ints {input: "01", ptr: new(*big.Int), value: big.NewInt(1)}, {input: "89FFFFFFFFFFFFFFFFFF", ptr: new(*big.Int), value: veryBigInt}, - {input: "820001", ptr: new(big.Int), error: "rlp: canon int error appends zero's for *big.Int"}, + {input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, {input: "10", ptr: new(big.Int), value: *big.NewInt(16)}, // non-pointer also works {input: "C0", ptr: new(*big.Int), error: "rlp: expected input string or byte for *big.Int"}, @@ -366,6 +383,11 @@ var decodeTests = []decodeTest{ value: recstruct{1, &recstruct{2, &recstruct{3, nil}}}, }, + { + input: "83222222", + ptr: new(simplestruct), + error: "rlp: expected input list for rlp.simplestruct", + }, { input: "C3010101", ptr: new(simplestruct), @@ -378,7 +400,7 @@ var decodeTests = []decodeTest{ }, // pointers - {input: "00", ptr: new(*uint), value: uintp(0)}, + {input: "00", ptr: new(*[]byte), value: &[]byte{0}}, {input: "80", ptr: new(*uint), value: (*uint)(nil)}, {input: "C0", ptr: new(*uint), value: (*uint)(nil)}, {input: "07", ptr: new(*uint), value: uintp(7)}, From 1e2c93aa2da453ef9548b9957b5ed453f60ce5ca Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 16 Apr 2015 10:15:14 +0200 Subject: [PATCH 08/13] rlp: reject non-minimal input strings Input strings of length 1 containing a byte < 56 are non-minimal and should be encoded as a single byte instead. Reject such strings. --- rlp/decode.go | 28 +++++++++++++++++++++------- rlp/decode_test.go | 32 ++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 6bd13aa8fb..43dd716b54 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -305,10 +305,11 @@ func decodeByteSlice(s *Stream, val reflect.Value) error { return decodeListSlice(s, val, decodeUint) } b, err := s.Bytes() - if err == nil { - val.SetBytes(b) + if err != nil { + return wrapStreamError(err, val.Type()) } - return err + val.SetBytes(b) + return nil } func decodeByteArray(s *Stream, val reflect.Value) error { @@ -333,6 +334,10 @@ func decodeByteArray(s *Stream, val reflect.Value) error { return err } zero(val, int(size)) + // Reject cases where single byte encoding should have been used. + if size == 1 && slice[0] < 56 { + return wrapStreamError(ErrCanonSize, val.Type()) + } case List: return decodeListArray(s, val, decodeUint) } @@ -477,7 +482,7 @@ var ( // Actual Errors ErrExpectedString = errors.New("rlp: expected String or Byte") ErrExpectedList = errors.New("rlp: expected List") - ErrCanonInt = errors.New("rlp: non-canonical (leading zero bytes) integer") + ErrCanonInt = errors.New("rlp: non-canonical integer format") ErrCanonSize = errors.New("rlp: non-canonical size information") ErrElemTooLarge = errors.New("rlp: element is larger than containing list") ErrValueTooLarge = errors.New("rlp: value size exceeds available input length") @@ -576,6 +581,9 @@ func (s *Stream) Bytes() ([]byte, error) { if err = s.readFull(b); err != nil { return nil, err } + if size == 1 && b[0] < 56 { + return nil, ErrCanonSize + } return b, nil default: return nil, ErrExpectedString @@ -631,11 +639,17 @@ func (s *Stream) uint(maxbits int) (uint64, error) { return 0, errUintOverflow } v, err := s.readUint(byte(size)) - if err == ErrCanonSize { + switch { + case err == ErrCanonSize: // Adjust error because we're not reading a size right now. - err = ErrCanonInt + return 0, ErrCanonInt + case err != nil: + return 0, err + case size > 0 && v < 56: + return 0, ErrCanonSize + default: + return v, nil } - return v, err default: return 0, ErrExpectedString } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index aa410de921..7e2ea2041b 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -93,12 +93,16 @@ func TestStreamErrors(t *testing.T) { {"00", calls{"ListEnd"}, nil, errNotInList}, {"C401020304", calls{"List", "Uint", "ListEnd"}, nil, errNotAtEOL}, - // Leading zero bytes are rejected when reading integers. + // Non-canonical integers (e.g. leading zero bytes). {"00", calls{"Uint"}, nil, ErrCanonInt}, {"820002", calls{"Uint"}, nil, ErrCanonInt}, + {"8133", calls{"Uint"}, nil, ErrCanonSize}, + {"8156", calls{"Uint"}, nil, nil}, // Size tags must use the smallest possible encoding. // Leading zero bytes in the size tag are also rejected. + {"8100", calls{"Uint"}, nil, ErrCanonSize}, + {"8100", calls{"Bytes"}, nil, ErrCanonSize}, {"B800", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, {"B90000", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, {"B90055", calls{"Kind"}, withoutInputLimit, ErrCanonSize}, @@ -112,7 +116,7 @@ func TestStreamErrors(t *testing.T) { {"", calls{"Kind"}, nil, io.EOF}, {"", calls{"Uint"}, nil, io.EOF}, {"", calls{"List"}, nil, io.EOF}, - {"8105", calls{"Uint", "Uint"}, nil, io.EOF}, + {"8158", calls{"Uint", "Uint"}, nil, io.EOF}, {"C0", calls{"List", "ListEnd", "List"}, nil, io.EOF}, // Input limit errors. @@ -129,11 +133,11 @@ func TestStreamErrors(t *testing.T) { // down toward zero in Stream.remaining, reading too far can overflow // remaining to a large value, effectively disabling the limit. {"C40102030401", calls{"Raw", "Uint"}, withCustomInputLimit(5), io.EOF}, - {"C4010203048102", calls{"Raw", "Uint"}, withCustomInputLimit(6), ErrValueTooLarge}, + {"C4010203048158", calls{"Raw", "Uint"}, withCustomInputLimit(6), ErrValueTooLarge}, // Check that the same calls are fine without a limit. {"C40102030401", calls{"Raw", "Uint"}, withoutInputLimit, nil}, - {"C4010203048102", calls{"Raw", "Uint"}, withoutInputLimit, nil}, + {"C4010203048158", calls{"Raw", "Uint"}, withoutInputLimit, nil}, // Unexpected EOF. This only happens when there is // no input limit, so the reader needs to be 'dumbed down'. @@ -295,13 +299,13 @@ var decodeTests = []decodeTest{ // integers {input: "05", ptr: new(uint32), value: uint32(5)}, {input: "80", ptr: new(uint32), value: uint32(0)}, - {input: "8105", ptr: new(uint32), value: uint32(5)}, {input: "820505", ptr: new(uint32), value: uint32(0x0505)}, {input: "83050505", ptr: new(uint32), value: uint32(0x050505)}, {input: "8405050505", ptr: new(uint32), value: uint32(0x05050505)}, {input: "850505050505", ptr: new(uint32), error: "rlp: input string too long for uint32"}, {input: "C0", ptr: new(uint32), error: "rlp: expected input string or byte for uint32"}, {input: "00", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"}, + {input: "8105", ptr: new(uint32), error: "rlp: non-canonical size information for uint32"}, {input: "820004", ptr: new(uint32), error: "rlp: non-canonical integer (leading zero bytes) for uint32"}, {input: "B8020004", ptr: new(uint32), error: "rlp: non-canonical size information for uint32"}, @@ -319,10 +323,16 @@ var decodeTests = []decodeTest{ // byte slices {input: "01", ptr: new([]byte), value: []byte{1}}, {input: "80", ptr: new([]byte), value: []byte{}}, + {input: "8D6162636465666768696A6B6C6D", ptr: new([]byte), value: []byte("abcdefghijklm")}, {input: "C0", ptr: new([]byte), value: []byte{}}, {input: "C3010203", ptr: new([]byte), value: []byte{1, 2, 3}}, + { + input: "8105", + ptr: new([]byte), + error: "rlp: non-canonical size information for []uint8", + }, { input: "C3820102", ptr: new([]byte), @@ -346,10 +356,15 @@ var decodeTests = []decodeTest{ ptr: new([5]byte), error: "rlp: input string too long for [5]uint8", }, + { + input: "8105", + ptr: new([5]byte), + error: "rlp: non-canonical size information for [5]uint8", + }, // byte array reuse (should be zeroed) {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, - {input: "8101", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String + {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: Byte {input: "C3010203", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 0, 0}}, @@ -369,9 +384,10 @@ var decodeTests = []decodeTest{ // big ints {input: "01", ptr: new(*big.Int), value: big.NewInt(1)}, {input: "89FFFFFFFFFFFFFFFFFF", ptr: new(*big.Int), value: veryBigInt}, - {input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, {input: "10", ptr: new(big.Int), value: *big.NewInt(16)}, // non-pointer also works {input: "C0", ptr: new(*big.Int), error: "rlp: expected input string or byte for *big.Int"}, + {input: "820001", ptr: new(big.Int), error: "rlp: non-canonical integer (leading zero bytes) for *big.Int"}, + {input: "8105", ptr: new(big.Int), error: "rlp: non-canonical size information for *big.Int"}, // structs {input: "C0", ptr: new(simplestruct), value: simplestruct{0, ""}}, @@ -404,7 +420,7 @@ var decodeTests = []decodeTest{ {input: "80", ptr: new(*uint), value: (*uint)(nil)}, {input: "C0", ptr: new(*uint), value: (*uint)(nil)}, {input: "07", ptr: new(*uint), value: uintp(7)}, - {input: "8108", ptr: new(*uint), value: uintp(8)}, + {input: "8158", ptr: new(*uint), value: uintp(0x58)}, {input: "C109", ptr: new(*[]uint), value: &[]uint{9}}, {input: "C58403030303", ptr: new(*[][]byte), value: &[][]byte{{3, 3, 3, 3}}}, From cad64fb911e7029bef876f16e0956b3b0b4bb4d0 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Apr 2015 01:16:46 +0200 Subject: [PATCH 09/13] rlp: stricter rules for structs and pointers The rules have changed as follows: * When decoding into pointers, empty values no longer produce a nil pointer. This can be overriden for struct fields using the struct tag "nil". * When decoding into structs, the input list must contain an element for each field. --- rlp/decode.go | 76 ++++++++++++++++++++++++++++++++-------------- rlp/decode_test.go | 65 +++++++++++++++++++++++++++++++-------- rlp/encode.go | 8 ++--- rlp/typecache.go | 51 ++++++++++++++++++++++--------- 4 files changed, 148 insertions(+), 52 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 43dd716b54..394f83fb23 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -36,17 +36,26 @@ type Decoder interface { // If the type implements the Decoder interface, decode calls // DecodeRLP. // -// To decode into a pointer, Decode will set the pointer to nil if the -// input has size zero. If the input has nonzero size, Decode will -// parse the input data into a value of the type being pointed to. -// If the pointer is non-nil, the existing value will reused. +// To decode into a pointer, Decode will decode into the value pointed +// to. If the pointer is nil, a new value of the pointer's element +// type is allocated. If the pointer is non-nil, the existing value +// will reused. // // To decode into a struct, Decode expects the input to be an RLP // list. The decoded elements of the list are assigned to each public -// field in the order given by the struct's definition. If the input -// list has too few elements, no error is returned and the remaining -// fields will have the zero value. -// Recursive struct types are supported. +// field in the order given by the struct's definition. The input list +// must contain an element for each decoded field. Decode returns an +// error if there are too few or too many elements. +// +// The decoding of struct fields honours one particular struct tag, +// "nil". This tag applies to pointer-typed fields and changes the +// decoding rules for the field such that input values of size zero +// decode as a nil pointer. This tag can be useful when decoding recursive +// types. +// +// type StructWithEmptyOK struct { +// Foo *[20]byte `rlp:"nil"` +// } // // To decode into a slice, the input must be a list and the resulting // slice will contain the input elements in order. @@ -54,7 +63,7 @@ type Decoder interface { // can also be an RLP string. // // To decode into a Go string, the input must be an RLP string. The -// bytes are taken as-is and will not necessarily be valid UTF-8. +// input bytes are taken as-is and will not necessarily be valid UTF-8. // // To decode into an unsigned integer type, the input must also be an RLP // string. The bytes are interpreted as a big endian representation of @@ -65,8 +74,8 @@ type Decoder interface { // To decode into an interface value, Decode stores one of these // in the value: // -// []interface{}, for RLP lists -// []byte, for RLP strings +// []interface{}, for RLP lists +// []byte, for RLP strings // // Non-empty interface types are not supported, nor are booleans, // signed integers, floating point numbers, maps, channels and @@ -136,7 +145,7 @@ var ( bigInt = reflect.TypeOf(big.Int{}) ) -func makeDecoder(typ reflect.Type) (dec decoder, err error) { +func makeDecoder(typ reflect.Type, tags tags) (dec decoder, err error) { kind := typ.Kind() switch { case typ.Implements(decoderInterface): @@ -156,6 +165,9 @@ func makeDecoder(typ reflect.Type) (dec decoder, err error) { case kind == reflect.Struct: return makeStructDecoder(typ) case kind == reflect.Ptr: + if tags.nilOK { + return makeOptionalPtrDecoder(typ) + } return makePtrDecoder(typ) case kind == reflect.Interface: return decodeInterface, nil @@ -214,7 +226,7 @@ func makeListDecoder(typ reflect.Type) (decoder, error) { return decodeByteSlice, nil } } - etypeinfo, err := cachedTypeInfo1(etype) + etypeinfo, err := cachedTypeInfo1(etype, tags{}) if err != nil { return nil, err } @@ -352,11 +364,6 @@ func zero(val reflect.Value, start int) { } } -type field struct { - index int - info *typeinfo -} - func makeStructDecoder(typ reflect.Type) (decoder, error) { fields, err := structFields(typ) if err != nil { @@ -369,8 +376,7 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) { for _, f := range fields { err = f.info.decoder(s, val.Field(f.index)) if err == EOL { - // too few elements. leave the rest at their zero value. - break + return &decodeError{msg: "too few elements", typ: typ} } else if err != nil { return addErrorContext(err, "."+typ.Field(f.index).Name) } @@ -380,9 +386,35 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) { return dec, nil } +// makePtrDecoder creates a decoder that decodes into +// the pointer's element type. func makePtrDecoder(typ reflect.Type) (decoder, error) { etype := typ.Elem() - etypeinfo, err := cachedTypeInfo1(etype) + etypeinfo, err := cachedTypeInfo1(etype, tags{}) + if err != nil { + return nil, err + } + dec := func(s *Stream, val reflect.Value) (err error) { + newval := val + if val.IsNil() { + newval = reflect.New(etype) + } + if err = etypeinfo.decoder(s, newval.Elem()); err == nil { + val.Set(newval) + } + return err + } + return dec, nil +} + +// makeOptionalPtrDecoder creates a decoder that decodes empty values +// as nil. Non-empty values are decoded into a value of the element type, +// just like makePtrDecoder does. +// +// This decoder is used for pointer-typed struct fields with struct tag "nil". +func makeOptionalPtrDecoder(typ reflect.Type) (decoder, error) { + etype := typ.Elem() + etypeinfo, err := cachedTypeInfo1(etype, tags{}) if err != nil { return nil, err } @@ -706,7 +738,7 @@ func (s *Stream) Decode(val interface{}) error { if rval.IsNil() { return errDecodeIntoNil } - info, err := cachedTypeInfo(rtyp.Elem()) + info, err := cachedTypeInfo(rtyp.Elem(), tags{}) if err != nil { return err } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 7e2ea2041b..fd52bd1be1 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -280,7 +280,7 @@ type simplestruct struct { type recstruct struct { I uint - Child *recstruct + Child *recstruct `rlp:"nil"` } var ( @@ -390,15 +390,33 @@ var decodeTests = []decodeTest{ {input: "8105", ptr: new(big.Int), error: "rlp: non-canonical size information for *big.Int"}, // structs - {input: "C0", ptr: new(simplestruct), value: simplestruct{0, ""}}, - {input: "C105", ptr: new(simplestruct), value: simplestruct{5, ""}}, - {input: "C50583343434", ptr: new(simplestruct), value: simplestruct{5, "444"}}, { - input: "C501C302C103", + input: "C50583343434", + ptr: new(simplestruct), + value: simplestruct{5, "444"}, + }, + { + input: "C601C402C203C0", ptr: new(recstruct), value: recstruct{1, &recstruct{2, &recstruct{3, nil}}}, }, + // struct errors + { + input: "C0", + ptr: new(simplestruct), + error: "rlp: too few elements for rlp.simplestruct", + }, + { + input: "C105", + ptr: new(simplestruct), + error: "rlp: too few elements for rlp.simplestruct", + }, + { + input: "C7C50583343434C0", + ptr: new([]*simplestruct), + error: "rlp: too few elements for rlp.simplestruct, decoding into ([]*rlp.simplestruct)[1]", + }, { input: "83222222", ptr: new(simplestruct), @@ -417,19 +435,15 @@ var decodeTests = []decodeTest{ // pointers {input: "00", ptr: new(*[]byte), value: &[]byte{0}}, - {input: "80", ptr: new(*uint), value: (*uint)(nil)}, - {input: "C0", ptr: new(*uint), value: (*uint)(nil)}, + {input: "80", ptr: new(*uint), value: uintp(0)}, + {input: "C0", ptr: new(*uint), error: "rlp: expected input string or byte for uint"}, {input: "07", ptr: new(*uint), value: uintp(7)}, {input: "8158", ptr: new(*uint), value: uintp(0x58)}, {input: "C109", ptr: new(*[]uint), value: &[]uint{9}}, {input: "C58403030303", ptr: new(*[][]byte), value: &[][]byte{{3, 3, 3, 3}}}, // check that input position is advanced also for empty values. - {input: "C3808005", ptr: new([]*uint), value: []*uint{nil, nil, uintp(5)}}, - - // pointer should be reset to nil - {input: "05", ptr: sharedPtr, value: uintp(5)}, - {input: "80", ptr: sharedPtr, value: (*uint)(nil)}, + {input: "C3808005", ptr: new([]*uint), value: []*uint{uintp(0), uintp(0), uintp(5)}}, // interface{} {input: "00", ptr: new(interface{}), value: []byte{0}}, @@ -599,6 +613,33 @@ func ExampleDecode() { // Decoded value: rlp.example{A:0xa, B:0x14, private:0x0, String:"foobar"} } +func ExampleDecode_structTagNil() { + // In this example, we'll use the "nil" struct tag to change + // how a pointer-typed field is decoded. The input contains an RLP + // list of one element, an empty string. + input := []byte{0xC1, 0x80} + + // This type uses the normal rules. + // The empty input string is decoded as a pointer to an empty Go string. + var normalRules struct { + String *string + } + Decode(bytes.NewReader(input), &normalRules) + fmt.Printf("normal: String = %q\n", *normalRules.String) + + // This type uses the struct tag. + // The empty input string is decoded as a nil pointer. + var withEmptyOK struct { + String *string `rlp:"nil"` + } + Decode(bytes.NewReader(input), &withEmptyOK) + fmt.Printf("with nil tag: String = %v\n", withEmptyOK.String) + + // Output: + // normal: String = "" + // with nil tag: String = +} + func ExampleStream() { input, _ := hex.DecodeString("C90A1486666F6F626172") s := NewStream(bytes.NewReader(input), 0) diff --git a/rlp/encode.go b/rlp/encode.go index 6cf6776d6a..10ff0ae79e 100644 --- a/rlp/encode.go +++ b/rlp/encode.go @@ -194,7 +194,7 @@ func (w *encbuf) Write(b []byte) (int, error) { func (w *encbuf) encode(val interface{}) error { rval := reflect.ValueOf(val) - ti, err := cachedTypeInfo(rval.Type()) + ti, err := cachedTypeInfo(rval.Type(), tags{}) if err != nil { return err } @@ -485,7 +485,7 @@ func writeInterface(val reflect.Value, w *encbuf) error { return nil } eval := val.Elem() - ti, err := cachedTypeInfo(eval.Type()) + ti, err := cachedTypeInfo(eval.Type(), tags{}) if err != nil { return err } @@ -493,7 +493,7 @@ func writeInterface(val reflect.Value, w *encbuf) error { } func makeSliceWriter(typ reflect.Type) (writer, error) { - etypeinfo, err := cachedTypeInfo1(typ.Elem()) + etypeinfo, err := cachedTypeInfo1(typ.Elem(), tags{}) if err != nil { return nil, err } @@ -530,7 +530,7 @@ func makeStructWriter(typ reflect.Type) (writer, error) { } func makePtrWriter(typ reflect.Type) (writer, error) { - etypeinfo, err := cachedTypeInfo1(typ.Elem()) + etypeinfo, err := cachedTypeInfo1(typ.Elem(), tags{}) if err != nil { return nil, err } diff --git a/rlp/typecache.go b/rlp/typecache.go index 398f25d90a..d512012e95 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -7,7 +7,7 @@ import ( var ( typeCacheMutex sync.RWMutex - typeCache = make(map[reflect.Type]*typeinfo) + typeCache = make(map[typekey]*typeinfo) ) type typeinfo struct { @@ -15,13 +15,25 @@ type typeinfo struct { writer } +// represents struct tags +type tags struct { + nilOK bool +} + +type typekey struct { + reflect.Type + // the key must include the struct tags because they + // might generate a different decoder. + tags +} + type decoder func(*Stream, reflect.Value) error type writer func(reflect.Value, *encbuf) error -func cachedTypeInfo(typ reflect.Type) (*typeinfo, error) { +func cachedTypeInfo(typ reflect.Type, tags tags) (*typeinfo, error) { typeCacheMutex.RLock() - info := typeCache[typ] + info := typeCache[typekey{typ, tags}] typeCacheMutex.RUnlock() if info != nil { return info, nil @@ -29,11 +41,12 @@ func cachedTypeInfo(typ reflect.Type) (*typeinfo, error) { // not in the cache, need to generate info for this type. typeCacheMutex.Lock() defer typeCacheMutex.Unlock() - return cachedTypeInfo1(typ) + return cachedTypeInfo1(typ, tags) } -func cachedTypeInfo1(typ reflect.Type) (*typeinfo, error) { - info := typeCache[typ] +func cachedTypeInfo1(typ reflect.Type, tags tags) (*typeinfo, error) { + key := typekey{typ, tags} + info := typeCache[key] if info != nil { // another goroutine got the write lock first return info, nil @@ -41,21 +54,27 @@ func cachedTypeInfo1(typ reflect.Type) (*typeinfo, error) { // put a dummmy value into the cache before generating. // if the generator tries to lookup itself, it will get // the dummy value and won't call itself recursively. - typeCache[typ] = new(typeinfo) - info, err := genTypeInfo(typ) + typeCache[key] = new(typeinfo) + info, err := genTypeInfo(typ, tags) if err != nil { // remove the dummy value if the generator fails - delete(typeCache, typ) + delete(typeCache, key) return nil, err } - *typeCache[typ] = *info - return typeCache[typ], err + *typeCache[key] = *info + return typeCache[key], err +} + +type field struct { + index int + info *typeinfo } func structFields(typ reflect.Type) (fields []field, err error) { for i := 0; i < typ.NumField(); i++ { if f := typ.Field(i); f.PkgPath == "" { // exported - info, err := cachedTypeInfo1(f.Type) + tags := parseStructTag(f.Tag.Get("rlp")) + info, err := cachedTypeInfo1(f.Type, tags) if err != nil { return nil, err } @@ -65,9 +84,13 @@ func structFields(typ reflect.Type) (fields []field, err error) { return fields, nil } -func genTypeInfo(typ reflect.Type) (info *typeinfo, err error) { +func parseStructTag(tag string) tags { + return tags{nilOK: tag == "nil"} +} + +func genTypeInfo(typ reflect.Type, tags tags) (info *typeinfo, err error) { info = new(typeinfo) - if info.decoder, err = makeDecoder(typ); err != nil { + if info.decoder, err = makeDecoder(typ, tags); err != nil { return nil, err } if info.writer, err = makeWriter(typ); err != nil { From 574d5d6ae6076c534d314c600ee0e6c0c161cc36 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Apr 2015 01:29:11 +0200 Subject: [PATCH 10/13] core/types: add rlp tag "nil" for Transaction.Recipient --- core/types/transaction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/types/transaction.go b/core/types/transaction.go index 6646bdf295..d8dcd74240 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -22,7 +22,7 @@ type Transaction struct { AccountNonce uint64 Price *big.Int GasLimit *big.Int - Recipient *common.Address // nil means contract creation + Recipient *common.Address `rlp:"nil"` // nil means contract creation Amount *big.Int Payload []byte V byte From 4d5a518a0ba7b0f1d42c73f3c28fe0828e7ea974 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Apr 2015 02:01:38 +0200 Subject: [PATCH 11/13] rlp: stop accepting lists for byte slices and byte arrays --- rlp/decode.go | 14 +++----------- rlp/decode_test.go | 41 +++++++---------------------------------- 2 files changed, 10 insertions(+), 45 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 394f83fb23..97eacf64bc 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -58,9 +58,8 @@ type Decoder interface { // } // // To decode into a slice, the input must be a list and the resulting -// slice will contain the input elements in order. -// As a special case, if the slice has a byte-size element type, the input -// can also be an RLP string. +// slice will contain the input elements in order. For byte slices, +// the input must be an RLP string. // // To decode into a Go string, the input must be an RLP string. The // input bytes are taken as-is and will not necessarily be valid UTF-8. @@ -309,13 +308,6 @@ func decodeListArray(s *Stream, val reflect.Value, elemdec decoder) error { } func decodeByteSlice(s *Stream, val reflect.Value) error { - kind, _, err := s.Kind() - if err != nil { - return err - } - if kind == List { - return decodeListSlice(s, val, decodeUint) - } b, err := s.Bytes() if err != nil { return wrapStreamError(err, val.Type()) @@ -351,7 +343,7 @@ func decodeByteArray(s *Stream, val reflect.Value) error { return wrapStreamError(ErrCanonSize, val.Type()) } case List: - return decodeListArray(s, val, decodeUint) + return wrapStreamError(ErrExpectedString, val.Type()) } return nil } diff --git a/rlp/decode_test.go b/rlp/decode_test.go index fd52bd1be1..0b69ff1f4c 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -323,56 +323,29 @@ var decodeTests = []decodeTest{ // byte slices {input: "01", ptr: new([]byte), value: []byte{1}}, {input: "80", ptr: new([]byte), value: []byte{}}, - {input: "8D6162636465666768696A6B6C6D", ptr: new([]byte), value: []byte("abcdefghijklm")}, - {input: "C0", ptr: new([]byte), value: []byte{}}, - {input: "C3010203", ptr: new([]byte), value: []byte{1, 2, 3}}, - - { - input: "8105", - ptr: new([]byte), - error: "rlp: non-canonical size information for []uint8", - }, - { - input: "C3820102", - ptr: new([]byte), - error: "rlp: input string too long for uint8, decoding into ([]uint8)[0]", - }, + {input: "C0", ptr: new([]byte), error: "rlp: expected input string or byte for []uint8"}, + {input: "8105", ptr: new([]byte), error: "rlp: non-canonical size information for []uint8"}, // byte arrays {input: "01", ptr: new([5]byte), value: [5]byte{1}}, {input: "80", ptr: new([5]byte), value: [5]byte{}}, {input: "850102030405", ptr: new([5]byte), value: [5]byte{1, 2, 3, 4, 5}}, - {input: "C0", ptr: new([5]byte), value: [5]byte{}}, - {input: "C3010203", ptr: new([5]byte), value: [5]byte{1, 2, 3, 0, 0}}, - { - input: "C3820102", - ptr: new([5]byte), - error: "rlp: input string too long for uint8, decoding into ([5]uint8)[0]", - }, - { - input: "86010203040506", - ptr: new([5]byte), - error: "rlp: input string too long for [5]uint8", - }, - { - input: "8105", - ptr: new([5]byte), - error: "rlp: non-canonical size information for [5]uint8", - }, + // byte array errors + {input: "C0", ptr: new([5]byte), error: "rlp: expected input string or byte for [5]uint8"}, + {input: "C3010203", ptr: new([5]byte), error: "rlp: expected input string or byte for [5]uint8"}, + {input: "86010203040506", ptr: new([5]byte), error: "rlp: input string too long for [5]uint8"}, + {input: "8105", ptr: new([5]byte), error: "rlp: non-canonical size information for [5]uint8"}, // byte array reuse (should be zeroed) {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: Byte - {input: "C3010203", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 0, 0}}, - {input: "C101", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: List // zero sized byte arrays {input: "80", ptr: new([0]byte), value: [0]byte{}}, - {input: "C0", ptr: new([0]byte), value: [0]byte{}}, {input: "01", ptr: new([0]byte), error: "rlp: input string too long for [0]uint8"}, {input: "8101", ptr: new([0]byte), error: "rlp: input string too long for [0]uint8"}, From 9c7281c17ebbdd6a8c10ecc618bcb9121215a21f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Apr 2015 02:13:32 +0200 Subject: [PATCH 12/13] p2p: make DiscReason bigger than byte We decode into [1]DiscReason in a few places. That doesn't work anymore because package rlp no longer accepts RLP lists for byte arrays. --- p2p/peer_error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/peer_error.go b/p2p/peer_error.go index 4021316306..a912f60644 100644 --- a/p2p/peer_error.go +++ b/p2p/peer_error.go @@ -57,7 +57,7 @@ func (self *peerError) Error() string { return self.message } -type DiscReason byte +type DiscReason uint const ( DiscRequested DiscReason = iota From 7180699d401e64b52447ccb2cfb33004b7deb672 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 17 Apr 2015 03:11:24 +0200 Subject: [PATCH 13/13] rlp: require declared number of input elements for array types --- rlp/decode.go | 40 +++++++++++++++------------------------- rlp/decode_test.go | 26 +++++++++++--------------- 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 97eacf64bc..6952ecaead 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -59,7 +59,9 @@ type Decoder interface { // // To decode into a slice, the input must be a list and the resulting // slice will contain the input elements in order. For byte slices, -// the input must be an RLP string. +// the input must be an RLP string. Array types decode similarly, with +// the additional restriction that the number of input elements (or +// bytes) must match the array's length. // // To decode into a Go string, the input must be an RLP string. The // input bytes are taken as-is and will not necessarily be valid UTF-8. @@ -279,19 +281,10 @@ func decodeListSlice(s *Stream, val reflect.Value, elemdec decoder) error { } func decodeListArray(s *Stream, val reflect.Value, elemdec decoder) error { - size, err := s.List() + _, err := s.List() if err != nil { return wrapStreamError(err, val.Type()) } - if size == 0 { - zero(val, 0) - return s.ListEnd() - } - - // The approach here is stolen from package json, although we differ - // in the semantics for arrays. package json discards remaining - // elements that would not fit into the array. We generate an error in - // this case because we'd be losing information. vlen := val.Len() i := 0 for ; i < vlen; i++ { @@ -302,7 +295,7 @@ func decodeListArray(s *Stream, val reflect.Value, elemdec decoder) error { } } if i < vlen { - zero(val, i) + return &decodeError{msg: "input list has too few elements", typ: val.Type()} } return wrapStreamError(s.ListEnd(), val.Type()) } @@ -321,23 +314,28 @@ func decodeByteArray(s *Stream, val reflect.Value) error { if err != nil { return err } + vlen := val.Len() switch kind { case Byte: - if val.Len() == 0 { + if vlen == 0 { return &decodeError{msg: "input string too long", typ: val.Type()} } + if vlen > 1 { + return &decodeError{msg: "input string too short", typ: val.Type()} + } bv, _ := s.Uint() val.Index(0).SetUint(bv) - zero(val, 1) case String: - if uint64(val.Len()) < size { + if uint64(vlen) < size { return &decodeError{msg: "input string too long", typ: val.Type()} } - slice := val.Slice(0, int(size)).Interface().([]byte) + if uint64(vlen) > size { + return &decodeError{msg: "input string too short", typ: val.Type()} + } + slice := val.Slice(0, vlen).Interface().([]byte) if err := s.readFull(slice); err != nil { return err } - zero(val, int(size)) // Reject cases where single byte encoding should have been used. if size == 1 && slice[0] < 56 { return wrapStreamError(ErrCanonSize, val.Type()) @@ -348,14 +346,6 @@ func decodeByteArray(s *Stream, val reflect.Value) error { return nil } -func zero(val reflect.Value, start int) { - z := reflect.Zero(val.Type().Elem()) - end := val.Len() - for i := start; i < end; i++ { - val.Index(i).Set(z) - } -} - func makeStructDecoder(typ reflect.Type) (decoder, error) { fields, err := structFields(typ) if err != nil { diff --git a/rlp/decode_test.go b/rlp/decode_test.go index 0b69ff1f4c..d07520bd06 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -290,11 +290,6 @@ var ( ) ) -var ( - sharedByteArray [5]byte - sharedPtr = new(*uint) -) - var decodeTests = []decodeTest{ // integers {input: "05", ptr: new(uint32), value: uint32(5)}, @@ -315,11 +310,16 @@ var decodeTests = []decodeTest{ {input: "F8020004", ptr: new([]uint), error: "rlp: non-canonical size information for []uint"}, // arrays - {input: "C0", ptr: new([5]uint), value: [5]uint{}}, {input: "C50102030405", ptr: new([5]uint), value: [5]uint{1, 2, 3, 4, 5}}, + {input: "C0", ptr: new([5]uint), error: "rlp: input list has too few elements for [5]uint"}, + {input: "C102", ptr: new([5]uint), error: "rlp: input list has too few elements for [5]uint"}, {input: "C6010203040506", ptr: new([5]uint), error: "rlp: input list has too many elements for [5]uint"}, {input: "F8020004", ptr: new([5]uint), error: "rlp: non-canonical size information for [5]uint"}, + // zero sized arrays + {input: "C0", ptr: new([0]uint), value: [0]uint{}}, + {input: "C101", ptr: new([0]uint), error: "rlp: input list has too many elements for [0]uint"}, + // byte slices {input: "01", ptr: new([]byte), value: []byte{1}}, {input: "80", ptr: new([]byte), value: []byte{}}, @@ -328,21 +328,17 @@ var decodeTests = []decodeTest{ {input: "8105", ptr: new([]byte), error: "rlp: non-canonical size information for []uint8"}, // byte arrays - {input: "01", ptr: new([5]byte), value: [5]byte{1}}, - {input: "80", ptr: new([5]byte), value: [5]byte{}}, + {input: "02", ptr: new([1]byte), value: [1]byte{2}}, {input: "850102030405", ptr: new([5]byte), value: [5]byte{1, 2, 3, 4, 5}}, // byte array errors + {input: "02", ptr: new([5]byte), error: "rlp: input string too short for [5]uint8"}, + {input: "80", ptr: new([5]byte), error: "rlp: input string too short for [5]uint8"}, + {input: "820000", ptr: new([5]byte), error: "rlp: input string too short for [5]uint8"}, {input: "C0", ptr: new([5]byte), error: "rlp: expected input string or byte for [5]uint8"}, {input: "C3010203", ptr: new([5]byte), error: "rlp: expected input string or byte for [5]uint8"}, {input: "86010203040506", ptr: new([5]byte), error: "rlp: input string too long for [5]uint8"}, - {input: "8105", ptr: new([5]byte), error: "rlp: non-canonical size information for [5]uint8"}, - - // byte array reuse (should be zeroed) - {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, - {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: String - {input: "850102030405", ptr: &sharedByteArray, value: [5]byte{1, 2, 3, 4, 5}}, - {input: "01", ptr: &sharedByteArray, value: [5]byte{1}}, // kind: Byte + {input: "8105", ptr: new([1]byte), error: "rlp: non-canonical size information for [1]uint8"}, // zero sized byte arrays {input: "80", ptr: new([0]byte), value: [0]byte{}},