Interface nil value not set on decoding, with panic

[l0nax]

The interface field isn't set to nil when msgpack code is nil.

Expected Behavior

The field value of the struct is set to nil.

Current Behavior

Application is panic'ing:

  reflect: reflect.Value.Set using unaddressable value
  /root/.gvm/gos/go1.16/src/reflect/value.go:260

  Full Stack Trace
  reflect.flag.mustBeAssignableSlow(0x16)
        /root/.gvm/gos/go1.16/src/reflect/value.go:260 +0x138
  reflect.flag.mustBeAssignable(...)
        /root/.gvm/gos/go1.16/src/reflect/value.go:247
  reflect.Value.Set(0x946980, 0xc000612078, 0x16, 0x946980, 0x0, 0x16)
        /root/.gvm/gos/go1.16/src/reflect/value.go:1558 +0x3b
  github.com/vmihailenco/msgpack/v5.ptrValueDecoder.func1(0xc000154480, 0x946980, 0xc000612078, 0x16, 0x946980, 0xc000612078)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode_value.go:130 +0x29d
  github.com/vmihailenco/msgpack/v5.(*Decoder).DecodeValue(0xc000154480, 0x946980, 0xc000612078, 0x16, 0xc000612078, 0x16)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode.go:309 +0x8e
  github.com/vmihailenco/msgpack/v5.decodeInterfaceValue(0xc000154480, 0x971e00, 0xc00000e5d8, 0x194, 0x1, 0x1)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode_value.go:184 +0x9a
  github.com/vmihailenco/msgpack/v5.(*field).DecodeValue(0xc00031c780, 0xc000154480, 0x9ac6e0, 0xc00000e5d0, 0x199, 0x0, 0x0)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/types.go:118 +0x9a
  github.com/vmihailenco/msgpack/v5.(*Decoder).decodeStruct(0xc000154480, 0x9ac6e0, 0xc00000e5d0, 0x199, 0x2, 0xc0001b8b18, 0x8575c5)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode_map.go:324 +0x24f
  github.com/vmihailenco/msgpack/v5.decodeStructValue(0xc000154480, 0x9ac6e0, 0xc00000e5d0, 0x199, 0x9ac6e0, 0x8)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode_map.go:282 +0x330
  github.com/vmihailenco/msgpack/v5.(*Decoder).DecodeValue(0xc000154480, 0x9ac6e0, 0xc00000e5d0, 0x199, 0xc00000e5d0, 0x199)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode.go:309 +0x8e
  github.com/vmihailenco/msgpack/v5.(*Decoder).Decode(0xc000154480, 0x946a40, 0xc00000e5d0, 0x0, 0xc0001b8c20)
        /root/.go/pkg/mod/github.com/vmihailenco/msgpack/v5@v5.3.1/decode.go:288 +0x178
[...]

Possible Solution

When a field is of type interface{} function decodeInterfaceValue(…) will be called. It should be checked if the next code, is nil and the nil behavior should be respected by the interfaceValue(…) method.

We've developed a patch – debugged and tested – but since we're not really familiar with the library this patch might be incomplete.

diff --git a/decode_value.go b/decode_value.go
index d2ff2ae..4fda5bd 100644
--- a/decode_value.go
+++ b/decode_value.go
@@ -5,6 +5,8 @@ import (
    "errors"
    "fmt"
    "reflect"
+
+   "github.com/vmihailenco/msgpack/v5/msgpcode"
 )

 var (
@@ -178,7 +180,9 @@ func decodeBoolValue(d *Decoder, v reflect.Value) error {
 }

 func decodeInterfaceValue(d *Decoder, v reflect.Value) error {
-   if v.IsNil() {
+   c, _ := d.PeekCode()
+
+   if v.IsNil() || c == msgpcode.Nil {
        return d.interfaceValue(v)
    }
    return d.DecodeValue(v.Elem())
@@ -199,6 +203,8 @@ func (d *Decoder) interfaceValue(v reflect.Value) error {
        }

        v.Set(reflect.ValueOf(vv))
+   } else if v.CanSet() {
+       v.Set(reflect.Zero(v.Type()))
    }

    return nil

Steps to Reproduce

package main

import (
    "bytes"
    "log"

    "github.com/vmihailenco/msgpack/v5"
)

type ReplyData struct {
    Result int
    Data   interface{}
}

type Recipe struct {
    Name string
}

func main() {
    myData := &ReplyData{
        Result: 0,
        Data:   nil,
    }

    rpObj := ReplyData{Result: 0, Data: &Recipe{}}

    s := serialize(myData)
    deserialize(s, &rpObj)

        fmt.Printf("%+v", rpObj)
    fmt.Printf("%+v", rpObj.Data)
}

func deserialize(raw []byte, data interface{}) {
    dec := msgpack.GetDecoder()
    defer msgpack.PutDecoder(dec)

    dec.Reset(bytes.NewReader(raw))
    dec.DisallowUnknownFields(true)

    if err := dec.Decode(data); err != nil {
        log.Fatal(err)
    }
}

func serialize(data interface{}) []byte {
    var buf bytes.Buffer

    enc := msgpack.GetEncoder()
    defer msgpack.PutEncoder(enc)

    enc.Reset(&buf)
    enc.UseCompactInts(true)
    enc.UseCompactFloats(true)

    if err := enc.Encode(data); err != nil {
        log.Fatal(err)
    }

    return buf.Bytes()
}

Context (Environment)

Detailed Description

See above

Possible Implementation

[l0nax]

Any updates to this issue @vmihailenco?

[rnov]

@l0nax facing the same issue, despite not being merged, has it worked for you ?

[rnov]

@vmihailenco have you considered this fix, are there any concerns? It seems the issue has not been addressed in the latter versions.