msgpack icon indicating copy to clipboard operation
msgpack copied to clipboard

use msgpack.rawmessage to decode panic reflect: reflect.Value.Set using unaddressable value

Open freeliver opened this issue 3 years ago • 0 comments

Issue tracker is used for reporting bugs and discussing new features. Please use Discord or stackoverflow for supporting issues.

Expected Behavior

no panic and unmarshal success

Current Behavior

panic: reflect: reflect.Value.Set using unaddressable value [recovered]
	panic: reflect: reflect.Value.Set using unaddressable value

goroutine 50 [running]:
testing.tRunner.func1.2(0x104d1aa00, 0x1400036b030)
	/usr/local/go/src/testing/testing.go:1143 +0x25c
testing.tRunner.func1(0x14000102780)
	/usr/local/go/src/testing/testing.go:1146 +0x384
panic(0x104d1aa00, 0x1400036b030)
	/usr/local/go/src/runtime/panic.go:965 +0x14c
reflect.flag.mustBeAssignableSlow(0x16)
	/usr/local/go/src/reflect/value.go:260 +0x118
reflect.flag.mustBeAssignable(...)
	/usr/local/go/src/reflect/value.go:247
reflect.Value.Set(0x104d587c0, 0x140001b4af8, 0x16, 0x104d587c0, 0x0, 0x16)
	/usr/local/go/src/reflect/value.go:1558 +0x30
github.com/vmihailenco/msgpack/v5.ptrValueDecoder.func1(0x1400013a000, 0x104d587c0, 0x140001b4af8, 0x16, 0x104d587c0, 0x140001b4af8)
	/msgpack/decode_value.go:130 +0x23c
github.com/vmihailenco/msgpack/v5.(*Decoder).DecodeValue(0x1400013a000, 0x104d587c0, 0x140001b4af8, 0x16, 0x140001b4af8, 0x16)
	/msgpack/decode.go:309 +0x78
github.com/vmihailenco/msgpack/v5.decodeInterfaceValue(0x1400013a000, 0x104d3b6a0, 0x140001b4ae8, 0x194, 0x1, 0x1)
	/msgpack/decode_value.go:184 +0x80
github.com/vmihailenco/msgpack/v5.(*field).DecodeValue(0x140001d7680, 0x1400013a000, 0x104d86080, 0x140001b4ae0, 0x199, 0x0, 0x0)
	/msgpack/types.go:118 +0x84
github.com/vmihailenco/msgpack/v5.(*Decoder).decodeStruct(0x1400013a000, 0x104d86080, 0x140001b4ae0, 0x199, 0x2, 0x104bb965c, 0x10527c120)
	/msgpack/decode_map.go:324 +0x1d8
github.com/vmihailenco/msgpack/v5.decodeStructValue(0x1400013a000, 0x104d86080, 0x140001b4ae0, 0x199, 0x104d86080, 0x8)
	msgpack/decode_map.go:282 +0x268
github.com/vmihailenco/msgpack/v5.(*Decoder).DecodeValue(0x1400013a000, 0x104d86080, 0x140001b4ae0, 0x199, 0x140001b4ae0, 0x199)
	/msgpack/decode.go:309 +0x78
github.com/vmihailenco/msgpack/v5.(*Decoder).Decode(0x1400013a000, 0x104cf7dc0, 0x140001b4ae0, 0x104bde0e4, 0x18)
	/msgpack/decode.go:288 +0x16c
github.com/vmihailenco/msgpack/v5.Unmarshal(0x140001f83c0, 0xd, 0x40, 0x104cf7dc0, 0x140001b4ae0, 0x0, 0x0)
	/msgpack/decode.go:58 +0xc8
TestMsgPackDecoderCacheBugs(0x14000102780)

Possible Solution

see pr https://github.com/vmihailenco/msgpack/pull/309

Steps to Reproduce

type TestObject struct {
	Type int
	Data interface{}
}

type TestData struct {
	Val1 int
	Val2 int
}

func TestMsgPackDecoderCacheBugs(t *testing.T) {
	var test1 = []*TestData{
		{
			Val1: 1001,
			Val2: 2002,
		},
		{
			Val1: 1003,
			Val2: 2004,
		},
	}

	bytes, err := msgpack.Marshal(test1)
	if err != nil {
		t.Fatalf("unexpected error:%s", err.Error())
	}

	var decodeVal []msgpack.RawMessage
	if err = msgpack.Unmarshal(bytes, &decodeVal); err != nil {
		t.Fatalf("unexpected error:%s", err.Error())
	}

	for idx, rawMsg := range decodeVal {
		testVal := TestData{}
		if err = msgpack.Unmarshal(rawMsg, &testVal); err != nil {
			t.Fatalf("unexpected error:%s", err.Error())
		}

		t.Logf("index:%d => %#v", idx, testVal)
	}

	var val = &TestObject{
		Type: 1,
		//Data is nil will panic reflect: reflect.Value.Set using unaddressable value
		Data: nil,
	}

	bytes2, err := msgpack.Marshal(val)
	if err != nil {
		t.Fatalf("unexpected error:%s", err.Error())
	}

	var decodeVal2 = TestObject{
		Type: 1,
		Data: &msgpack.RawMessage{},
	}
	if err = msgpack.Unmarshal(bytes2, &decodeVal2); err != nil {
		t.Fatalf("unexpected error:%s", err.Error())
	}

}

Context (Environment)

go 1.16.3

Detailed Description

1. typeDecMap.Load(typ.Elem()) return wrong decoder before custom decoder


func _getDecoder(typ reflect.Type) decoderFunc {
	kind := typ.Kind()

	if kind == reflect.Ptr {
          

     //here if value is &msgpack.RawMessage, and will use cache msgpack.RawMessage=>ptrValueDecoder, then goto ptrValueDecoder and v can't set will panic error 

		if _, ok := typeDecMap.Load(typ.Elem()); ok {
			return ptrValueDecoder(typ)
		}
	}
        
       // value implements speical decodertype should before typeDecMap cache process
	if typ.Implements(customDecoderType) {
		return nilAwareDecoder(typ, decodeCustomValue)
	}
	if typ.Implements(unmarshalerType) {
		return nilAwareDecoder(typ, unmarshalValue)
	}
	if typ.Implements(binaryUnmarshalerType) {
		return nilAwareDecoder(typ, unmarshalBinaryValue)
	}
	if typ.Implements(textUnmarshalerType) {
		return nilAwareDecoder(typ, unmarshalTextValue)
	}
      .....
}

2. if has nil code and v can't set such as (&msgpack.RawMesssage{}) value will panic

func ptrValueDecoder(typ reflect.Type) decoderFunc {
	decoder := getDecoder(typ.Elem())
	return func(d *Decoder, v reflect.Value) error {
		if d.hasNilCode() {
                        //v is &msgpack.RawMesssage can't set, add v.CanSet() check
			if !v.IsNil() {
				v.Set(reflect.Zero(v.Type()))
			}
			return d.DecodeNil()
		}
		if v.IsNil() {
			v.Set(reflect.New(v.Type().Elem()))
		}
		return decoder(d, v.Elem())
	}
}

Possible Implementation

see Detailed Description

freeliver avatar Jun 03 '21 07:06 freeliver