From cd1c879818a5d4cc3513f335007d328800e639e7 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 20 Sep 2024 23:58:05 -0400 Subject: [PATCH 1/4] Pass appropriate empty Value to hooks Signed-off-by: Yuri Shkuro --- mapstructure.go | 74 +++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index 05bc140..76e8f8d 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -442,22 +442,33 @@ func (d *Decoder) Decode(input interface{}) error { return err } -// Decodes an unknown data type into a specific reflection value. -func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) error { - var inputVal reflect.Value - if input != nil { - inputVal = reflect.ValueOf(input) - - // We need to check here if input is a typed nil. Typed nils won't - // match the "input == nil" below so we check that here. - if inputVal.Kind() == reflect.Ptr && inputVal.IsNil() { - input = nil - } +// A comparison input == nil will fail if input is a typed nil. +// This function converts a typed nil to an actual, untyped nil. +func toRealNil(input interface{}) interface{} { + if input == nil { + return nil } + val := reflect.ValueOf(input) + k := val.Kind() + if (k == reflect.Ptr || + k == reflect.Interface || + k == reflect.Map || + k == reflect.Slice || + k == reflect.Array) && val.IsNil() { + return nil + } + return input +} - decodeNil := d.config.DecodeNil && d.config.DecodeHook != nil - - if input == nil { +// Decodes an unknown data type into a specific reflection value. +func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) error { + var ( + inputVal = reflect.ValueOf(input) + outputKind = getKind(outVal) + decodeNil = d.config.DecodeNil && d.cachedDecodeHook != nil + ) + input = toRealNil(input) + if input == nil || !inputVal.IsValid() { // If the data is nil, then we don't set anything, unless ZeroFields is set // to true. if d.config.ZeroFields { @@ -467,40 +478,37 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e d.config.Metadata.Keys = append(d.config.Metadata.Keys, name) } } - if !decodeNil { return nil } } - if !inputVal.IsValid() { - if !decodeNil { - // If the input value is invalid, then we just set the value - // to be the zero value. - outVal.Set(reflect.Zero(outVal.Type())) - if d.config.Metadata != nil && name != "" { - d.config.Metadata.Keys = append(d.config.Metadata.Keys, name) - } - return nil - } - - // If we get here, we have an untyped nil so the type of the input is assumed. - // We do this because all subsequent code requires a valid value for inputVal. - var mapVal map[string]interface{} - inputVal = reflect.MakeMap(reflect.TypeOf(mapVal)) - } - if d.cachedDecodeHook != nil { // We have a DecodeHook, so let's pre-process the input. + if !inputVal.IsValid() { + // Hooks need a valid inputVal, so reset it to zero value of outVal type. + switch outputKind { + case reflect.Struct, reflect.Map: + var mapVal map[string]interface{} + inputVal = reflect.ValueOf(mapVal) + case reflect.Slice, reflect.Array: + var sliceVal []interface{} + inputVal = reflect.ValueOf(sliceVal) + default: + inputVal = reflect.Zero(outVal.Type()) + } + } var err error input, err = d.cachedDecodeHook(inputVal, outVal) if err != nil { return fmt.Errorf("error decoding '%s': %w", name, err) } } + if toRealNil(input) == nil { + return nil + } var err error - outputKind := getKind(outVal) addMetaKey := true switch outputKind { case reflect.Bool: From b0e65b30997c3f9e72b4b2d58b281377853fadbf Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 22 Sep 2024 17:05:50 -0400 Subject: [PATCH 2/4] fix Signed-off-by: Yuri Shkuro --- mapstructure.go | 71 +++++++++++++++++++++++++------------------- mapstructure_test.go | 67 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 31 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index 76e8f8d..b6b3be6 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -442,22 +442,15 @@ func (d *Decoder) Decode(input interface{}) error { return err } -// A comparison input == nil will fail if input is a typed nil. -// This function converts a typed nil to an actual, untyped nil. -func toRealNil(input interface{}) interface{} { +// isNil returns true if the input is nil or a typed nil pointer. +func isNil(input interface{}) bool { if input == nil { - return nil + return true } val := reflect.ValueOf(input) k := val.Kind() - if (k == reflect.Ptr || - k == reflect.Interface || - k == reflect.Map || - k == reflect.Slice || - k == reflect.Array) && val.IsNil() { - return nil - } - return input + return (k == reflect.Ptr || + /*k == reflect.Interface || k == reflect.Map || k == reflect.Slice*/ false) && val.IsNil() } // Decodes an unknown data type into a specific reflection value. @@ -467,8 +460,14 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e outputKind = getKind(outVal) decodeNil = d.config.DecodeNil && d.cachedDecodeHook != nil ) - input = toRealNil(input) - if input == nil || !inputVal.IsValid() { + if input != nil { + // We need to check here if input is a typed nil. Typed nils won't + // match the "input == nil" below so we check that here. + if inputVal.Kind() == reflect.Ptr && inputVal.IsNil() { + input = nil + } + } + if input == nil { // If the data is nil, then we don't set anything, unless ZeroFields is set // to true. if d.config.ZeroFields { @@ -482,29 +481,41 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e return nil } } + if !inputVal.IsValid() { + if !decodeNil { + // If the input value is invalid, then we just set the value + // to be the zero value. + outVal.Set(reflect.Zero(outVal.Type())) + if d.config.Metadata != nil && name != "" { + d.config.Metadata.Keys = append(d.config.Metadata.Keys, name) + } + return nil + } + // Hooks need a valid inputVal, so reset it to zero value of outVal type. + switch outputKind { + case reflect.Struct, reflect.Map: + // create empty map + var mapVal map[string]interface{} + inputVal = reflect.ValueOf(mapVal) + // inputVal = reflect.MakeMap(reflect.TypeOf(mapVal)) + case reflect.Slice, reflect.Array: + // create nil slice + var sliceVal []interface{} + inputVal = reflect.ValueOf(sliceVal) + default: + inputVal = reflect.Zero(outVal.Type()) + } + } if d.cachedDecodeHook != nil { // We have a DecodeHook, so let's pre-process the input. - if !inputVal.IsValid() { - // Hooks need a valid inputVal, so reset it to zero value of outVal type. - switch outputKind { - case reflect.Struct, reflect.Map: - var mapVal map[string]interface{} - inputVal = reflect.ValueOf(mapVal) - case reflect.Slice, reflect.Array: - var sliceVal []interface{} - inputVal = reflect.ValueOf(sliceVal) - default: - inputVal = reflect.Zero(outVal.Type()) - } - } var err error input, err = d.cachedDecodeHook(inputVal, outVal) if err != nil { return fmt.Errorf("error decoding '%s': %w", name, err) } } - if toRealNil(input) == nil { + if isNil(input) { return nil } @@ -789,8 +800,8 @@ func (d *Decoder) decodeBool(name string, data interface{}, val reflect.Value) e } default: return fmt.Errorf( - "'%s' expected type '%s', got unconvertible type '%s', value: '%v'", - name, val.Type(), dataVal.Type(), data) + "'%s' expected type '%s', got unconvertible type '%#v', value: '%#v'", + name, val, dataVal, data) } return nil diff --git a/mapstructure_test.go b/mapstructure_test.go index e30ff47..e2466f8 100644 --- a/mapstructure_test.go +++ b/mapstructure_test.go @@ -3083,12 +3083,13 @@ func TestDecoder_IgnoreUntaggedFieldsWithStruct(t *testing.T) { } } -func TestDecoder_CanPerformDecodingForNilInputs(t *testing.T) { +func TestDecoder_DecodeNilOption(t *testing.T) { t.Parallel() type Transformed struct { Message string When string + Boolean *bool // } helloHook := func(reflect.Type, reflect.Type, interface{}) (interface{}, error) { @@ -3100,6 +3101,9 @@ func TestDecoder_CanPerformDecodingForNilInputs(t *testing.T) { appendHook := func(from reflect.Value, to reflect.Value) (interface{}, error) { if from.Kind() == reflect.Map { stringMap := from.Interface().(map[string]interface{}) + if stringMap == nil { + stringMap = make(map[string]interface{}) + } stringMap["when"] = "see you later" return stringMap, nil } @@ -3248,6 +3252,67 @@ func TestDecoder_CanPerformDecodingForNilInputs(t *testing.T) { } } +func TestDecoder_ExpandNilStructPointersHookFunc(t *testing.T) { + // a decoder hook that expands nil pointers in a struct to their zero value + // if the input map contains the corresponding key. + decodeHook := func(from reflect.Value, to reflect.Value) (any, error) { + if from.Kind() == reflect.Map && to.Kind() == reflect.Map { + toElem := to.Type().Elem() + if toElem.Kind() == reflect.Ptr && toElem.Elem().Kind() == reflect.Struct { + fromRange := from.MapRange() + for fromRange.Next() { + fromKey := fromRange.Key() + fromValue := fromRange.Value() + if fromValue.IsNil() { + newFromValue := reflect.New(toElem.Elem()) + from.SetMapIndex(fromKey, newFromValue) + } + } + } + } + return from.Interface(), nil + } + type Struct struct { + Name string + } + type TestConfig struct { + Boolean *bool `mapstructure:"boolean"` + Struct *Struct `mapstructure:"struct"` + MapStruct map[string]*Struct `mapstructure:"map_struct"` + } + stringMap := map[string]any{ + "boolean": nil, + "struct": nil, + "map_struct": map[string]any{ + "struct": nil, + }, + } + var result TestConfig + decoder, err := NewDecoder(&DecoderConfig{ + Result: &result, + DecodeNil: true, + DecodeHook: decodeHook, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + if err := decoder.Decode(stringMap); err != nil { + t.Fatalf("got an err: %s", err) + } + if result.Boolean != nil { + t.Errorf("nil Boolean expected, got '%#v'", result.Boolean) + } + if result.Struct != nil { + t.Errorf("nil Struct expected, got '%#v'", result.Struct) + } + if len(result.MapStruct) == 0 { + t.Fatalf("not-empty MapStruct expected, got '%#v'", result.MapStruct) + } + if _, ok := result.MapStruct["struct"]; !ok { + t.Errorf("MapStruct['struct'] expected") + } +} + func testSliceInput(t *testing.T, input map[string]interface{}, expected *Slice) { var result Slice err := Decode(input, &result) From 0a04d624f38ed48437a356b61fc89924d9f6b88f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 22 Sep 2024 17:14:01 -0400 Subject: [PATCH 3/4] fix Signed-off-by: Yuri Shkuro --- mapstructure.go | 13 ++++--------- mapstructure_test.go | 1 - 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index b6b3be6..a8a18ea 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -448,9 +448,7 @@ func isNil(input interface{}) bool { return true } val := reflect.ValueOf(input) - k := val.Kind() - return (k == reflect.Ptr || - /*k == reflect.Interface || k == reflect.Map || k == reflect.Slice*/ false) && val.IsNil() + return val.Kind() == reflect.Ptr && val.IsNil() } // Decodes an unknown data type into a specific reflection value. @@ -460,12 +458,9 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e outputKind = getKind(outVal) decodeNil = d.config.DecodeNil && d.cachedDecodeHook != nil ) - if input != nil { - // We need to check here if input is a typed nil. Typed nils won't - // match the "input == nil" below so we check that here. - if inputVal.Kind() == reflect.Ptr && inputVal.IsNil() { - input = nil - } + if isNil(input) { + // Typed nils won't match the "input == nil" below, so reset input. + input = nil } if input == nil { // If the data is nil, then we don't set anything, unless ZeroFields is set diff --git a/mapstructure_test.go b/mapstructure_test.go index e2466f8..519e722 100644 --- a/mapstructure_test.go +++ b/mapstructure_test.go @@ -3089,7 +3089,6 @@ func TestDecoder_DecodeNilOption(t *testing.T) { type Transformed struct { Message string When string - Boolean *bool // } helloHook := func(reflect.Type, reflect.Type, interface{}) (interface{}, error) { From b5334ceb289a1c4d587f7253d5dedb5161545f6b Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 22 Sep 2024 17:29:10 -0400 Subject: [PATCH 4/4] cleanup Signed-off-by: Yuri Shkuro --- mapstructure.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index a8a18ea..e77e63b 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -489,14 +489,11 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e // Hooks need a valid inputVal, so reset it to zero value of outVal type. switch outputKind { case reflect.Struct, reflect.Map: - // create empty map var mapVal map[string]interface{} - inputVal = reflect.ValueOf(mapVal) - // inputVal = reflect.MakeMap(reflect.TypeOf(mapVal)) + inputVal = reflect.ValueOf(mapVal) // create nil map pointer case reflect.Slice, reflect.Array: - // create nil slice var sliceVal []interface{} - inputVal = reflect.ValueOf(sliceVal) + inputVal = reflect.ValueOf(sliceVal) // create nil slice pointer default: inputVal = reflect.Zero(outVal.Type()) }