Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
freeliver opened this issue Jun 3, 2021 · 0 comments

Comments

@freeliver
Copy link

freeliver commented Jun 3, 2021

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
#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

@ghost ghost mentioned this issue Jul 1, 2021
Alexander-r pushed a commit to Alexander-r/msgpack that referenced this issue Sep 11, 2023
Alexander-r added a commit to Alexander-r/msgpack that referenced this issue Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant