Skip to content

Commit 8618c30

Browse files
Remove len tag parsing from the reflect codec (#2559)
1 parent e89d972 commit 8618c30

File tree

4 files changed

+45
-93
lines changed

4 files changed

+45
-93
lines changed

codec/reflectcodec/struct_fielder.go

+11-35
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,31 @@ package reflectcodec
66
import (
77
"fmt"
88
"reflect"
9-
"strconv"
109
"sync"
1110

1211
"github.com/ava-labs/avalanchego/codec"
1312
)
1413

15-
const (
16-
// SliceLenTagName that specifies the length of a slice.
17-
SliceLenTagName = "len"
18-
19-
// TagValue is the value the tag must have to be serialized.
20-
TagValue = "true"
21-
)
14+
// TagValue is the value the tag must have to be serialized.
15+
const TagValue = "true"
2216

2317
var _ StructFielder = (*structFielder)(nil)
2418

25-
type FieldDesc struct {
26-
Index int
27-
MaxSliceLen uint32
28-
}
29-
3019
// StructFielder handles discovery of serializable fields in a struct.
3120
type StructFielder interface {
3221
// Returns the fields that have been marked as serializable in [t], which is
33-
// a struct type. Additionally, returns the custom maximum length slice that
34-
// may be serialized into the field, if any.
22+
// a struct type.
3523
// Returns an error if a field has tag "[tagName]: [TagValue]" but the field
3624
// is un-exported.
3725
// GetSerializedField(Foo) --> [1,5,8] means Foo.Field(1), Foo.Field(5),
3826
// Foo.Field(8) are to be serialized/deserialized.
39-
GetSerializedFields(t reflect.Type) ([]FieldDesc, error)
27+
GetSerializedFields(t reflect.Type) ([]int, error)
4028
}
4129

42-
func NewStructFielder(tagNames []string, maxSliceLen uint32) StructFielder {
30+
func NewStructFielder(tagNames []string) StructFielder {
4331
return &structFielder{
4432
tags: tagNames,
45-
maxSliceLen: maxSliceLen,
46-
serializedFieldIndices: make(map[reflect.Type][]FieldDesc),
33+
serializedFieldIndices: make(map[reflect.Type][]int),
4734
}
4835
}
4936

@@ -54,17 +41,15 @@ type structFielder struct {
5441
// if it has at least one of the specified tags.
5542
tags []string
5643

57-
maxSliceLen uint32
58-
5944
// Key: a struct type
6045
// Value: Slice where each element is index in the struct type of a field
6146
// that is serialized/deserialized e.g. Foo --> [1,5,8] means Foo.Field(1),
6247
// etc. are to be serialized/deserialized. We assume this cache is pretty
6348
// small (a few hundred keys at most) and doesn't take up much memory.
64-
serializedFieldIndices map[reflect.Type][]FieldDesc
49+
serializedFieldIndices map[reflect.Type][]int
6550
}
6651

67-
func (s *structFielder) GetSerializedFields(t reflect.Type) ([]FieldDesc, error) {
52+
func (s *structFielder) GetSerializedFields(t reflect.Type) ([]int, error) {
6853
if serializedFields, ok := s.getCachedSerializedFields(t); ok { // use pre-computed result
6954
return serializedFields, nil
7055
}
@@ -73,7 +58,7 @@ func (s *structFielder) GetSerializedFields(t reflect.Type) ([]FieldDesc, error)
7358
defer s.lock.Unlock()
7459

7560
numFields := t.NumField()
76-
serializedFields := make([]FieldDesc, 0, numFields)
61+
serializedFields := make([]int, 0, numFields)
7762
for i := 0; i < numFields; i++ { // Go through all fields of this struct
7863
field := t.Field(i)
7964

@@ -96,22 +81,13 @@ func (s *structFielder) GetSerializedFields(t reflect.Type) ([]FieldDesc, error)
9681
field.Name,
9782
)
9883
}
99-
sliceLenField := field.Tag.Get(SliceLenTagName)
100-
maxSliceLen := s.maxSliceLen
101-
102-
if newLen, err := strconv.ParseUint(sliceLenField, 10, 31); err == nil {
103-
maxSliceLen = uint32(newLen)
104-
}
105-
serializedFields = append(serializedFields, FieldDesc{
106-
Index: i,
107-
MaxSliceLen: maxSliceLen,
108-
})
84+
serializedFields = append(serializedFields, i)
10985
}
11086
s.serializedFieldIndices[t] = serializedFields // cache result
11187
return serializedFields, nil
11288
}
11389

114-
func (s *structFielder) getCachedSerializedFields(t reflect.Type) ([]FieldDesc, bool) {
90+
func (s *structFielder) getCachedSerializedFields(t reflect.Type) ([]int, bool) {
11591
s.lock.RLock()
11692
defer s.lock.RUnlock()
11793

codec/reflectcodec/type_codec.go

+27-29
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func New(typer TypeCodec, tagNames []string, maxSliceLen uint32) codec.Codec {
8181
return &genericCodec{
8282
typer: typer,
8383
maxSliceLen: maxSliceLen,
84-
fielder: NewStructFielder(tagNames, maxSliceLen),
84+
fielder: NewStructFielder(tagNames),
8585
}
8686
}
8787

@@ -208,8 +208,8 @@ func (c *genericCodec) size(
208208
size int
209209
constSize = true
210210
)
211-
for _, fieldDesc := range serializedFields {
212-
innerSize, innerConstSize, err := c.size(value.Field(fieldDesc.Index), typeStack)
211+
for _, fieldIndex := range serializedFields {
212+
innerSize, innerConstSize, err := c.size(value.Field(fieldIndex), typeStack)
213213
if err != nil {
214214
return 0, false, err
215215
}
@@ -292,7 +292,7 @@ func (c *genericCodec) MarshalInto(value interface{}, p *wrappers.Packer) error
292292
return errMarshalNil // can't marshal nil
293293
}
294294

295-
return c.marshal(reflect.ValueOf(value), p, c.maxSliceLen, nil /*=typeStack*/)
295+
return c.marshal(reflect.ValueOf(value), p, nil /*=typeStack*/)
296296
}
297297

298298
// marshal writes the byte representation of [value] to [p]
@@ -301,7 +301,6 @@ func (c *genericCodec) MarshalInto(value interface{}, p *wrappers.Packer) error
301301
func (c *genericCodec) marshal(
302302
value reflect.Value,
303303
p *wrappers.Packer,
304-
maxSliceLen uint32,
305304
typeStack set.Set[reflect.Type],
306305
) error {
307306
switch valueKind := value.Kind(); valueKind {
@@ -340,7 +339,7 @@ func (c *genericCodec) marshal(
340339
return errMarshalNil
341340
}
342341

343-
return c.marshal(value.Elem(), p, c.maxSliceLen, typeStack)
342+
return c.marshal(value.Elem(), p, typeStack)
344343
case reflect.Interface:
345344
if value.IsNil() {
346345
return errMarshalNil
@@ -355,18 +354,18 @@ func (c *genericCodec) marshal(
355354
if err := c.typer.PackPrefix(p, underlyingType); err != nil {
356355
return err
357356
}
358-
if err := c.marshal(value.Elem(), p, c.maxSliceLen, typeStack); err != nil {
357+
if err := c.marshal(value.Elem(), p, typeStack); err != nil {
359358
return err
360359
}
361360
typeStack.Remove(underlyingType)
362361
return p.Err
363362
case reflect.Slice:
364363
numElts := value.Len() // # elements in the slice/array. 0 if this slice is nil.
365-
if uint32(numElts) > maxSliceLen {
364+
if uint32(numElts) > c.maxSliceLen {
366365
return fmt.Errorf("%w; slice length, %d, exceeds maximum length, %d",
367366
codec.ErrMaxSliceLenExceeded,
368367
numElts,
369-
maxSliceLen,
368+
c.maxSliceLen,
370369
)
371370
}
372371
p.PackInt(uint32(numElts)) // pack # elements
@@ -386,7 +385,7 @@ func (c *genericCodec) marshal(
386385
return p.Err
387386
}
388387
for i := 0; i < numElts; i++ { // Process each element in the slice
389-
if err := c.marshal(value.Index(i), p, c.maxSliceLen, typeStack); err != nil {
388+
if err := c.marshal(value.Index(i), p, typeStack); err != nil {
390389
return err
391390
}
392391
}
@@ -406,7 +405,7 @@ func (c *genericCodec) marshal(
406405
)
407406
}
408407
for i := 0; i < numElts; i++ { // Process each element in the array
409-
if err := c.marshal(value.Index(i), p, c.maxSliceLen, typeStack); err != nil {
408+
if err := c.marshal(value.Index(i), p, typeStack); err != nil {
410409
return err
411410
}
412411
}
@@ -416,20 +415,20 @@ func (c *genericCodec) marshal(
416415
if err != nil {
417416
return err
418417
}
419-
for _, fieldDesc := range serializedFields { // Go through all fields of this struct that are serialized
420-
if err := c.marshal(value.Field(fieldDesc.Index), p, fieldDesc.MaxSliceLen, typeStack); err != nil { // Serialize the field and write to byte array
418+
for _, fieldIndex := range serializedFields { // Go through all fields of this struct that are serialized
419+
if err := c.marshal(value.Field(fieldIndex), p, typeStack); err != nil { // Serialize the field and write to byte array
421420
return err
422421
}
423422
}
424423
return nil
425424
case reflect.Map:
426425
keys := value.MapKeys()
427426
numElts := len(keys)
428-
if uint32(numElts) > maxSliceLen {
427+
if uint32(numElts) > c.maxSliceLen {
429428
return fmt.Errorf("%w; map length, %d, exceeds maximum length, %d",
430429
codec.ErrMaxSliceLenExceeded,
431430
numElts,
432-
maxSliceLen,
431+
c.maxSliceLen,
433432
)
434433
}
435434
p.PackInt(uint32(numElts)) // pack # elements
@@ -448,7 +447,7 @@ func (c *genericCodec) marshal(
448447
startOffset := p.Offset
449448
endOffset := p.Offset
450449
for i, key := range keys {
451-
if err := c.marshal(key, p, c.maxSliceLen, typeStack); err != nil {
450+
if err := c.marshal(key, p, typeStack); err != nil {
452451
return err
453452
}
454453
if p.Err != nil {
@@ -481,7 +480,7 @@ func (c *genericCodec) marshal(
481480
}
482481

483482
// serialize and pack value
484-
if err := c.marshal(value.MapIndex(key.key), p, c.maxSliceLen, typeStack); err != nil {
483+
if err := c.marshal(value.MapIndex(key.key), p, typeStack); err != nil {
485484
return err
486485
}
487486
}
@@ -506,7 +505,7 @@ func (c *genericCodec) Unmarshal(bytes []byte, dest interface{}) error {
506505
if destPtr.Kind() != reflect.Ptr {
507506
return errNeedPointer
508507
}
509-
if err := c.unmarshal(&p, destPtr.Elem(), c.maxSliceLen, nil /*=typeStack*/); err != nil {
508+
if err := c.unmarshal(&p, destPtr.Elem(), nil /*=typeStack*/); err != nil {
510509
return err
511510
}
512511
if p.Offset != len(bytes) {
@@ -525,7 +524,6 @@ func (c *genericCodec) Unmarshal(bytes []byte, dest interface{}) error {
525524
func (c *genericCodec) unmarshal(
526525
p *wrappers.Packer,
527526
value reflect.Value,
528-
maxSliceLen uint32,
529527
typeStack set.Set[reflect.Type],
530528
) error {
531529
switch value.Kind() {
@@ -588,11 +586,11 @@ func (c *genericCodec) unmarshal(
588586
if p.Err != nil {
589587
return fmt.Errorf("couldn't unmarshal slice: %w", p.Err)
590588
}
591-
if numElts32 > maxSliceLen {
589+
if numElts32 > c.maxSliceLen {
592590
return fmt.Errorf("%w; array length, %d, exceeds maximum length, %d",
593591
codec.ErrMaxSliceLenExceeded,
594592
numElts32,
595-
maxSliceLen,
593+
c.maxSliceLen,
596594
)
597595
}
598596
if numElts32 > math.MaxInt32 {
@@ -618,7 +616,7 @@ func (c *genericCodec) unmarshal(
618616
zeroValue := reflect.Zero(innerType)
619617
for i := 0; i < numElts; i++ {
620618
value.Set(reflect.Append(value, zeroValue))
621-
if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, typeStack); err != nil {
619+
if err := c.unmarshal(p, value.Index(i), typeStack); err != nil {
622620
return err
623621
}
624622
}
@@ -636,7 +634,7 @@ func (c *genericCodec) unmarshal(
636634
return nil
637635
}
638636
for i := 0; i < numElts; i++ {
639-
if err := c.unmarshal(p, value.Index(i), c.maxSliceLen, typeStack); err != nil {
637+
if err := c.unmarshal(p, value.Index(i), typeStack); err != nil {
640638
return err
641639
}
642640
}
@@ -659,7 +657,7 @@ func (c *genericCodec) unmarshal(
659657
typeStack.Add(intfImplementorType)
660658

661659
// Unmarshal into the struct
662-
if err := c.unmarshal(p, intfImplementor, c.maxSliceLen, typeStack); err != nil {
660+
if err := c.unmarshal(p, intfImplementor, typeStack); err != nil {
663661
return err
664662
}
665663

@@ -673,8 +671,8 @@ func (c *genericCodec) unmarshal(
673671
return fmt.Errorf("couldn't unmarshal struct: %w", err)
674672
}
675673
// Go through the fields and umarshal into them
676-
for _, fieldDesc := range serializedFieldIndices {
677-
if err := c.unmarshal(p, value.Field(fieldDesc.Index), fieldDesc.MaxSliceLen, typeStack); err != nil {
674+
for _, fieldIndex := range serializedFieldIndices {
675+
if err := c.unmarshal(p, value.Field(fieldIndex), typeStack); err != nil {
678676
return err
679677
}
680678
}
@@ -685,7 +683,7 @@ func (c *genericCodec) unmarshal(
685683
// Create a new pointer to a new value of the underlying type
686684
v := reflect.New(t)
687685
// Fill the value
688-
if err := c.unmarshal(p, v.Elem(), c.maxSliceLen, typeStack); err != nil {
686+
if err := c.unmarshal(p, v.Elem(), typeStack); err != nil {
689687
return err
690688
}
691689
// Assign to the top-level struct's member
@@ -720,7 +718,7 @@ func (c *genericCodec) unmarshal(
720718

721719
keyStartOffset := p.Offset
722720

723-
if err := c.unmarshal(p, mapKey, c.maxSliceLen, typeStack); err != nil {
721+
if err := c.unmarshal(p, mapKey, typeStack); err != nil {
724722
return err
725723
}
726724

@@ -738,7 +736,7 @@ func (c *genericCodec) unmarshal(
738736

739737
// Get the value
740738
mapValue := reflect.New(mapValueType).Elem()
741-
if err := c.unmarshal(p, mapValue, c.maxSliceLen, typeStack); err != nil {
739+
if err := c.unmarshal(p, mapValue, typeStack); err != nil {
742740
return err
743741
}
744742

codec/test_codec.go

+2-24
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ var (
3939
TestNegativeNumbers,
4040
TestTooLargeUnmarshal,
4141
TestUnmarshalInvalidInterface,
42-
TestRestrictedSlice,
4342
TestExtraSpace,
4443
TestSliceLengthOverflow,
4544
TestMap,
@@ -874,27 +873,6 @@ func TestUnmarshalInvalidInterface(codec GeneralCodec, t testing.TB) {
874873
}
875874
}
876875

877-
// Ensure deserializing slices that have been length restricted errors correctly
878-
func TestRestrictedSlice(codec GeneralCodec, t testing.TB) {
879-
require := require.New(t)
880-
881-
type inner struct {
882-
Bytes []byte `serialize:"true" len:"2"`
883-
}
884-
bytes := []byte{0, 0, 0, 0, 0, 3, 0, 1, 2}
885-
886-
manager := NewDefaultManager()
887-
require.NoError(manager.RegisterCodec(0, codec))
888-
889-
s := inner{}
890-
_, err := manager.Unmarshal(bytes, &s)
891-
require.ErrorIs(err, ErrMaxSliceLenExceeded)
892-
893-
s.Bytes = []byte{0, 1, 2}
894-
_, err = manager.Marshal(0, s)
895-
require.ErrorIs(err, ErrMaxSliceLenExceeded)
896-
}
897-
898876
// Test unmarshaling something with extra data
899877
func TestExtraSpace(codec GeneralCodec, t testing.TB) {
900878
require := require.New(t)
@@ -909,12 +887,12 @@ func TestExtraSpace(codec GeneralCodec, t testing.TB) {
909887
require.ErrorIs(err, ErrExtraSpace)
910888
}
911889

912-
// Ensure deserializing slices that have been length restricted errors correctly
890+
// Ensure deserializing slices whose lengths exceed MaxInt32 error correctly
913891
func TestSliceLengthOverflow(codec GeneralCodec, t testing.TB) {
914892
require := require.New(t)
915893

916894
type inner struct {
917-
Vals []uint32 `serialize:"true" len:"2"`
895+
Vals []uint32 `serialize:"true"`
918896
}
919897
bytes := []byte{
920898
// Codec Version:

snow/engine/avalanche/vertex/stateless_vertex.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,11 @@ func (v statelessVertex) Txs() [][]byte {
9494

9595
type innerStatelessVertex struct {
9696
Version uint16 `json:"version"`
97-
ChainID ids.ID `json:"chainID" serializeV0:"true" serializeV1:"true"`
98-
Height uint64 `json:"height" serializeV0:"true" serializeV1:"true"`
99-
Epoch uint32 `json:"epoch" serializeV0:"true"`
100-
ParentIDs []ids.ID `json:"parentIDs" len:"128" serializeV0:"true" serializeV1:"true"`
101-
Txs [][]byte `json:"txs" len:"128" serializeV0:"true"`
97+
ChainID ids.ID `json:"chainID" serializeV0:"true" serializeV1:"true"`
98+
Height uint64 `json:"height" serializeV0:"true" serializeV1:"true"`
99+
Epoch uint32 `json:"epoch" serializeV0:"true"`
100+
ParentIDs []ids.ID `json:"parentIDs" serializeV0:"true" serializeV1:"true"`
101+
Txs [][]byte `json:"txs" serializeV0:"true"`
102102
}
103103

104104
func (v innerStatelessVertex) Verify() error {

0 commit comments

Comments
 (0)