return errors when trying to rlp encode nil pointers that shouldn't b…

…e nil

rlp/decode.go

@@ -424,13 +424,14 @@ func makePtrDecoder(typ reflect.Type, tag rlpstruct.Tags) (decoder, error) {
 	switch {
 	case etypeinfo.decoderErr != nil:
 		return nil, etypeinfo.decoderErr
-	case !tag.NilOK:
+	case tag.NilKind == rlpstruct.NilKind(0):
 		return makeSimplePtrDecoder(etype, etypeinfo), nil
 	default:
 		return makeNilPtrDecoder(etype, etypeinfo, tag), nil
 	}
 }
 
+// makeSimplePtrDecoder creates a decoder for pointer types that should not be nil
 func makeSimplePtrDecoder(etype reflect.Type, etypeinfo *typeinfo) decoder {
 	return func(s *Stream, val reflect.Value) (err error) {
 		newval := val
@@ -445,9 +446,10 @@ func makeSimplePtrDecoder(etype reflect.Type, etypeinfo *typeinfo) decoder {
 }
 
 // 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.
+// values are decoded into a value of the element type, just like makeSimplePtrDecoder
+// does.
 //
-// This decoder is used for pointer-typed struct fields with struct tag "nil".
+// This decoder is used for pointer-typed struct fields with a rlp:"nil*" tag.
 func makeNilPtrDecoder(etype reflect.Type, etypeinfo *typeinfo, ts rlpstruct.Tags) decoder {
 	typ := reflect.PtrTo(etype)
 	nilPtr := reflect.Zero(typ)
@@ -476,14 +478,7 @@ func makeNilPtrDecoder(etype reflect.Type, etypeinfo *typeinfo, ts rlpstruct.Tag
 			val.Set(nilPtr)
 			return nil
 		}
-		newval := val
-		if val.IsNil() {
-			newval = reflect.New(etype)
-		}
-		if err = etypeinfo.decoder(s, newval.Elem()); err == nil {
-			val.Set(newval)
-		}
-		return err
+		return makeSimplePtrDecoder(etype, etypeinfo)(s, val)
 	}
 }
 

rlp/encode.go

@@ -367,9 +367,13 @@ func makeStructWriter(typ reflect.Type) (writer, error) {
 }
 
 func makePtrWriter(typ reflect.Type, ts rlpstruct.Tags) (writer, error) {
-	nilEncoding := byte(0xC0)
-	if typeNilKind(typ.Elem(), ts) == String {
-		nilEncoding = 0x80
+	nilEncoding := byte(0x0)
+	if ts.NilOK {
+		if typeNilKind(typ.Elem(), ts) == String {
+			nilEncoding = 0x80
+		} else {
+			nilEncoding = 0xC0
+		}
 	}
 
 	etypeinfo := theTC.infoWhileGenerating(typ.Elem(), rlpstruct.Tags{})
@@ -381,6 +385,9 @@ func makePtrWriter(typ reflect.Type, ts rlpstruct.Tags) (writer, error) {
 		if ev := val.Elem(); ev.IsValid() {
 			return etypeinfo.writer(ev, w)
 		}
+		if nilEncoding == 0x0 {
+			return fmt.Errorf("rlp: ptr value of type %v should not be nil", typ)
+		}
 		w.str = append(w.str, nilEncoding)
 		return nil
 	}

rlp/encode_test.go

@@ -290,9 +290,9 @@ var encTests = []encTest{
 	{val: &optionalBigIntField{A: 1}, output: "C101"},
 	{val: &optionalPtrField{A: 1}, output: "C101"},
 	{val: &optionalPtrFieldNil{A: 1}, output: "C101"},
-	{val: &nonOptionalPtrField{A: 1}, output: "C20180"}, // encodes without error but decode fails -- should encode return error?
+	{val: &nonOptionalPtrField{A: 1}, error: "rlp: ptr value of type *[3]uint8 should not be nil"},
 	{val: &multipleOptionalFields{A: &[3]byte{1, 2, 3}, B: &[3]byte{1, 2, 3}}, output: "C88301020383010203"},
-	{val: &multipleOptionalFields{A: nil, B: &[3]byte{1, 2, 3}}, output: "C58083010203"}, // encodes without error but decode fails -- should encode return error?
+	{val: &multipleOptionalFields{A: nil, B: &[3]byte{1, 2, 3}}, error: "rlp: ptr value of type *[3]uint8 should not be nil"},
 	{val: &multipleOptionalFields{A: nil, B: nil}, output: "C0"},
 
 	// nil
@@ -307,34 +307,46 @@ var encTests = []encTest{
 	{val: (*[]struct{ uint })(nil), output: "C0"},
 	{val: (*interface{})(nil), output: "C0"},
 
-	// nil struct fields
+	// nilable struct fields
+	{
+		val: struct {
+			X *[]byte `rlp:"nil"`
+		}{},
+		output: "C180",
+	},
 	{
 		val: struct {
 			X *[]byte
 		}{},
+		error: "rlp: ptr value of type *[]uint8 should not be nil",
+	},
+	{
+		val: struct {
+			X *[2]byte `rlp:"nil"`
+		}{},
 		output: "C180",
 	},
 	{
 		val: struct {
 			X *[2]byte
 		}{},
-		output: "C180",
+		error: "rlp: ptr value of type *[2]uint8 should not be nil",
 	},
 	{
 		val: struct {
-			X *uint64
+			X *uint64 `rlp:"nil"`
 		}{},
 		output: "C180",
 	},
 	{
 		val: struct {
-			X *uint64 `rlp:"nilList"`
+			X *uint64
 		}{},
-		output: "C1C0",
+		error: "rlp: ptr value of type *uint64 should not be nil",
 	},
 	{
 		val: struct {
-			X *[]uint64
+			X *[]uint64 `rlp:"nil"`
 		}{},
 		output: "C1C0",
 	},
@@ -353,8 +365,13 @@ var encTests = []encTest{
 	{val: &testEncoder{}, output: "00010001000100010001"},
 	{val: &testEncoder{errors.New("test error")}, error: "test error"},
 	{val: struct{ E testEncoderValueMethod }{}, output: "C3FAFEF0"},
-	{val: struct{ E *testEncoderValueMethod }{}, output: "C1C0"},
-
+	{val: struct{ E *testEncoderValueMethod }{}, error: "rlp: ptr value of type *rlp.testEncoderValueMethod should not be nil"},
+	{
+		val: struct {
+			E *testEncoderValueMethod `rlp:"nil"`
+		}{},
+		output: "C1C0",
+	},
 	// Verify that the Encoder interface works for unsupported types like func().
 	{val: undecodableEncoder(func() {}), output: "F5F5F5"},
 

rlp/internal/rlpstruct/rlpstruct.go

@@ -64,10 +64,12 @@ const (
 
 // Tags represents struct tags.
 type Tags struct {
-	// rlp:"nil" controls whether empty input results in a nil pointer.
-	// nilKind is the kind of empty value allowed for the field.
+	// NilOK is true for struct fields that were tagged with rlp:"nil*", and for
+	// all top-level pointers (which cannot have tags).
+	NilOK bool
+	// NilKind is non-zero for struct fields that were tagged with rlp:"nil*", and
+	// specifies the kind of empty value allowed for the field.
 	NilKind NilKind
-	NilOK   bool
 
 	// rlp:"optional" allows for a field to be missing in the input list.
 	// If this is set, all subsequent fields must also be optional.

rlp/typecache.go

@@ -44,8 +44,6 @@ type decoder func(*Stream, reflect.Value) error
 
 type writer func(reflect.Value, *encBuffer) error
 
-var theTC = newTypeCache()
-
 type typeCache struct {
 	cur atomic.Value
 
@@ -54,6 +52,11 @@ type typeCache struct {
 	next map[typekey]*typeinfo
 }
 
+var (
+	theTC    = newTypeCache()
+	rootTags = rlpstruct.Tags{NilOK: true} // tags value to use for top level calls
+)
+
 func newTypeCache() *typeCache {
 	c := new(typeCache)
 	c.cur.Store(make(map[typekey]*typeinfo))
@@ -71,13 +74,13 @@ func cachedWriter(typ reflect.Type) (writer, error) {
 }
 
 func (c *typeCache) info(typ reflect.Type) *typeinfo {
-	key := typekey{Type: typ}
+	key := typekey{typ, rootTags}
 	if info := c.cur.Load().(map[typekey]*typeinfo)[key]; info != nil {
 		return info
 	}
 
 	// Not in the cache, need to generate info for this type.
-	return c.generate(typ, rlpstruct.Tags{})
+	return c.generate(typ, rootTags)
 }
 
 func (c *typeCache) generate(typ reflect.Type, tags rlpstruct.Tags) *typeinfo {
@@ -213,13 +216,12 @@ func rtypeToStructType(typ reflect.Type, rec map[reflect.Type]*rlpstruct.Type) *
 
 // typeNilKind gives the RLP value kind for nil pointers to 'typ'.
 func typeNilKind(typ reflect.Type, tags rlpstruct.Tags) Kind {
-	styp := rtypeToStructType(typ, nil)
-
 	var nk rlpstruct.NilKind
-	if tags.NilOK {
+	if tags.NilKind != rlpstruct.NilKind(0) {
+		// an rlp:"nil*" tag was specified for this field, so always use that value.
 		nk = tags.NilKind
 	} else {
-		nk = styp.DefaultNilValue()
+		nk = rtypeToStructType(typ, nil).DefaultNilValue()
 	}
 	switch nk {
 	case rlpstruct.NilKindString: