From 43ccee71103b10b43e6ec663ebd5f4f8511ef52f Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 6 May 2021 14:50:21 +0200 Subject: [PATCH 1/9] rlp: WIP optional fields --- rlp/decode.go | 3 +++ rlp/decode_test.go | 28 ++++++++++++++++++++++++++++ rlp/doc.go | 37 ++++++++++++++++++++++++++++++++----- rlp/encode.go | 44 +++++++++++++++++++++++++++++++++++++------- rlp/encode_test.go | 6 ++++++ rlp/typecache.go | 38 ++++++++++++++++++++++++++++++++++---- 6 files changed, 140 insertions(+), 16 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 79b7ef062664..47fccd796945 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -397,6 +397,9 @@ func makeStructDecoder(typ reflect.Type) (decoder, error) { for _, f := range fields { err := f.info.decoder(s, val.Field(f.index)) if err == EOL { + if f.optional { + break + } return &decodeError{msg: "too few elements", typ: typ} } else if err != nil { return addErrorContext(err, "."+typ.Field(f.index).Name) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index d94c3969b221..ac66945ff3aa 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -369,6 +369,12 @@ type intField struct { X int } +type optionalFields struct { + A uint32 + B uint32 `rlp:"optional"` + C uint32 `rlp:"optional"` +} + var ( veryBigInt = big.NewInt(0).Add( big.NewInt(0).Lsh(big.NewInt(0xFFFFFFFFFFFFFF), 16), @@ -592,6 +598,28 @@ 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", + }, + // RawValue {input: "01", ptr: new(RawValue), value: RawValue(unhex("01"))}, {input: "82FFFF", ptr: new(RawValue), value: RawValue(unhex("82FFFF"))}, diff --git a/rlp/doc.go b/rlp/doc.go index 7e6ee8520019..df11d522714c 100644 --- a/rlp/doc.go +++ b/rlp/doc.go @@ -102,21 +102,48 @@ 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 encoding/json, the "-" tag ignores fields. -The "-" tag ignores fields. + type StructWithIgnoredField struct{ + Ignored uint `rlp:"-"` + Field uint + } + +All other supported struct tags have the purpose of modifying the mapping between +RLP lists and Go struct 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. +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 +decoding an RLP list into a struct, optional fields may be omitted from the input list. +When encoding a struct with optional fields, the output RLP list contains all values up to +the last non-zero optional field. + + type StructWithOptionalFields struct{ + Required uint + Optional1 uint `rlp:"optional"` + Optional2 uint `rlp:"optional"` + } 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 StructWithNilField struct { + Field *[3]byte `rlp:"nil"` } +In the example above, the field allows two possible input sizes. For input 0xC180 (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 diff --git a/rlp/encode.go b/rlp/encode.go index 77b591045d2f..c00e56022e50 100644 --- a/rlp/encode.go +++ b/rlp/encode.go @@ -546,15 +546,45 @@ 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 { + lh := w.list() + for i := 0; i < firstOptionalField; i++ { + if err := fields[i].info.writer(val.Field(fields[i].index), w); err != nil { + return err + } + } + lastField := len(fields) - 1 + for ; lastField >= firstOptionalField; lastField-- { + if !val.Field(fields[lastField].index).IsZero() { + break + } + } + for i := firstOptionalField; 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 418ee10a350e..3236e397fcb8 100644 --- a/rlp/encode_test.go +++ b/rlp/encode_test.go @@ -264,6 +264,12 @@ var encTests = []encTest{ {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{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"}, + // nil {val: (*uint)(nil), output: "80"}, {val: (*string)(nil), output: "80"}, diff --git a/rlp/typecache.go b/rlp/typecache.go index 6026e1a64951..ebb995548b60 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -44,6 +44,8 @@ type tags struct { // or empty lists. nilKind Kind + 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. @@ -104,28 +106,54 @@ 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 { + anyOptional = true + } else { + if anyOptional { + err := fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) + return nil, err + } + } 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,6 +194,8 @@ func parseStructTag(typ reflect.Type, fi, lastPublic int) (tags, error) { case "nilList": ts.nilKind = List } + case "optional": + ts.optional = true case "tail": ts.tail = true if fi != lastPublic { From 8b2a17445fcf0970a9eb7264bb78d73a2f5259db Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 6 May 2021 15:08:28 +0200 Subject: [PATCH 2/9] rlp: tweak documentation --- rlp/doc.go | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/rlp/doc.go b/rlp/doc.go index df11d522714c..113828e39b90 100644 --- a/rlp/doc.go +++ b/rlp/doc.go @@ -102,18 +102,16 @@ Signed integers, floating point numbers, maps, channels and functions cannot be Struct Tags -As with encoding/json, the "-" tag ignores fields. +As with other encoding packages, the "-" tag ignores fields. type StructWithIgnoredField struct{ Ignored uint `rlp:"-"` Field uint } -All other supported struct tags have the purpose of modifying the mapping between -RLP lists and Go struct 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. +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 @@ -121,37 +119,43 @@ up any excess list elements into a slice. } 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 -decoding an RLP list into a struct, optional fields may be omitted from the input list. +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" 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. +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, the field allows two possible input sizes. For input 0xC180 (list +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 From a0039570d90ff70f6b67cee7cc5fd3f0ed3d3431 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 6 May 2021 15:20:02 +0200 Subject: [PATCH 3/9] rlp: hack in support for optional + tail --- rlp/typecache.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rlp/typecache.go b/rlp/typecache.go index ebb995548b60..44c1dee25328 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -129,13 +129,12 @@ func structFields(typ reflect.Type) (fields []field, err error) { continue } // If any field has the "optional" tag, subsequent fields must also have it. - if tags.optional { + if tags.optional || tags.tail { anyOptional = true - } else { - if anyOptional { - err := fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) - return nil, err - } + } + if tags.optional && !anyOptional { + err := fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) + return nil, err } info := cachedTypeInfo1(f.Type, tags) fields = append(fields, field{i, info, tags.optional}) From b3e2673accfc6e48ef1b536ce4851bd05c8f08b9 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 11:43:15 +0200 Subject: [PATCH 4/9] rlp: add more tests for optional fields --- rlp/decode_test.go | 103 ++++++++++++++++++++++++++++++++++++++++----- rlp/encode_test.go | 16 ++++++- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index ac66945ff3aa..95490b239f65 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -370,9 +370,36 @@ type intField struct { } type optionalFields struct { - A uint32 - B uint32 `rlp:"optional"` - C uint32 `rlp:"optional"` + 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 ( @@ -382,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}, @@ -557,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" @@ -619,6 +640,66 @@ var decodeTests = []decodeTest{ 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" does not preclude "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", + }, // RawValue {input: "01", ptr: new(RawValue), value: RawValue(unhex("01"))}, diff --git a/rlp/encode_test.go b/rlp/encode_test.go index 3236e397fcb8..74e8ededcb7e 100644 --- a/rlp/encode_test.go +++ b/rlp/encode_test.go @@ -257,18 +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"}, From 695e2a47b2e805c32d2cd29254df39b15090f31e Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 11:47:23 +0200 Subject: [PATCH 5/9] rlp: simplify optional fields encoder --- rlp/decode.go | 2 +- rlp/encode.go | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/rlp/decode.go b/rlp/decode.go index 47fccd796945..3f5872b25a2d 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()) } diff --git a/rlp/encode.go b/rlp/encode.go index c00e56022e50..b7e74a133f2e 100644 --- a/rlp/encode.go +++ b/rlp/encode.go @@ -565,19 +565,14 @@ func makeStructWriter(typ reflect.Type) (writer, error) { // 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 { - lh := w.list() - for i := 0; i < firstOptionalField; i++ { - if err := fields[i].info.writer(val.Field(fields[i].index), w); err != nil { - return err - } - } lastField := len(fields) - 1 for ; lastField >= firstOptionalField; lastField-- { if !val.Field(fields[lastField].index).IsZero() { break } } - for i := firstOptionalField; i <= lastField; i++ { + 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 } From 491aea19bdbbd527757186cad10b923122716e83 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 12:17:52 +0200 Subject: [PATCH 6/9] rlp: set missing optional fields to zero --- rlp/decode.go | 13 ++++++++++++- rlp/decode_test.go | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/rlp/decode.go b/rlp/decode.go index 3f5872b25a2d..b340aa029e43 100644 --- a/rlp/decode.go +++ b/rlp/decode.go @@ -394,10 +394,14 @@ 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} @@ -410,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 95490b239f65..e04dadb7a1c5 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -701,6 +701,28 @@ var decodeTests = []decodeTest{ 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"))}, From 4bc80504d4723e3c2137d0397ed7ba7fd7a9e9b7 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 13:03:46 +0200 Subject: [PATCH 7/9] rlp: fix struct tag validation of "optional" Also reorganizes tags struct to avoid padding. --- rlp/decode_test.go | 34 ++++++++++++++++++++++++++++++++++ rlp/typecache.go | 22 +++++++++++++--------- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index e04dadb7a1c5..af4cd0acc47e 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -953,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/typecache.go b/rlp/typecache.go index 44c1dee25328..fb1d16c2bd46 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -38,17 +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:"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. + // 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. @@ -131,8 +130,7 @@ func structFields(typ reflect.Type) (fields []field, err error) { // If any field has the "optional" tag, subsequent fields must also have it. if tags.optional || tags.tail { anyOptional = true - } - if tags.optional && !anyOptional { + } else if anyOptional { err := fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) return nil, err } @@ -195,11 +193,17 @@ func parseStructTag(typ reflect.Type, fi, lastPublic int) (tags, error) { } 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"} } From f391fbc1c889e6ec59b055387e260ccadd30aa38 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 13:12:57 +0200 Subject: [PATCH 8/9] rlp: remove line --- rlp/typecache.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rlp/typecache.go b/rlp/typecache.go index fb1d16c2bd46..3910dcf0806e 100644 --- a/rlp/typecache.go +++ b/rlp/typecache.go @@ -131,8 +131,7 @@ func structFields(typ reflect.Type) (fields []field, err error) { if tags.optional || tags.tail { anyOptional = true } else if anyOptional { - err := fmt.Errorf(`rlp: struct field %v.%s needs "optional" tag`, typ, f.Name) - return nil, err + 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, tags.optional}) From a9899a234db89786c8c9421bc3093eb8a47dae01 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Fri, 7 May 2021 14:28:48 +0200 Subject: [PATCH 9/9] rlp: fix comment --- rlp/decode_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rlp/decode_test.go b/rlp/decode_test.go index af4cd0acc47e..87a330633230 100644 --- a/rlp/decode_test.go +++ b/rlp/decode_test.go @@ -671,7 +671,7 @@ var decodeTests = []decodeTest{ value: optionalPtrField{A: 1}, }, { - input: "C20180", // not accepted because "optional" does not preclude "nil" + 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", },