Skip to content

Commit

Permalink
Regression for non-addressable structs (#16)
Browse files Browse the repository at this point in the history
When we removed the default, unsafe mode, it caused a regression
where we are no longer able to deserialize things like

```go
map[string]struct{A string}
```

when we are deserializing into an already existing `struct` instance.
This is because the `struct` in the map is not addressable by default,
which caused a runtime panic. The previous unsafe version wrote
directly to the underlying memory and bypassed this check

We fix that by marking structs as an "immutable" type in the decoder,
forcing it to recreate the `struct` rather than try to write to
a non-addressable `struct`.

(This was found by testing `hashicorp/nomad`, which uses
`net-rpc-msgpack`, which uses this pattern of decoding two messages
into one value.)
  • Loading branch information
swenson committed Dec 7, 2022
1 parent 948591c commit 810d5fd
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
23 changes: 23 additions & 0 deletions codec/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3268,6 +3268,29 @@ func TestMultipleEncDec(t *testing.T) {
doTestMultipleEncDec(t, "json", testJsonH)
}

func encodeMsgPack(in interface{}) (*bytes.Buffer, error) {
buf := bytes.NewBuffer(nil)
hd := MsgpackHandle{}
enc := NewEncoder(buf, &hd)
err := enc.Encode(in)
return buf, err
}

func TestMapStructDoubleDecode(t *testing.T) {
// we should be able to decode into structs in a map
// if the struct is already present, it is not addressable, so has to be recreated
x := map[string]struct{ A string }{}
x["abc"] = struct{ A string }{"def"}
out, err := encodeMsgPack(x)
if err != nil {
t.Fatal(err)
}
err = decodeMsgPack(out.Bytes(), &x)
if err != nil {
t.Fatal(err)
}
}

// TODO:
//
// Add Tests for the following:
Expand Down
2 changes: 1 addition & 1 deletion codec/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ var immutableKindsSet = [32]bool{
// reflect.Ptr
// reflect.Slice
reflect.String: true,
// reflect.Struct
reflect.Struct: true,
// reflect.UnsafePointer
}

Expand Down

0 comments on commit 810d5fd

Please sign in to comment.