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

[decode] regression from v4: invalid code=d3 decoding string/bytes length on complex nested structure #327

Open
lizthegrey opened this issue Dec 5, 2021 · 1 comment
Assignees
Labels

Comments

@lizthegrey
Copy link

lizthegrey commented Dec 5, 2021

Expected Behavior (v4)

https://go.dev/play/p/BXgPx5l-SDf

Structure roundtrips successfully.

[142 161 105 192 161 103 202 64 160 0 0 161 97 210 0 0 0 5 161 106 129 161 97 211 0 0 0 0 0 0 0 1 161 109 129 211 0 0 0 0 0 0 0 1 129 161 50 129 211 0 0 0 0 0 0 0 3 211 0 0 0 0 0 0 0 4 161 98 207 0 0 0 0 0 0 0 5 161 107 146 211 0 0 0 0 0 0 0 1 211 0 0 0 0 0 0 0 2 161 108 129 211 0 0 0 0 0 0 0 1 129 203 64 0 0 0 0 0 0 0 203 64 8 0 0 0 0 0 0 161 110 129 161 110 192 161 99 204 5 161 100 195 161 101 163 102 111 111 161 102 196 3 102 111 111 161 104 203 127 240 0 0 0 0 0 0]
<nil>
map[a:5 b:5 c:5 d:true e:foo f:[102 111 111] g:5 h:+Inf i:<nil> j:map[a:1] k:[1 2] l:map[1:map[2:3]] m:map[1:map[2:map[3:4]]] n:map[n:<nil>]]

Current Behavior (v5)

Decode error happens when roundtripping

https://go.dev/play/p/5hcEjhSWohg

[142 161 101 163 102 111 111 161 104 203 127 240 0 0 0 0 0 0 161 108 129 211 0 0 0 0 0 0 0 1 129 203 64 0 0 0 0 0 0 0 203 64 8 0 0 0 0 0 0 161 98 207 0 0 0 0 0 0 0 5 161 100 195 161 102 196 3 102 111 111 161 107 146 211 0 0 0 0 0 0 0 1 211 0 0 0 0 0 0 0 2 161 105 192 161 99 204 5 161 106 129 161 97 1 161 97 210 0 0 0 5 161 103 202 64 160 0 0 161 109 129 211 0 0 0 0 0 0 0 1 129 161 50 129 211 0 0 0 0 0 0 0 3 211 0 0 0 0 0 0 0 4 161 110 129 161 110 192]
msgpack: invalid code=d3 decoding string/bytes length

Steps to Reproduce

See above

v4 encode -> v5 decode has the error. https://go.dev/play/p/gYEuy6p42C6
v5 encode -> v4 decode does not have the error. https://go.dev/play/p/fQBCJZiW03f

@lizthegrey lizthegrey changed the title regression from v4: invalid code=d3 decoding string/bytes length on complex nested structure [decode] regression from v4: invalid code=d3 decoding string/bytes length on complex nested structure Dec 5, 2021
@vmihailenco vmihailenco self-assigned this Dec 16, 2021
@vmihailenco
Copy link
Owner

vmihailenco commented Jan 24, 2022

Hi,

The smaller reproducer looks like this:

package main

import (
	"bytes"
	"fmt"

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

func main() {
	var ret interface{}
	b, _ := v4.Marshal(map[string]interface{}{
		"2": map[int64]int64{
			3: 4,
		},
	})
	fmt.Println(b)
	decoder := v5.NewDecoder(bytes.NewReader(b))
	// decoder.SetMapDecoder(func(dec *v5.Decoder) (interface{}, error) {
	// 	return dec.DecodeTypedMap()
	// })
	decoder.UseLooseInterfaceDecoding(true)
	decoder.SetCustomStructTag("json")
	err := decoder.Decode(&ret)
	fmt.Println(err)
	fmt.Println(ret)
}

To fix it, uncomment the code that sets map decoder. This behavior is explained and documented here. I am not sure there is anything else we can do.

oleg-jukovec added a commit to tarantool/go-tarantool that referenced this issue May 19, 2022
An user can continue to use msgpack.v2 with tag:
go_tarantool_msgpack_v2

There are two problems that cannot be unified:

- Different signature and logic of RegisterExt in msgpack.v2[1] and
msgpack.v5[2].
- Different logic of map decoding, see[3][4].

These issues have been fixed in msgpack.v5 implementation, see
msgpack.go and uuid/msgpack.go.

Files naming changed:
msgpack.go - msgpack.v5 usage for the code
msgpack_helper_test.go - msgpack.v5 usage for tests
msgpack_v2.go - msgpack.v2 usage for the code
msgpack_v2_helper_test.go - msgpack.v2 usage for tests

1. https://pkg.go.dev/github.com/vmihailenco/msgpack@v2.9.2+incompatible#RegisterExt
2. https://pkg.go.dev/github.com/vmihailenco/msgpack/v5#RegisterExt
3. vmihailenco/msgpack#327
4. https://msgpack.uptrace.dev/guide/#decoding-maps-into-an-interface/
oleg-jukovec added a commit to tarantool/go-tarantool that referenced this issue May 19, 2022
An user can continue to use msgpack.v2 with tag:
go_tarantool_msgpack_v2

There are two problems that cannot be unified:

- Different signature and logic of RegisterExt in msgpack.v2[1] and
msgpack.v5[2].
- Different logic of map decoding, see[3][4].

These issues have been fixed in msgpack.v5 implementation, see
msgpack.go and uuid/msgpack.go.

Files naming changed:
msgpack.go - msgpack.v5 usage for the code
msgpack_helper_test.go - msgpack.v5 usage for tests
msgpack_v2.go - msgpack.v2 usage for the code
msgpack_v2_helper_test.go - msgpack.v2 usage for tests

1. https://pkg.go.dev/github.com/vmihailenco/msgpack@v2.9.2+incompatible#RegisterExt
2. https://pkg.go.dev/github.com/vmihailenco/msgpack/v5#RegisterExt
3. vmihailenco/msgpack#327
4. https://msgpack.uptrace.dev/guide/#decoding-maps-into-an-interface/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants