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

Ensure that intermediate maps during struct to struct decoding are settable #203

Merged
merged 1 commit into from Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 26 additions & 5 deletions mapstructure.go
Expand Up @@ -906,11 +906,22 @@ func (d *Decoder) decodeMapFromStruct(name string, dataVal reflect.Value, val re
mType := reflect.MapOf(vKeyType, vElemType)
vMap := reflect.MakeMap(mType)

err := d.decode(keyName, x.Interface(), vMap)
// Creating a pointer to a map so that other methods can completely
// overwrite the map if need be (looking at you decodeMapFromMap). The
// indirection allows the underlying map to be settable (CanSet() == true)
// where as reflect.MakeMap returns an unsettable map.
addrVal := reflect.New(vMap.Type())
reflect.Indirect(addrVal).Set(vMap)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you know its a pointer I usually prefer addrVal.Elem() over indirect. Indirect just calls that with an additional check but it feels more idiomatic reflect usage. Same with the few other uses.


err := d.decode(keyName, x.Interface(), reflect.Indirect(addrVal))
if err != nil {
return err
}

// the underlying map may have been completely overwritten so pull
// it indirectly out of the enclosing value.
vMap = reflect.Indirect(addrVal)

if squash {
for _, k := range vMap.MapKeys() {
valMap.SetMapIndex(k, vMap.MapIndex(k))
Expand Down Expand Up @@ -1154,13 +1165,23 @@ func (d *Decoder) decodeStruct(name string, data interface{}, val reflect.Value)
// Not the most efficient way to do this but we can optimize later if
// we want to. To convert from struct to struct we go to map first
// as an intermediary.
m := make(map[string]interface{})
mval := reflect.Indirect(reflect.ValueOf(&m))
if err := d.decodeMapFromStruct(name, dataVal, mval, mval); err != nil {

// Make a new map to hold our result
mapType := reflect.TypeOf((map[string]interface{})(nil))
mval := reflect.MakeMap(mapType)

// Creating a pointer to a map so that other methods can completely
// overwrite the map if need be (looking at you decodeMapFromMap). The
// indirection allows the underlying map to be settable (CanSet() == true)
// where as reflect.MakeMap returns an unsettable map.
addrVal := reflect.New(mval.Type())

reflect.Indirect(addrVal).Set(mval)
if err := d.decodeMapFromStruct(name, dataVal, reflect.Indirect(addrVal), mval); err != nil {
return err
}

result := d.decodeStructFromMap(name, mval, val)
result := d.decodeStructFromMap(name, reflect.Indirect(addrVal), val)
return result

default:
Expand Down
64 changes: 64 additions & 0 deletions mapstructure_bugs_test.go
Expand Up @@ -3,6 +3,7 @@ package mapstructure
import (
"reflect"
"testing"
"time"
)

// GH-1, GH-10, GH-96
Expand Down Expand Up @@ -475,3 +476,66 @@ func TestDecodeBadDataTypeInSlice(t *testing.T) {
t.Error("An error was expected, got nil")
}
}

// #202 Ensure that intermediate maps in the struct -> struct decode process are settable
// and not just the elements within them.
func TestDecodeIntermeidateMapsSettable(t *testing.T) {
type Timestamp struct {
Seconds int64
Nanos int32
}

type TsWrapper struct {
Timestamp *Timestamp
}

type TimeWrapper struct {
Timestamp time.Time
}

input := TimeWrapper{
Timestamp: time.Unix(123456789, 987654),
}

expected := TsWrapper{
Timestamp: &Timestamp{
Seconds: 123456789,
Nanos: 987654,
},
}

timePtrType := reflect.TypeOf((*time.Time)(nil))
mapStrInfType := reflect.TypeOf((map[string]interface{})(nil))

var actual TsWrapper
decoder, err := NewDecoder(&DecoderConfig{
Result: &actual,
DecodeHook: func(from, to reflect.Type, data interface{}) (interface{}, error) {
if from == timePtrType && to == mapStrInfType {
ts := data.(*time.Time)
nanos := ts.UnixNano()

seconds := nanos / 1000000000
nanos = nanos % 1000000000

return &map[string]interface{}{
"Seconds": seconds,
"Nanos": int32(nanos),
}, nil
}
return data, nil
},
})

if err != nil {
t.Fatalf("failed to create decoder: %v", err)
}

if err := decoder.Decode(&input); err != nil {
t.Fatalf("failed to decode input: %v", err)
}

if !reflect.DeepEqual(expected, actual) {
t.Fatalf("expected: %#[1]v (%[1]T), got: %#[2]v (%[2]T)", expected, actual)
}
}