From 700df1442d714cb3c42a602c39c042ce88be463f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 14:37:13 +0200 Subject: [PATCH] rlp: add support for optional struct fields (#22832) This adds support for a new struct tag "optional". Using this tag, structs used for RLP encoding/decoding can be extended in a backwards-compatible way, by adding new fields at the end. --- rlp/decode.go | 18 ++++- rlp/decode_test.go | 181 +++++++++++++++++++++++++++++++++++++++++++-- rlp/doc.go | 67 ++++++++++++----- rlp/encode.go | 39 ++++++++-- rlp/encode_test.go | 22 +++++- rlp/typecache.go | 54 +++++++++++--- 6 files changed, 333 insertions(+), 48 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 79b7ef062..b340aa029 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -229,7 +229,7 @@ func decodeBigInt(s *Stream, val reflect.Value) error { i = new(big.Int) val.Set(reflect.ValueOf(i)) } - // Reject leading zero bytes + // Reject leading zero bytes. if len(b) > 0 && b[0] == 0 { return wrapStreamError(ErrCanonInt, val.Type()) } @@ -394,9 +394,16 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) { if _, err := s.List(); err != nil { return wrapStreamError(err, typ) } - for _, f := range fields { + for i, f := range fields { err := f.info.decoder(s, val.Field(f.index)) if err == EOL { + if f.optional { + // The field is optional, so reaching the end of the list before + // reaching the last field is acceptable. All remaining undecoded + // fields are zeroed. + zeroFields(val, fields[i:]) + break + } return &decodeError{msg: "too few elements", typ: typ} } else if err != nil { return addErrorContext(err, "."+typ.Field(f.index).Name) @@ -407,6 +414,13 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) { return dec, nil } +func zeroFields(structval reflect.Value, fields []field) { + for _, f := range fields { + fv := structval.Field(f.index) + fv.Set(reflect.Zero(fv.Type())) + } +} + // makePtrDecoder creates a decoder that decodes into the pointer's element type. func makePtrDecoder(typ reflect.Type, tag tags) (decoder, error) { etype := typ.Elem() diff --git a/rlp/decode_test.go b/rlp/decode_test.go index d94c3969b..87a330633 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -369,6 +369,39 @@ type intField struct { X int } +type optionalFields struct { + A uint + B uint `rlp:"optional"` + C uint `rlp:"optional"` +} + +type optionalAndTailField struct { + A uint + B uint `rlp:"optional"` + Tail []uint `rlp:"tail"` +} + +type optionalBigIntField struct { + A uint + B *big.Int `rlp:"optional"` +} + +type optionalPtrField struct { + A uint + B *[3]byte `rlp:"optional"` +} + +type optionalPtrFieldNil struct { + A uint + B *[3]byte `rlp:"optional,nil"` +} + +type ignoredField struct { + A uint + B uint `rlp:"-"` + C uint +} + var ( veryBigInt = big.NewInt(0).Add( big.NewInt(0).Lsh(big.NewInt(0xFFFFFFFFFFFFFF), 16), @@ -376,12 +409,6 @@ var ( ) ) -type hasIgnoredField struct { - A uint - B uint `rlp:"-"` - C uint -} - var decodeTests = []decodeTest{ // booleans {input: "01", ptr: new(bool), value: true}, @@ -551,8 +578,8 @@ var decodeTests = []decodeTest{ // struct tag "-" { input: "C20102", - ptr: new(hasIgnoredField), - value: hasIgnoredField{A: 1, C: 2}, + ptr: new(ignoredField), + value: ignoredField{A: 1, C: 2}, }, // struct tag "nilList" @@ -592,6 +619,110 @@ var decodeTests = []decodeTest{ value: nilStringSlice{X: &[]uint{3}}, }, + // struct tag "optional" + { + input: "C101", + ptr: new(optionalFields), + value: optionalFields{1, 0, 0}, + }, + { + input: "C20102", + ptr: new(optionalFields), + value: optionalFields{1, 2, 0}, + }, + { + input: "C3010203", + ptr: new(optionalFields), + value: optionalFields{1, 2, 3}, + }, + { + input: "C401020304", + ptr: new(optionalFields), + error: "rlp: input list has too many elements for rlp.optionalFields", + }, + { + input: "C101", + ptr: new(optionalAndTailField), + value: optionalAndTailField{A: 1}, + }, + { + input: "C20102", + ptr: new(optionalAndTailField), + value: optionalAndTailField{A: 1, B: 2, Tail: []uint{}}, + }, + { + input: "C401020304", + ptr: new(optionalAndTailField), + value: optionalAndTailField{A: 1, B: 2, Tail: []uint{3, 4}}, + }, + { + input: "C101", + ptr: new(optionalBigIntField), + value: optionalBigIntField{A: 1, B: nil}, + }, + { + input: "C20102", + ptr: new(optionalBigIntField), + value: optionalBigIntField{A: 1, B: big.NewInt(2)}, + }, + { + input: "C101", + ptr: new(optionalPtrField), + value: optionalPtrField{A: 1}, + }, + { + input: "C20180", // not accepted because "optional" doesn't enable "nil" + ptr: new(optionalPtrField), + error: "rlp: input string too short for [3]uint8, decoding into (rlp.optionalPtrField).B", + }, + { + input: "C20102", + ptr: new(optionalPtrField), + error: "rlp: input string too short for [3]uint8, decoding into (rlp.optionalPtrField).B", + }, + { + input: "C50183010203", + ptr: new(optionalPtrField), + value: optionalPtrField{A: 1, B: &[3]byte{1, 2, 3}}, + }, + { + input: "C101", + ptr: new(optionalPtrFieldNil), + value: optionalPtrFieldNil{A: 1}, + }, + { + input: "C20180", // accepted because "nil" tag allows empty input + ptr: new(optionalPtrFieldNil), + value: optionalPtrFieldNil{A: 1}, + }, + { + input: "C20102", + ptr: new(optionalPtrFieldNil), + error: "rlp: input string too short for [3]uint8, decoding into (rlp.optionalPtrFieldNil).B", + }, + + // struct tag "optional" field clearing + { + input: "C101", + ptr: &optionalFields{A: 9, B: 8, C: 7}, + value: optionalFields{A: 1, B: 0, C: 0}, + }, + { + input: "C20102", + ptr: &optionalFields{A: 9, B: 8, C: 7}, + value: optionalFields{A: 1, B: 2, C: 0}, + }, + { + input: "C20102", + ptr: &optionalAndTailField{A: 9, B: 8, Tail: []uint{7, 6, 5}}, + value: optionalAndTailField{A: 1, B: 2, Tail: []uint{}}, + }, + { + input: "C101", + ptr: &optionalPtrField{A: 9, B: &[3]byte{8, 7, 6}}, + value: optionalPtrField{A: 1}, + }, + // RawValue {input: "01", ptr: new(RawValue), value: RawValue(unhex("01"))}, {input: "82FFFF", ptr: new(RawValue), value: RawValue(unhex("82FFFF"))}, @@ -822,6 +953,40 @@ func TestDecoderFunc(t *testing.T) { x() } +// This tests the validity checks for fields with struct tag "optional". +func TestInvalidOptionalField(t *testing.T) { + type ( + invalid1 struct { + A uint `rlp:"optional"` + B uint + } + invalid2 struct { + T []uint `rlp:"tail,optional"` + } + invalid3 struct { + T []uint `rlp:"optional,tail"` + } + ) + + tests := []struct { + v interface{} + err string + }{ + {v: new(invalid1), err: `rlp: struct field rlp.invalid1.B needs "optional" tag`}, + {v: new(invalid2), err: `rlp: invalid struct tag "optional" for rlp.invalid2.T (also has "tail" tag)`}, + {v: new(invalid3), err: `rlp: invalid struct tag "tail" for rlp.invalid3.T (also has "optional" tag)`}, + } + for _, test := range tests { + err := DecodeBytes(unhex("C20102"), test.v) + if err == nil { + t.Errorf("no error for %T", test.v) + } else if err.Error() != test.err { + t.Errorf("wrong error for %T: %v", test.v, err.Error()) + } + } + +} + func ExampleDecode() { input, _ := hex.DecodeString("C90A1486666F6F626172") diff --git a/rlp/doc.go b/rlp/doc.go index 7e6ee8520..113828e39 100644 --- a/rlp/doc.go +++ b/rlp/doc.go @@ -102,29 +102,60 @@ Signed integers, floating point numbers, maps, channels and functions cannot be Struct Tags -Package rlp honours certain struct tags: "-", "tail", "nil", "nilList" and "nilString". +As with other encoding packages, the "-" tag ignores fields. -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"` + type StructWithIgnoredField struct{ + Ignored uint `rlp:"-"` + Field uint } +Go struct values encode/decode as RLP lists. There are two ways of influencing the mapping +of fields to list elements. The "tail" tag, which may only be used on the last exported +struct field, allows slurping up any excess list elements into a slice. + + type StructWithTail struct{ + Field uint + Tail []string `rlp:"tail"` + } + +The "optional" tag says that the field may be omitted if it is zero-valued. If this tag is +used on a struct field, all subsequent public fields must also be declared optional. + +When encoding a struct with optional fields, the output RLP list contains all values up to +the last non-zero optional field. + +When decoding into a struct, optional fields may be omitted from the end of the input +list. For the example below, this means input lists of one, two, or three elements are +accepted. + + type StructWithOptionalFields struct{ + Required uint + Optional1 uint `rlp:"optional"` + Optional2 uint `rlp:"optional"` + } + +The "nil", "nilList" and "nilString" tags apply to pointer-typed fields only, and change +the decoding rules for the field type. For regular pointer fields without the "nil" tag, +input values must always match the required input length exactly and the decoder does not +produce nil values. When the "nil" tag is set, input values of size zero decode as a nil +pointer. This is especially useful for recursive types. + + type StructWithNilField struct { + Field *[3]byte `rlp:"nil"` + } + +In the example above, Field allows two possible input sizes. For input 0xC180 (a list +containing an empty string) Field is set to nil after decoding. For input 0xC483000000 (a +list containing a 3-byte string), Field is set to a non-nil array pointer. + 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. +"nil" tag, the kind of empty value allowed for a type is chosen automatically. A 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. +tags. Using these tags encodes/decodes a Go nil pointer value as the empty RLP value kind +defined by the tag. */ package rlp diff --git a/rlp/encode.go b/rlp/encode.go index 77b591045..b7e74a133 100644 --- a/rlp/encode.go +++ b/rlp/encode.go @@ -546,15 +546,40 @@ func makeStructWriter(typ reflect.Type) (writer, error) { return nil, structFieldError{typ, f.index, f.info.writerErr} } } - writer := func(val reflect.Value, w *encbuf) error { - lh := w.list() - for _, f := range fields { - if err := f.info.writer(val.Field(f.index), w); err != nil { - return err + + var writer writer + firstOptionalField := firstOptionalField(fields) + if firstOptionalField == len(fields) { + // This is the writer function for structs without any optional fields. + writer = func(val reflect.Value, w *encbuf) error { + lh := w.list() + for _, f := range fields { + if err := f.info.writer(val.Field(f.index), w); err != nil { + return err + } } + w.listEnd(lh) + return nil + } + } else { + // If there are any "optional" fields, the writer needs to perform additional + // checks to determine the output list length. + writer = func(val reflect.Value, w *encbuf) error { + lastField := len(fields) - 1 + for ; lastField >= firstOptionalField; lastField-- { + if !val.Field(fields[lastField].index).IsZero() { + break + } + } + lh := w.list() + for i := 0; i <= lastField; i++ { + if err := fields[i].info.writer(val.Field(fields[i].index), w); err != nil { + return err + } + } + w.listEnd(lh) + return nil } - w.listEnd(lh) - return nil } return writer, nil } diff --git a/rlp/encode_test.go b/rlp/encode_test.go index 418ee10a3..74e8ededc 100644 --- a/rlp/encode_test.go +++ b/rlp/encode_test.go @@ -257,12 +257,30 @@ var encTests = []encTest{ {val: simplestruct{A: 3, B: "foo"}, output: "C50383666F6F"}, {val: &recstruct{5, nil}, output: "C205C0"}, {val: &recstruct{5, &recstruct{4, &recstruct{3, nil}}}, output: "C605C404C203C0"}, + {val: &intField{X: 3}, error: "rlp: type int is not RLP-serializable (struct field rlp.intField.X)"}, + + // struct tag "-" + {val: &ignoredField{A: 1, B: 2, C: 3}, output: "C20103"}, + + // struct tag "tail" {val: &tailRaw{A: 1, Tail: []RawValue{unhex("02"), unhex("03")}}, output: "C3010203"}, {val: &tailRaw{A: 1, Tail: []RawValue{unhex("02")}}, output: "C20102"}, {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)"}, + + // struct tag "optional" + {val: &optionalFields{}, output: "C180"}, + {val: &optionalFields{A: 1}, output: "C101"}, + {val: &optionalFields{A: 1, B: 2}, output: "C20102"}, + {val: &optionalFields{A: 1, B: 2, C: 3}, output: "C3010203"}, + {val: &optionalFields{A: 1, B: 0, C: 3}, output: "C3018003"}, + {val: &optionalAndTailField{A: 1}, output: "C101"}, + {val: &optionalAndTailField{A: 1, B: 2}, output: "C20102"}, + {val: &optionalAndTailField{A: 1, Tail: []uint{5, 6}}, output: "C401800506"}, + {val: &optionalAndTailField{A: 1, Tail: []uint{5, 6}}, output: "C401800506"}, + {val: &optionalBigIntField{A: 1}, output: "C101"}, + {val: &optionalPtrField{A: 1}, output: "C101"}, + {val: &optionalPtrFieldNil{A: 1}, output: "C101"}, // nil {val: (*uint)(nil), output: "80"}, diff --git a/rlp/typecache.go b/rlp/typecache.go index 6026e1a64..3910dcf08 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -38,15 +38,16 @@ type typeinfo struct { // 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 is the kind of empty value allowed for the field. nilKind Kind + nilOK bool - // 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. + // rlp:"optional" allows for a field to be missing in the input list. + // If this is set, all subsequent fields must also be optional. + optional bool + + // 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. @@ -104,28 +105,51 @@ func cachedTypeInfo1(typ reflect.Type, tags tags) *typeinfo { } type field struct { - index int - info *typeinfo + index int + info *typeinfo + optional bool } +// structFields resolves the typeinfo of all public fields in a struct type. func structFields(typ reflect.Type) (fields []field, err error) { - lastPublic := lastPublicField(typ) + var ( + lastPublic = lastPublicField(typ) + anyOptional = false + ) for i := 0; i < typ.NumField(); i++ { if f := typ.Field(i); f.PkgPath == "" { // exported tags, err := parseStructTag(typ, i, lastPublic) if err != nil { return nil, err } + + // Skip rlp:"-" fields. if tags.ignored { continue } + // If any field has the "optional" tag, subsequent fields must also have it. + if tags.optional || tags.tail { + anyOptional = true + } else if anyOptional { + return nil, fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) + } info := cachedTypeInfo1(f.Type, tags) - fields = append(fields, field{i, info}) + fields = append(fields, field{i, info, tags.optional}) } } return fields, nil } +// anyOptionalFields returns the index of the first field with "optional" tag. +func firstOptionalField(fields []field) int { + for i, f := range fields { + if f.optional { + return i + } + } + return len(fields) +} + type structFieldError struct { typ reflect.Type field int @@ -166,11 +190,19 @@ func parseStructTag(typ reflect.Type, fi, lastPublic int) (tags, error) { case "nilList": ts.nilKind = List } + case "optional": + ts.optional = true + if ts.tail { + return ts, structTagError{typ, f.Name, t, `also has "tail" tag`} + } case "tail": ts.tail = true if fi != lastPublic { return ts, structTagError{typ, f.Name, t, "must be on last field"} } + if ts.optional { + return ts, structTagError{typ, f.Name, t, `also has "optional" tag`} + } if f.Type.Kind() != reflect.Slice { return ts, structTagError{typ, f.Name, t, "field type is not slice"} }