v2: UUID unmarshal bug

[romsar]

I have that struct:

type Space struct {
	UUID uuid.UUID `json:"uuid" msgpack:"uuid"`
	Name string    `json:"name" msgpack:"name"`
}

So I do simple marshal/unmarshal:

space := &comments.Space{}

b, err := msgpack.Marshal(&comments.Space{UUID: uuid.New(), Name: "foo bar"})

var item comments.Space
err = msgpack.Unmarshal(b, &item)

fmt.Println(item) // UUID: 123e4567-e89b-12d3-a456-426614174000 Name: ""

As you can see Name is empty after unmashalling, but It have to be "foo bar".

If I use https://github.com/citilinkru/uuid-msgpack the code above will work good and name will be "foo bar", but I can't combine both packages because of conflicts.

[romsar]

fixed by using build tag "go_tarantool_msgpack_v5"

[oleg-jukovec]

Thank you for the issue!

The problem is relevant for msgpack.v2, the reason probably in that line:

go-tarantool/uuid/uuid.go

Line 46 in bda4442

n, err := d.Buffered().Read(bytes)

It looks like it doesn't move a pointer to unreaded data in the decoder buffer.

The problem should be fixed with Marshaler/Unmarshaler interface (see the decimal implementation) insead of encodeUUID/decodeUUID.

go-tarantool/decimal/decimal.go

Line 59 in bda4442

func (decNum *Decimal) MarshalMsgpack() ([]byte, error) {

go-tarantool/decimal/decimal.go

Line 80 in bda4442

func (decNum *Decimal) UnmarshalMsgpack(b []byte) error {

A test to reproduce (I will improve it later):

--- a/uuid/uuid_test.go
+++ b/uuid/uuid_test.go
@@ -8,6 +8,7 @@ import (
        "time"
 
        "github.com/google/uuid"
+       "gopkg.in/vmihailenco/msgpack.v2"
        . "github.com/tarantool/go-tarantool"
        "github.com/tarantool/go-tarantool/test_helpers"
        _ "github.com/tarantool/go-tarantool/uuid"
@@ -133,6 +134,30 @@ func TestReplace(t *testing.T) {
        tupleValueIsId(t, respSel.Data, id)
 }
 
+type TestMarshalTuple struct {
+       UUID uuid.UUID `json:"uuid" msgpack:"uuid"`
+       Name string    `json:"name" msgpack:"name"`
+}
+
+func TestMatshal(t *testing.T) {
+       space := TestMarshalTuple{UUID: uuid.New(), Name: "foo bar"}
+       //space := TestMarshalTuple{UUID: "ololo", Name: "foo bar"}
+       b, err := msgpack.Marshal(&space)
+       if err != nil {
+               t.Fatalf("Unable to marshal: %s", err)
+       }
+
+       var item TestMarshalTuple
+       err = msgpack.Unmarshal(b, &item)
+       if err != nil {
+               t.Fatalf("Unable to unmarshal: %s", err)
+       }
+
+       if item.Name != space.Name {
+               t.Fatalf("names is not equal: %s != %s", space.Name, item.Name)
+       }
+}
+

[oleg-jukovec]

The problem comes from the implementation of msgpack.v2.

In the first case, we only get external data in the begginning of d.Buffered(). The value is decoded via ext.go code:

#0  github.com/tarantool/go-tarantool/uuid.decodeUUID (d=0xc0000a0820, v=..., ~r0=...) at /home/ozi/src/git/go-tarantool/uuid/uuid.go:43
#1  0x00000000007034c2 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).DecodeValue (d=0xc0000a0820, v=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:191
#2  0x0000000000719f09 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).ext (d=0xc0000a0820, c=216 '\330', ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/ext.go:160
#3  0x0000000000704d3f in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).DecodeInterface (d=0xc0000a0820, ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:297
#4  0x000000000070dca5 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).decodeSlice (d=0xc0000a0820, c=145 '\221', ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode_slice.go:174
#5  0x0000000000704095 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).DecodeInterface (d=0xc0000a0820, ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:267
#6  0x000000000070dca5 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).decodeSlice (d=0xc0000a0820, c=221 '\335', ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode_slice.go:174
#7  0x0000000000704a97 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).DecodeInterface (d=0xc0000a0820, ~r0=..., ~r1=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:291
...
(gdb) p bytes.array
$8 = (uint8 *) 0xc000016d00 "\310\360\372\037\332)C\214\240@9?\021&\255\071"

In the second case (in the issue) we have (external id, external len, external data) in the beginning of d.Buffered(). The value is decoded via decodeUUID directly:

#0  github.com/tarantool/go-tarantool/uuid.decodeUUID (d=0xc000214140, v=..., ~r0=...) at /home/ozi/src/git/go-tarantool/uuid/uuid.go:43
#1  0x000000000071b84c in gopkg.in/vmihailenco/msgpack%2ev2.(*field).DecodeValue (f=0xc00020c200, d=0xc000214140, strct=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/types.go:90
#2  0x00000000007083d9 in gopkg.in/vmihailenco/msgpack%2ev2.decodeStructValue (d=0xc000214140, strct=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode_map.go:254
#3  0x00000000007034c2 in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).DecodeValue (d=0xc000214140, v=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:191
#4  0x00000000007032cd in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).decode (d=0xc000214140, dst=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:186
#5  0x0000000000701ced in gopkg.in/vmihailenco/msgpack%2ev2.(*Decoder).Decode (d=0xc000214140, v=..., ~r0=...)
    at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:72
#6  0x00000000007018e9 in gopkg.in/vmihailenco/msgpack%2ev2.Unmarshal (data=..., v=..., ~r0=...) at /home/ozi/src/gopath/pkg/mod/gopkg.in/vmihailenco/[email protected]/decode.go:37
...
(gdb) p bytes.array
$8 = (uint8 *) 0xc000016d00 "\330\002\310\360\372\037\332)C\214\240@9?\021&"

I see three ways:

1. We can leave it as it is. We can recommend to provide manually methods for encoding-decoding structures with UUID for msgpack.v2. Like in the tests:

go-tarantool/uuid/uuid_test.go

Lines 35 to 50 in 460bf88

	func (t *TupleUUID) DecodeMsgpack(d *decoder) error {
	var err error
	var l int
	if l, err = d.DecodeArrayLen(); err != nil {
	return err
	}
	if l != 1 {
	return fmt.Errorf("array len doesn't match: %d", l)
	}
	res, err := d.DecodeInterface()
	if err != nil {
	return err
	}
	t.id = res.(uuid.UUID)
	return nil
	}

2. We can add a function that would switch global behavior. This is useless for msgpack.v5, but we will keep backward compatibility.

3. We can implement a custom type UUID and to implement Marshaler and Unmarshaler interface for it. As we have already done with Datetime and Decimal. But this breaks backward compatibility.

[oleg-jukovec]

A commit with a test for the issue:
fc812c4

[oleg-jukovec]

Unfortunately, these are problems with msgpack.v2 library (we found some more bugs with external types). It can only be fixed by updating to msgpack/v5.

We will do in the next major update of the connector.

[oleg-jukovec]

To solve the problem, a user need to use the build tag: go_tarantool_msgpack_v5 or wait until v2 version of the connector. Work on v2 has begun, it may be completed in the next quarter or half-year.