rlp: improve nil pointer handling (#20064)

* rlp: improve nil pointer handling

In both encoder and decoder, the rules for encoding nil pointers were a
bit hard to understand, and didn't leave much choice. Since RLP allows
two empty values (empty list, empty string), any protocol built on RLP
must choose either of these values to represent the null value in a
certain context.

This change adds choice in the form of two new struct tags, "nilString"
and "nilList". These can be used to specify how a nil pointer value is
encoded. The "nil" tag still exists, but its implementation is now
explicit and defines exactly how nil pointers are handled in a single
place.

Another important change in this commit is how nil pointers and the
Encoder interface interact. The EncodeRLP method was previously called
even on nil values, which was supposed to give users a choice of how
their value would be handled when nil. It turns out this is a stupid
idea. If you create a network protocol containing an object defined in
another package, it's better to be able to say that the object should be
a list or string when nil in the definition of the protocol message
rather than defining the encoding of nil on the object itself.

As of this commit, the encoding rules for pointers now take precedence
over the Encoder interface rule. I think the "nil" tag will work fine
for most cases. For special kinds of objects which are a struct in Go
but strings in RLP, code using the object can specify the desired
encoding of nil using the "nilString" and "nilList" tags.

* rlp: propagate struct field type errors

If a struct contained fields of undecodable type, the encoder and
decoder would panic instead of returning an error. Fix this by
propagating type errors in makeStruct{Writer,Decoder} and add a test.

rlp/decode.go

@@ -55,81 +55,23 @@ var (
 	}
 )
 
-// Decoder is implemented by types that require custom RLP
-// decoding rules or need to decode into private fields.
+// Decoder is implemented by types that require custom RLP decoding rules or need to decode
+// into private fields.
 //
-// The DecodeRLP method should read one value from the given
-// Stream. It is not forbidden to read less or more, but it might
-// be confusing.
+// The DecodeRLP method should read one value from the given Stream. It is not forbidden to
+// read less or more, but it might be confusing.
 type Decoder interface {
 	DecodeRLP(*Stream) error
 }
 
-// Decode parses RLP-encoded data from r and stores the result in the
-// value pointed to by val. Val must be a non-nil pointer. If r does
-// not implement ByteReader, Decode will do its own buffering.
+// Decode parses RLP-encoded data from r and stores the result in the value pointed to by
+// val. Please see package-level documentation for the decoding rules. Val must be a
+// non-nil pointer.
 //
-// Decode uses the following type-dependent decoding rules:
+// If r does not implement ByteReader, Decode will do its own buffering.
 //
-// If the type implements the Decoder interface, decode calls
-// DecodeRLP.
-//
-// 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 be 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. 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 certain struct tags, "tail",
-// "nil" and "-".
-//
-// The "-" tag ignores fields.
-//
-// For an explanation of "tail", see the example.
-//
-// The "nil" 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. For byte slices,
-// 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.
-//
-// 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
-// the integer. If the RLP string is larger than the bit size of the
-// type, Decode will return an error. Decode also supports *big.Int.
-// There is no size limit for big integers.
-//
-// To decode into a boolean, the input must contain an unsigned integer
-// of value zero (false) or one (true).
-//
-// To decode into an interface value, Decode stores one of these
-// in the value:
-//
-//	  []interface{}, for RLP lists
-//	  []byte, for RLP strings
-//
-// Non-empty interface types are not supported, nor are 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
+// 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 {
@@ -140,9 +82,8 @@ func Decode(r io.Reader, val interface{}) error {
 	return stream.Decode(val)
 }
 
-// DecodeBytes parses RLP data from b into val.
-// Please see the documentation of Decode for the decoding rules.
-// The input must contain exactly one value and no trailing data.
+// DecodeBytes parses RLP data from b into val. Please see package-level documentation for
+// the decoding rules. The input must contain exactly one value and no trailing data.
 func DecodeBytes(b []byte, val interface{}) error {
 	r := bytes.NewReader(b)
 
@@ -211,14 +152,15 @@ func makeDecoder(typ reflect.Type, tags tags) (dec decoder, err error) {
 	switch {
 	case typ == rawValueType:
 		return decodeRawValue, nil
-	case typ.Implements(decoderInterface):
 		return decodeDecoder, nil
-	case kind != reflect.Ptr && reflect.PtrTo(typ).Implements(decoderInterface):
-		return decodeDecoderNoPtr, nil
 	case typ.AssignableTo(reflect.PtrTo(bigInt)):
 		return decodeBigInt, nil
 	case typ.AssignableTo(bigInt):
 		return decodeBigIntNoPtr, nil
+	case kind == reflect.Ptr:
+		return makePtrDecoder(typ, tags)
+	case reflect.PtrTo(typ).Implements(decoderInterface):
+		return decodeDecoder, nil
 	case isUint(kind):
 		return decodeUint, nil
 	case kind == reflect.Bool:
@@ -229,11 +171,6 @@ func makeDecoder(typ reflect.Type, tags tags) (dec decoder, err error) {
 		return makeListDecoder(typ, tags)
 	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
 	default:
@@ -448,6 +385,11 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) {
 	if err != nil {
 		return nil, err
 	}
+	for _, f := range fields {
+		if f.info.decoderErr != nil {
+			return nil, structFieldError{typ, f.index, f.info.decoderErr}
+		}
+	}
 	dec := func(s *Stream, val reflect.Value) (err error) {
 		if _, err := s.List(); err != nil {
 			return wrapStreamError(err, typ)
@@ -465,15 +407,22 @@ 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) {
+// makePtrDecoder creates a decoder that decodes into the pointer's element type.
+func makePtrDecoder(typ reflect.Type, tag tags) (decoder, error) {
 	etype := typ.Elem()
 	etypeinfo := cachedTypeInfo1(etype, tags{})
-	if etypeinfo.decoderErr != nil {
+	switch {
+	case etypeinfo.decoderErr != nil:
 		return nil, etypeinfo.decoderErr
+	case !tag.nilOK:
+		return makeSimplePtrDecoder(etype, etypeinfo), nil
+	default:
+		return makeNilPtrDecoder(etype, etypeinfo, tag.nilKind), nil
 	}
-	dec := func(s *Stream, val reflect.Value) (err error) {
+}
+
+func makeSimplePtrDecoder(etype reflect.Type, etypeinfo *typeinfo) decoder {
+	return func(s *Stream, val reflect.Value) (err error) {
 		newval := val
 		if val.IsNil() {
 			newval = reflect.New(etype)
@@ -483,30 +432,35 @@ func makePtrDecoder(typ reflect.Type) (decoder, error) {
 		}
 		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.
+// makeNilPtrDecoder 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 := cachedTypeInfo1(etype, tags{})
-	if etypeinfo.decoderErr != nil {
-		return nil, etypeinfo.decoderErr
-	}
-	dec := func(s *Stream, val reflect.Value) (err error) {
+func makeNilPtrDecoder(etype reflect.Type, etypeinfo *typeinfo, nilKind Kind) decoder {
+	typ := reflect.PtrTo(etype)
+	nilPtr := reflect.Zero(typ)
+	return func(s *Stream, val reflect.Value) (err error) {
 		kind, size, err := s.Kind()
-		if err != nil || size == 0 && kind != Byte {
+		if err != nil {
+			val.Set(nilPtr)
+			return wrapStreamError(err, typ)
+		}
+		// Handle empty values as a nil pointer.
+		if kind != Byte && size == 0 {
+			if kind != nilKind {
+				return &decodeError{
+					msg: fmt.Sprintf("wrong kind of empty value (got %v, want %v)", kind, nilKind),
+					typ: typ,
+				}
+			}
 			// rearm s.Kind. This is important because the input
 			// position must advance to the next value even though
 			// we don't read anything.
 			s.kind = -1
-			// set the pointer to nil.
-			val.Set(reflect.Zero(typ))
-			return err
+			val.Set(nilPtr)
+			return nil
 		}
 		newval := val
 		if val.IsNil() {
@@ -517,7 +471,6 @@ func makeOptionalPtrDecoder(typ reflect.Type) (decoder, error) {
 		}
 		return err
 	}
-	return dec, nil
 }
 
 var ifsliceType = reflect.TypeOf([]interface{}{})
@@ -546,21 +499,8 @@ func decodeInterface(s *Stream, val reflect.Value) error {
 	return nil
 }
 
-// This decoder is used for non-pointer values of types
-// that implement the Decoder interface using a pointer receiver.
-func decodeDecoderNoPtr(s *Stream, val reflect.Value) error {
-	return val.Addr().Interface().(Decoder).DecodeRLP(s)
-}
-
 func decodeDecoder(s *Stream, val reflect.Value) error {
-	// Decoder instances are not handled using the pointer rule if the type
-	// implements Decoder with pointer receiver (i.e. always)
-	// because it might handle empty values specially.
-	// We need to allocate one here in this case, like makePtrDecoder does.
-	if val.Kind() == reflect.Ptr && val.IsNil() {
-		val.Set(reflect.New(val.Type().Elem()))
-	}
-	return val.Interface().(Decoder).DecodeRLP(s)
+	return val.Addr().Interface().(Decoder).DecodeRLP(s)
 }
 
 // Kind represents the kind of value contained in an RLP stream.

rlp/decode_test.go

@@ -327,6 +327,10 @@ type recstruct struct {
 	Child *recstruct `rlp:"nil"`
 }
 
+type invalidNilTag struct {
+	X []byte `rlp:"nil"`
+}
+
 type invalidTail1 struct {
 	A uint `rlp:"tail"`
 	B string
@@ -353,6 +357,18 @@ type tailPrivateFields struct {
 	x, y bool
 }
 
+type nilListUint struct {
+	X *uint `rlp:"nilList"`
+}
+
+type nilStringSlice struct {
+	X *[]uint `rlp:"nilString"`
+}
+
+type intField struct {
+	X int
+}
+
 var (
 	veryBigInt = big.NewInt(0).Add(
 		big.NewInt(0).Lsh(big.NewInt(0xFFFFFFFFFFFFFF), 16),
@@ -485,20 +501,20 @@ var decodeTests = []decodeTest{
 		error: "rlp: expected input string or byte for uint, decoding into (rlp.recstruct).Child.I",
 	},
 	{
-		input: "C0",
-		ptr:   new(invalidTail1),
-		error: "rlp: invalid struct tag \"tail\" for rlp.invalidTail1.A (must be on last field)",
-	},
-	{
-		input: "C0",
-		ptr:   new(invalidTail2),
-		error: "rlp: invalid struct tag \"tail\" for rlp.invalidTail2.B (field type is not slice)",
+		input: "C103",
+		ptr:   new(intField),
+		error: "rlp: type int is not RLP-serializable (struct field rlp.intField.X)",
 	},
 	{
 		input: "C50102C20102",
 		ptr:   new(tailUint),
 		error: "rlp: expected input string or byte for uint, decoding into (rlp.tailUint).Tail[1]",
 	},
+	{
+		input: "C0",
+		ptr:   new(invalidNilTag),
+		error: `rlp: invalid struct tag "nil" for rlp.invalidNilTag.X (field is not a pointer)`,
+	},
 
 	// struct tag "tail"
 	{
@@ -521,6 +537,16 @@ var decodeTests = []decodeTest{
 		ptr:   new(tailPrivateFields),
 		value: tailPrivateFields{A: 1, Tail: []uint{2, 3}},
 	},
+	{
+		input: "C0",
+		ptr:   new(invalidTail1),
+		error: `rlp: invalid struct tag "tail" for rlp.invalidTail1.A (must be on last field)`,
+	},
+	{
+		input: "C0",
+		ptr:   new(invalidTail2),
+		error: `rlp: invalid struct tag "tail" for rlp.invalidTail2.B (field type is not slice)`,
+	},
 
 	// struct tag "-"
 	{
@@ -529,6 +555,43 @@ var decodeTests = []decodeTest{
 		value: hasIgnoredField{A: 1, C: 2},
 	},
 
+	// struct tag "nilList"
+	{
+		input: "C180",
+		ptr:   new(nilListUint),
+		error: "rlp: wrong kind of empty value (got String, want List) for *uint, decoding into (rlp.nilListUint).X",
+	},
+	{
+		input: "C1C0",
+		ptr:   new(nilListUint),
+		value: nilListUint{},
+	},
+	{
+		input: "C103",
+		ptr:   new(nilListUint),
+		value: func() interface{} {
+			v := uint(3)
+			return nilListUint{X: &v}
+		}(),
+	},
+
+	// struct tag "nilString"
+	{
+		input: "C1C0",
+		ptr:   new(nilStringSlice),
+		error: "rlp: wrong kind of empty value (got List, want String) for *[]uint, decoding into (rlp.nilStringSlice).X",
+	},
+	{
+		input: "C180",
+		ptr:   new(nilStringSlice),
+		value: nilStringSlice{},
+	},
+	{
+		input: "C2C103",
+		ptr:   new(nilStringSlice),
+		value: nilStringSlice{X: &[]uint{3}},
+	},
+
 	// RawValue
 	{input: "01", ptr: new(RawValue), value: RawValue(unhex("01"))},
 	{input: "82FFFF", ptr: new(RawValue), value: RawValue(unhex("82FFFF"))},
@@ -672,6 +735,22 @@ func TestDecodeDecoder(t *testing.T) {
 	}
 }
 
+func TestDecodeDecoderNilPointer(t *testing.T) {
+	var s struct {
+		T1 *testDecoder `rlp:"nil"`
+		T2 *testDecoder
+	}
+	if err := Decode(bytes.NewReader(unhex("C2C002")), &s); err != nil {
+		t.Fatalf("Decode error: %v", err)
+	}
+	if s.T1 != nil {
+		t.Errorf("decoder T1 allocated for empty input (called: %v)", s.T1.called)
+	}
+	if s.T2 == nil || !s.T2.called {
+		t.Errorf("decoder T2 not allocated/called")
+	}
+}
+
 type byteDecoder byte
 
 func (bd *byteDecoder) DecodeRLP(s *Stream) error {

rlp/doc.go

@@ -17,17 +17,114 @@
 /*
 Package rlp implements the RLP serialization format.
 
-The purpose of RLP (Recursive Linear Prefix) is to encode arbitrarily
-nested arrays of binary data, and RLP is the main encoding method used
-to serialize objects in Ethereum. The only purpose of RLP is to encode
-structure; encoding specific atomic data types (eg. strings, ints,
-floats) is left up to higher-order protocols; in Ethereum integers
-must be represented in big endian binary form with no leading zeroes
-(thus making the integer value zero equivalent to the empty byte
-array).
-
-RLP values are distinguished by a type tag. The type tag precedes the
-value in the input stream and defines the size and kind of the bytes
-that follow.
+The purpose of RLP (Recursive Linear Prefix) is to encode arbitrarily nested arrays of
+binary data, and RLP is the main encoding method used to serialize objects in Ethereum.
+The only purpose of RLP is to encode structure; encoding specific atomic data types (eg.
+strings, ints, floats) is left up to higher-order protocols. In Ethereum integers must be
+represented in big endian binary form with no leading zeroes (thus making the integer
+value zero equivalent to the empty string).
+
+RLP values are distinguished by a type tag. The type tag precedes the value in the input
+stream and defines the size and kind of the bytes that follow.
+
+
+Encoding Rules
+
+Package rlp uses reflection and encodes RLP based on the Go type of the value.
+
+If the type implements the Encoder interface, Encode calls EncodeRLP. It does not
+call EncodeRLP on nil pointer values.
+
+To encode a pointer, the value being pointed to is encoded. A nil pointer to a struct
+type, slice or array always encodes as an empty RLP list unless the slice or array has
+elememt type byte. A nil pointer to any other value encodes as the empty string.
+
+Struct values are encoded as an RLP list of all their encoded public fields. Recursive
+struct types are supported.
+
+To encode slices and arrays, the elements are encoded as an RLP list of the value's
+elements. Note that arrays and slices with element type uint8 or byte are always encoded
+as an RLP string.
+
+A Go string is encoded as an RLP string.
+
+An unsigned integer value is encoded as an RLP string. Zero always encodes as an empty RLP
+string. big.Int values are treated as integers. Signed integers (int, int8, int16, ...)
+are not supported and will return an error when encoding.
+
+Boolean values are encoded as the unsigned integers zero (false) and one (true).
+
+An interface value encodes as the value contained in the interface.
+
+Floating point numbers, maps, channels and functions are not supported.
+
+
+Decoding Rules
+
+Decoding uses the following type-dependent rules:
+
+If the type implements the Decoder interface, DecodeRLP is called.
+
+To decode into a pointer, the value will be decoded as the element type of the pointer. 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 be reused. Note that package rlp never leaves a
+pointer-type struct field as nil unless one of the "nil" struct tags is present.
+
+To decode into a struct, decoding 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. The input list must contain an element for each decoded field. Decoding
+returns an error if there are too few or too many elements for the struct.
+
+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. Array types
+decode similarly, with the additional restriction that the number of input elements (or
+bytes) must match the array's defined 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.
+
+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 the integer. If the RLP string is larger
+than the bit size of the type, decoding will return an error. Decode also supports
+*big.Int. There is no size limit for big integers.
+
+To decode into a boolean, the input must contain an unsigned integer of value zero (false)
+or one (true).
+
+To decode into an interface value, one of these types is stored in the value:
+
+	  []interface{}, for RLP lists
+	  []byte, for RLP strings
+
+Non-empty interface types are not supported when decoding.
+Signed integers, floating point numbers, maps, channels and functions cannot be decoded into.
+
+
+Struct Tags
+
+Package rlp honours certain struct tags: "-", "tail", "nil", "nilList" and "nilString".
+
+The "-" tag ignores fields.
+
+The "tail" tag, which may only be used on the last exported struct field, allows slurping
+up any excess list elements into a slice. See examples for more details.
+
+The "nil" 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 StructWithOptionalFoo struct {
+        Foo *[20]byte `rlp:"nil"`
+    }
+
+RLP supports two kinds of empty values: empty lists and empty strings. When using the
+"nil" tag, the kind of empty value allowed for a type is chosen automatically. A struct
+field whose Go type is a pointer to an unsigned integer, string, boolean or byte
+array/slice expects an empty RLP string. Any other pointer field type encodes/decodes as
+an empty RLP list.
+
+The choice of null value can be made explicit with the "nilList" and "nilString" struct
+tags. Using these tags encodes/decodes a Go nil pointer value as the kind of empty
+RLP value defined by the tag.
 */
 package rlp

rlp/encode.go

@@ -49,36 +49,7 @@ type Encoder interface {
 // perform many small writes in some cases. Consider making w
 // buffered.
 //
-// Encode uses the following type-dependent encoding rules:
-//
-// If the type implements the Encoder interface, Encode calls
-// EncodeRLP. This is true even for nil pointers, please see the
-// documentation for Encoder.
-//
-// To encode a pointer, the value being pointed to is encoded. For nil
-// pointers, Encode will encode the zero value of the type. A nil
-// pointer to a struct type always encodes as an empty RLP list.
-// A nil pointer to an array encodes as an empty list (or empty string
-// if the array has element type byte).
-//
-// Struct values are encoded as an RLP list of all their encoded
-// public fields. Recursive struct types are supported.
-//
-// To encode slices and arrays, the elements are encoded as an RLP
-// list of the value's elements. Note that arrays and slices with
-// element type uint8 or byte are always encoded as an RLP string.
-//
-// A Go string is encoded as an RLP string.
-//
-// An unsigned integer value is encoded as an RLP string. Zero always
-// encodes as an empty RLP string. Encode also supports *big.Int.
-//
-// Boolean values are encoded as unsigned integers zero (false) and one (true).
-//
-// An interface value encodes as the value contained in the interface.
-//
-// Signed integers are not supported, nor are floating point numbers, maps,
-// channels and functions.
+// Please see package-level documentation of encoding rules.
 func Encode(w io.Writer, val interface{}) error {
 	if outer, ok := w.(*encbuf); ok {
 		// Encode was called by some type's EncodeRLP.
@@ -95,7 +66,7 @@ func Encode(w io.Writer, val interface{}) error {
 }
 
 // EncodeToBytes returns the RLP encoding of val.
-// Please see the documentation of Encode for the encoding rules.
+// Please see package-level documentation for the encoding rules.
 func EncodeToBytes(val interface{}) ([]byte, error) {
 	eb := encbufPool.Get().(*encbuf)
 	defer encbufPool.Put(eb)
@@ -349,16 +320,14 @@ func makeWriter(typ reflect.Type, ts tags) (writer, error) {
 	switch {
 	case typ == rawValueType:
 		return writeRawValue, nil
-	case typ.Implements(encoderInterface):
-		return writeEncoder, nil
-	case kind != reflect.Ptr && reflect.PtrTo(typ).Implements(encoderInterface):
-		return writeEncoderNoPtr, nil
-	case kind == reflect.Interface:
-		return writeInterface, nil
 	case typ.AssignableTo(reflect.PtrTo(bigInt)):
 		return writeBigIntPtr, nil
 	case typ.AssignableTo(bigInt):
 		return writeBigIntNoPtr, nil
+	case kind == reflect.Ptr:
+		return makePtrWriter(typ, ts)
+	case reflect.PtrTo(typ).Implements(encoderInterface):
+		return makeEncoderWriter(typ), nil
 	case isUint(kind):
 		return writeUint, nil
 	case kind == reflect.Bool:
@@ -373,8 +342,8 @@ func makeWriter(typ reflect.Type, ts tags) (writer, error) {
 		return makeSliceWriter(typ, ts)
 	case kind == reflect.Struct:
 		return makeStructWriter(typ)
-	case kind == reflect.Ptr:
-		return makePtrWriter(typ)
+	case kind == reflect.Interface:
+		return writeInterface, nil
 	default:
 		return nil, fmt.Errorf("rlp: type %v is not RLP-serializable", typ)
 	}
@@ -470,26 +439,6 @@ func writeString(val reflect.Value, w *encbuf) error {
 	return nil
 }
 
-func writeEncoder(val reflect.Value, w *encbuf) error {
-	return val.Interface().(Encoder).EncodeRLP(w)
-}
-
-// writeEncoderNoPtr handles non-pointer values that implement Encoder
-// with a pointer receiver.
-func writeEncoderNoPtr(val reflect.Value, w *encbuf) error {
-	if !val.CanAddr() {
-		// We can't get the address. It would be possible to make the
-		// value addressable by creating a shallow copy, but this
-		// creates other problems so we're not doing it (yet).
-		//
-		// package json simply doesn't call MarshalJSON for cases like
-		// this, but encodes the value as if it didn't implement the
-		// interface. We don't want to handle it that way.
-		return fmt.Errorf("rlp: game over: unadressable value of type %v, EncodeRLP is pointer method", val.Type())
-	}
-	return val.Addr().Interface().(Encoder).EncodeRLP(w)
-}
-
 func writeInterface(val reflect.Value, w *encbuf) error {
 	if val.IsNil() {
 		// Write empty list. This is consistent with the previous RLP
@@ -531,6 +480,11 @@ func makeStructWriter(typ reflect.Type) (writer, error) {
 	if err != nil {
 		return nil, err
 	}
+	for _, f := range fields {
+		if f.info.writerErr != nil {
+			return nil, structFieldError{typ, f.index, f.info.writerErr}
+		}
+	}
 	writer := func(val reflect.Value, w *encbuf) error {
 		lh := w.list()
 		for _, f := range fields {
@@ -544,44 +498,51 @@ func makeStructWriter(typ reflect.Type) (writer, error) {
 	return writer, nil
 }
 
-func makePtrWriter(typ reflect.Type) (writer, error) {
+func makePtrWriter(typ reflect.Type, ts tags) (writer, error) {
 	etypeinfo := cachedTypeInfo1(typ.Elem(), tags{})
 	if etypeinfo.writerErr != nil {
 		return nil, etypeinfo.writerErr
 	}
-
-	// determine nil pointer handler
-	var nilfunc func(*encbuf) error
-	kind := typ.Elem().Kind()
-	switch {
-	case kind == reflect.Array && isByte(typ.Elem().Elem()):
-		nilfunc = func(w *encbuf) error {
-			w.str = append(w.str, 0x80)
-			return nil
-		}
-	case kind == reflect.Struct || kind == reflect.Array:
-		nilfunc = func(w *encbuf) error {
-			// encoding the zero value of a struct/array could trigger
-			// infinite recursion, avoid that.
-			w.listEnd(w.list())
-			return nil
-		}
-	default:
-		zero := reflect.Zero(typ.Elem())
-		nilfunc = func(w *encbuf) error {
-			return etypeinfo.writer(zero, w)
-		}
+	// Determine how to encode nil pointers.
+	var nilKind Kind
+	if ts.nilOK {
+		nilKind = ts.nilKind // use struct tag if provided
+	} else {
+		nilKind = defaultNilKind(typ.Elem())
 	}
 
 	writer := func(val reflect.Value, w *encbuf) error {
 		if val.IsNil() {
-			return nilfunc(w)
+			if nilKind == String {
+				w.str = append(w.str, 0x80)
+			} else {
+				w.listEnd(w.list())
+			}
+			return nil
 		}
 		return etypeinfo.writer(val.Elem(), w)
 	}
 	return writer, nil
 }
 
+func makeEncoderWriter(typ reflect.Type) writer {
+	if typ.Implements(encoderInterface) {
+		return func(val reflect.Value, w *encbuf) error {
+			return val.Interface().(Encoder).EncodeRLP(w)
+		}
+	}
+	w := func(val reflect.Value, w *encbuf) error {
+		if !val.CanAddr() {
+			// package json simply doesn't call MarshalJSON for this case, but encodes the
+			// value as if it didn't implement the interface. We don't want to handle it that
+			// way.
+			return fmt.Errorf("rlp: unadressable value of type %v, EncodeRLP is pointer method", val.Type())
+		}
+		return val.Addr().Interface().(Encoder).EncodeRLP(w)
+	}
+	return w
+}
+
 // putint writes i to the beginning of b in big endian byte
 // order, using the least number of bytes needed to represent i.
 func putint(b []byte, i uint64) (size int) {

rlp/encode_test.go

@@ -33,8 +33,9 @@ type testEncoder struct {
 
 func (e *testEncoder) EncodeRLP(w io.Writer) error {
 	if e == nil {
-		w.Write([]byte{0, 0, 0, 0})
-	} else if e.err != nil {
+		panic("EncodeRLP called on nil value")
+	}
+	if e.err != nil {
 		return e.err
 	} else {
 		w.Write([]byte{0, 1, 0, 1, 0, 1, 0, 1, 0, 1})
@@ -42,6 +43,13 @@ func (e *testEncoder) EncodeRLP(w io.Writer) error {
 	return nil
 }
 
+type testEncoderValueMethod struct{}
+
+func (e testEncoderValueMethod) EncodeRLP(w io.Writer) error {
+	w.Write([]byte{0xFA, 0xFE, 0xF0})
+	return nil
+}
+
 type byteEncoder byte
 
 func (e byteEncoder) EncodeRLP(w io.Writer) error {
@@ -52,8 +60,8 @@ func (e byteEncoder) EncodeRLP(w io.Writer) error {
 type undecodableEncoder func()
 
 func (f undecodableEncoder) EncodeRLP(w io.Writer) error {
-	_, err := w.Write(EmptyList)
-	return err
+	w.Write([]byte{0xF5, 0xF5, 0xF5})
+	return nil
 }
 
 type encodableReader struct {
@@ -226,6 +234,7 @@ var encTests = []encTest{
 	{val: &tailRaw{A: 1, Tail: []RawValue{}}, output: "C101"},
 	{val: &tailRaw{A: 1, Tail: nil}, output: "C101"},
 	{val: &hasIgnoredField{A: 1, B: 2, C: 3}, output: "C20103"},
+	{val: &intField{X: 3}, error: "rlp: type int is not RLP-serializable (struct field rlp.intField.X)"},
 
 	// nil
 	{val: (*uint)(nil), output: "80"},
@@ -239,22 +248,66 @@ var encTests = []encTest{
 	{val: (*[]struct{ uint })(nil), output: "C0"},
 	{val: (*interface{})(nil), output: "C0"},
 
+	// nil struct fields
+	{
+		val: struct {
+			X *[]byte
+		}{},
+		output: "C180",
+	},
+	{
+		val: struct {
+			X *[2]byte
+		}{},
+		output: "C180",
+	},
+	{
+		val: struct {
+			X *uint64
+		}{},
+		output: "C180",
+	},
+	{
+		val: struct {
+			X *uint64 `rlp:"nilList"`
+		}{},
+		output: "C1C0",
+	},
+	{
+		val: struct {
+			X *[]uint64
+		}{},
+		output: "C1C0",
+	},
+	{
+		val: struct {
+			X *[]uint64 `rlp:"nilString"`
+		}{},
+		output: "C180",
+	},
+
 	// interfaces
 	{val: []io.Reader{reader}, output: "C3C20102"}, // the contained value is a struct
 
 	// Encoder
-	{val: (*testEncoder)(nil), output: "00000000"},
+	{val: (*testEncoder)(nil), output: "C0"},
 	{val: &testEncoder{}, output: "00010001000100010001"},
 	{val: &testEncoder{errors.New("test error")}, error: "test error"},
-	// verify that the Encoder interface works for unsupported types like func().
-	{val: undecodableEncoder(func() {}), output: "C0"},
-	// verify that pointer method testEncoder.EncodeRLP is called for
+	{val: struct{ E testEncoderValueMethod }{}, output: "C3FAFEF0"},
+	{val: struct{ E *testEncoderValueMethod }{}, output: "C1C0"},
+
+	// Verify that the Encoder interface works for unsupported types like func().
+	{val: undecodableEncoder(func() {}), output: "F5F5F5"},
+
+	// Verify that pointer method testEncoder.EncodeRLP is called for
 	// addressable non-pointer values.
 	{val: &struct{ TE testEncoder }{testEncoder{}}, output: "CA00010001000100010001"},
 	{val: &struct{ TE testEncoder }{testEncoder{errors.New("test error")}}, error: "test error"},
-	// verify the error for non-addressable non-pointer Encoder
-	{val: testEncoder{}, error: "rlp: game over: unadressable value of type rlp.testEncoder, EncodeRLP is pointer method"},
-	// verify the special case for []byte
+
+	// Verify the error for non-addressable non-pointer Encoder.
+	{val: testEncoder{}, error: "rlp: unadressable value of type rlp.testEncoder, EncodeRLP is pointer method"},
+
+	// Verify Encoder takes precedence over []byte.
 	{val: []byteEncoder{0, 1, 2, 3, 4}, output: "C5C0C0C0C0C0"},
 }
 

rlp/encoder_example_test.go

@@ -28,15 +28,7 @@ type MyCoolType struct {
 
 // EncodeRLP writes x as RLP list [a, b] that omits the Name field.
 func (x *MyCoolType) EncodeRLP(w io.Writer) (err error) {
-	// Note: the receiver can be a nil pointer. This allows you to
-	// control the encoding of nil, but it also means that you have to
-	// check for a nil receiver.
-	if x == nil {
-		err = Encode(w, []uint{0, 0})
-	} else {
-		err = Encode(w, []uint{x.a, x.b})
-	}
-	return err
+	return Encode(w, []uint{x.a, x.b})
 }
 
 func ExampleEncoder() {
@@ -49,6 +41,6 @@ func ExampleEncoder() {
 	fmt.Printf("%v → %X\n", t, bytes)
 
 	// Output:
-	// <nil> → C28080
+	// <nil> → C0
 	// &{foobar 5 6} → C20506
 }

rlp/typecache.go

@@ -35,22 +35,28 @@ type typeinfo struct {
 	writerErr  error // error from makeWriter
 }
 
-// represents struct tags
+// tags represents struct tags.
 type tags struct {
 	// rlp:"nil" controls whether empty input results in a nil pointer.
 	nilOK bool
+
+	// This controls whether nil pointers are encoded/decoded as empty strings
+	// or empty lists.
+	nilKind Kind
+
 	// rlp:"tail" controls whether this field swallows additional list
 	// elements. It can only be set for the last field, which must be
 	// of slice type.
 	tail bool
+
 	// rlp:"-" ignores fields.
 	ignored bool
 }
 
+// typekey is the key of a type in typeCache. It includes the struct tags because
+// they might generate a different decoder.
 type typekey struct {
 	reflect.Type
-	// the key must include the struct tags because they
-	// might generate a different decoder.
 	tags
 }
 
@@ -120,6 +126,25 @@ func structFields(typ reflect.Type) (fields []field, err error) {
 	return fields, nil
 }
 
+type structFieldError struct {
+	typ   reflect.Type
+	field int
+	err   error
+}
+
+func (e structFieldError) Error() string {
+	return fmt.Sprintf("%v (struct field %v.%s)", e.err, e.typ, e.typ.Field(e.field).Name)
+}
+
+type structTagError struct {
+	typ             reflect.Type
+	field, tag, err string
+}
+
+func (e structTagError) Error() string {
+	return fmt.Sprintf("rlp: invalid struct tag %q for %v.%s (%s)", e.tag, e.typ, e.field, e.err)
+}
+
 func parseStructTag(typ reflect.Type, fi, lastPublic int) (tags, error) {
 	f := typ.Field(fi)
 	var ts tags
@@ -128,15 +153,26 @@ func parseStructTag(typ reflect.Type, fi, lastPublic int) (tags, error) {
 		case "":
 		case "-":
 			ts.ignored = true
-		case "nil":
+		case "nil", "nilString", "nilList":
 			ts.nilOK = true
+			if f.Type.Kind() != reflect.Ptr {
+				return ts, structTagError{typ, f.Name, t, "field is not a pointer"}
+			}
+			switch t {
+			case "nil":
+				ts.nilKind = defaultNilKind(f.Type.Elem())
+			case "nilString":
+				ts.nilKind = String
+			case "nilList":
+				ts.nilKind = List
+			}
 		case "tail":
 			ts.tail = true
 			if fi != lastPublic {
-				return ts, fmt.Errorf(`rlp: invalid struct tag "tail" for %v.%s (must be on last field)`, typ, f.Name)
+				return ts, structTagError{typ, f.Name, t, "must be on last field"}
 			}
 			if f.Type.Kind() != reflect.Slice {
-				return ts, fmt.Errorf(`rlp: invalid struct tag "tail" for %v.%s (field type is not slice)`, typ, f.Name)
+				return ts, structTagError{typ, f.Name, t, "field type is not slice"}
 			}
 		default:
 			return ts, fmt.Errorf("rlp: unknown struct tag %q on %v.%s", t, typ, f.Name)
@@ -160,6 +196,20 @@ func (i *typeinfo) generate(typ reflect.Type, tags tags) {
 	i.writer, i.writerErr = makeWriter(typ, tags)
 }
 
+// defaultNilKind determines whether a nil pointer to typ encodes/decodes
+// as an empty string or empty list.
+func defaultNilKind(typ reflect.Type) Kind {
+	k := typ.Kind()
+	if isUint(k) || k == reflect.String || k == reflect.Bool || isByteArray(typ) {
+		return String
+	}
+	return List
+}
+
 func isUint(k reflect.Kind) bool {
 	return k >= reflect.Uint && k <= reflect.Uintptr
 }
+
+func isByteArray(typ reflect.Type) bool {
+	return (typ.Kind() == reflect.Slice || typ.Kind() == reflect.Array) && isByte(typ.Elem())
+}