From 91a7a4a7867718ccb6c9620120a1be5680ad0abd Mon Sep 17 00:00:00 2001 From: Jeffrey Wilcke Date: Wed, 11 May 2016 13:21:25 +0200 Subject: [PATCH] accounts/abi: fixed unpacking in to already slice interfaces Previously it was assumed that wheneven type `[]interface{}` was given that the interface was empty. The abigen rightfully assumed that interface slices which already have pre-allocated variable sets to be assigned. This PR fixes that by checking that the given `[]interface{}` is larger than zero and assigns each value using the generic `set` function (this function has also been moved to abi/reflect.go) and checks whether the assignment was possible. The generic assignment function `set` now also deals with pointers (useful for interface slice mentioned above) by dereferencing the pointer until it finds a setable type. --- accounts/abi/abi.go | 59 ++++++++++++++++++++-------------------- accounts/abi/abi_test.go | 31 +++++++++++++++++++++ accounts/abi/reflect.go | 35 +++++++++++++++++++++++- 3 files changed, 94 insertions(+), 31 deletions(-) diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index 32df6f19d..1b07b2f68 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -238,8 +238,16 @@ func (abi ABI) Unpack(v interface{}, name string, output []byte) error { return fmt.Errorf("abi: unmarshalling empty output") } - value := reflect.ValueOf(v).Elem() - typ := value.Type() + // make sure the passed value is a pointer + valueOf := reflect.ValueOf(v) + if reflect.Ptr != valueOf.Kind() { + return fmt.Errorf("abi: Unpack(non-pointer %T)", v) + } + + var ( + value = valueOf.Elem() + typ = value.Type() + ) if len(method.Outputs) > 1 { switch value.Kind() { @@ -268,6 +276,25 @@ func (abi ABI) Unpack(v interface{}, name string, output []byte) error { return fmt.Errorf("abi: cannot marshal tuple in to slice %T (only []interface{} is supported)", v) } + // if the slice already contains values, set those instead of the interface slice itself. + if value.Len() > 0 { + if len(method.Outputs) > value.Len() { + return fmt.Errorf("abi: cannot marshal in to slices of unequal size (require: %v, got: %v)", len(method.Outputs), value.Len()) + } + + for i := 0; i < len(method.Outputs); i++ { + marshalledValue, err := toGoType(i, method.Outputs[i], output) + if err != nil { + return err + } + reflectValue := reflect.ValueOf(marshalledValue) + if err := set(value.Index(i).Elem(), reflectValue, method.Outputs[i]); err != nil { + return err + } + } + return nil + } + // create a new slice and start appending the unmarshalled // values to the new interface slice. z := reflect.MakeSlice(typ, 0, len(method.Outputs)) @@ -296,34 +323,6 @@ func (abi ABI) Unpack(v interface{}, name string, output []byte) error { return nil } -// set attempts to assign src to dst by either setting, copying or otherwise. -// -// set is a bit more lenient when it comes to assignment and doesn't force an as -// strict ruleset as bare `reflect` does. -func set(dst, src reflect.Value, output Argument) error { - dstType := dst.Type() - srcType := src.Type() - - switch { - case dstType.AssignableTo(src.Type()): - dst.Set(src) - case dstType.Kind() == reflect.Array && srcType.Kind() == reflect.Slice: - if !dstType.Elem().AssignableTo(r_byte) { - return fmt.Errorf("abi: cannot unmarshal %v in to array of elem %v", src.Type(), dstType.Elem()) - } - - if dst.Len() < output.Type.SliceSize { - return fmt.Errorf("abi: cannot unmarshal src (len=%d) in to dst (len=%d)", output.Type.SliceSize, dst.Len()) - } - reflect.Copy(dst, src) - case dstType.Kind() == reflect.Interface: - dst.Set(src) - default: - return fmt.Errorf("abi: cannot unmarshal %v in to %v", src.Type(), dst.Type()) - } - return nil -} - func (abi *ABI) UnmarshalJSON(data []byte) error { var fields []struct { Type string diff --git a/accounts/abi/abi_test.go b/accounts/abi/abi_test.go index 05535b3b5..df89ba138 100644 --- a/accounts/abi/abi_test.go +++ b/accounts/abi/abi_test.go @@ -289,6 +289,37 @@ func TestSimpleMethodUnpack(t *testing.T) { } } +func TestUnpackSetInterfaceSlice(t *testing.T) { + var ( + var1 = new(uint8) + var2 = new(uint8) + ) + out := []interface{}{var1, var2} + abi, err := JSON(strings.NewReader(`[{"type":"function", "name":"ints", "outputs":[{"type":"uint8"}, {"type":"uint8"}]}]`)) + if err != nil { + t.Fatal(err) + } + marshalledReturn := append(pad([]byte{1}, 32, true), pad([]byte{2}, 32, true)...) + err = abi.Unpack(&out, "ints", marshalledReturn) + if err != nil { + t.Fatal(err) + } + if *var1 != 1 { + t.Errorf("expected var1 to be 1, got", *var1) + } + if *var2 != 2 { + t.Errorf("expected var2 to be 2, got", *var2) + } + + out = []interface{}{var1} + err = abi.Unpack(&out, "ints", marshalledReturn) + + expErr := "abi: cannot marshal in to slices of unequal size (require: 2, got: 1)" + if err == nil || err.Error() != expErr { + t.Error("expected err:", expErr, "Got:", err) + } +} + func TestPack(t *testing.T) { for i, test := range []struct { typ string diff --git a/accounts/abi/reflect.go b/accounts/abi/reflect.go index 780c64c66..ab5020200 100644 --- a/accounts/abi/reflect.go +++ b/accounts/abi/reflect.go @@ -16,7 +16,10 @@ package abi -import "reflect" +import ( + "fmt" + "reflect" +) // indirect recursively dereferences the value until it either gets the value // or finds a big.Int @@ -62,3 +65,33 @@ func mustArrayToByteSlice(value reflect.Value) reflect.Value { reflect.Copy(slice, value) return slice } + +// set attempts to assign src to dst by either setting, copying or otherwise. +// +// set is a bit more lenient when it comes to assignment and doesn't force an as +// strict ruleset as bare `reflect` does. +func set(dst, src reflect.Value, output Argument) error { + dstType := dst.Type() + srcType := src.Type() + + switch { + case dstType.AssignableTo(src.Type()): + dst.Set(src) + case dstType.Kind() == reflect.Array && srcType.Kind() == reflect.Slice: + if !dstType.Elem().AssignableTo(r_byte) { + return fmt.Errorf("abi: cannot unmarshal %v in to array of elem %v", src.Type(), dstType.Elem()) + } + + if dst.Len() < output.Type.SliceSize { + return fmt.Errorf("abi: cannot unmarshal src (len=%d) in to dst (len=%d)", output.Type.SliceSize, dst.Len()) + } + reflect.Copy(dst, src) + case dstType.Kind() == reflect.Interface: + dst.Set(src) + case dstType.Kind() == reflect.Ptr: + return set(dst.Elem(), src, output) + default: + return fmt.Errorf("abi: cannot unmarshal %v in to %v", src.Type(), dst.Type()) + } + return nil +}