From e256817b532478359a33e80ddeaf7080b4f896b4 Mon Sep 17 00:00:00 2001 From: Dmitry Zenovich Date: Thu, 28 Feb 2019 20:37:11 +0300 Subject: [PATCH 1/3] nils should be accepted as zero values of any type only when WeaklyTypedInput=true --- mapstructure.go | 6 +++ mapstructure_bugs_test.go | 7 +-- mapstructure_test.go | 110 +++++++++++++++++++++++++++++++++----- 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index 256ee63f..d7b9bf9c 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -77,6 +77,7 @@ type DecoderConfig struct { // - single values are converted to slices if required. Each // element is weakly decoded. For example: "4" can become []int{4} // if the target type is an int slice. + // - nils are accepted as zero values of any type // WeaklyTypedInput bool @@ -236,6 +237,11 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e } if input == nil { + // We convert nils into zero values only if WeaklyTypedInput is set to true + if !d.config.WeaklyTypedInput && outVal.Kind() != reflect.Ptr && outVal.Kind() != reflect.Struct { + return fmt.Errorf("'%s' should not be null (expected type: %s)", name, outVal.Kind()) + } + // If the data is nil, then we don't set anything, unless ZeroFields is set // to true. if d.config.ZeroFields { diff --git a/mapstructure_bugs_test.go b/mapstructure_bugs_test.go index ba6590a6..896adeeb 100644 --- a/mapstructure_bugs_test.go +++ b/mapstructure_bugs_test.go @@ -110,9 +110,10 @@ func TestDecode_NilValue(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { config := &DecoderConfig{ - Metadata: new(Metadata), - Result: tc.target, - ZeroFields: true, + Metadata: new(Metadata), + Result: tc.target, + ZeroFields: true, + WeaklyTypedInput: true, } decoder, err := NewDecoder(config) diff --git a/mapstructure_test.go b/mapstructure_test.go index 27ac10eb..952e7b18 100644 --- a/mapstructure_test.go +++ b/mapstructure_test.go @@ -239,6 +239,84 @@ func TestBasicTypes(t *testing.T) { } } +func TestNullPointers(t *testing.T) { + t.Parallel() + + input := map[string]interface{}{ + "vstring": nil, + "vint": nil, + "Vuint": nil, + "vbool": nil, + "Vfloat": nil, + "vsilent": nil, + "vdata": nil, + "vjsonInt": nil, + "vjsonFloat": nil, + "vjsonNumber": nil, + } + + var result Basic + err := Decode(input, &result) + + if err == nil { + t.Errorf("should be an error") + t.FailNow() + } + if !strings.Contains(err.Error(), "'Vstring' should not be null (expected type: string)") { + t.Errorf("no error for 'Vstring' in %#v", err) + } + if !strings.Contains(err.Error(), "'Vint' should not be null (expected type: int)") { + t.Errorf("no error for 'Vint' in %#v", err) + } + if !strings.Contains(err.Error(), "'Vbool' should not be null (expected type: bool)") { + t.Errorf("no error for 'Vbool' in %#v", err) + } + if !strings.Contains(err.Error(), "'Vfloat' should not be null (expected type: float64)") { + t.Errorf("no error for 'Vfloat' in %#v", err) + } + if !strings.Contains(err.Error(), "'Vdata' should not be null (expected type: interface)") { + t.Errorf("no error for 'Vdata' in %#v", err) + } + if !strings.Contains(err.Error(), "'VjsonInt' should not be null (expected type: int)") { + t.Errorf("no error for 'VjsonInt' in %#v", err) + } + if !strings.Contains(err.Error(), "'VjsonFloat' should not be null (expected type: float64)") { + t.Errorf("no error for 'VjsonFloat' in %#v", err) + } + if !strings.Contains(err.Error(), "'VjsonNumber' should not be null (expected type: string)") { + t.Errorf("no error for 'VjsonNumber' in %#v", err) + } +} + +func TestNullPointers_WeakDecode(t *testing.T) { + t.Parallel() + + input := map[string]interface{}{ + "vstring": nil, + "vint": nil, + "Vuint": nil, + "vbool": nil, + "Vfloat": nil, + "vsilent": nil, + "vdata": nil, + "vjsonInt": nil, + "vjsonFloat": nil, + "vjsonNumber": nil, + } + + var result Basic + err := WeakDecode(input, &result) + + if err != nil { + t.Fatalf("got an err: %s", err) + } + + expected := Basic{} + if result != expected { + t.Errorf("result is not a zero structure: %#v", result) + } +} + func TestBasic_IntWithFloat(t *testing.T) { t.Parallel() @@ -1469,21 +1547,29 @@ func TestDecodeTable(t *testing.T) { { "basic pointer to non-pointer", &BasicPointer{ - Vstring: stringPtr("vstring"), - Vint: intPtr(2), - Vuint: uintPtr(3), - Vbool: boolPtr(true), - Vfloat: floatPtr(4.56), - Vdata: interfacePtr([]byte("data")), + Vstring: stringPtr("vstring"), + Vint: intPtr(2), + Vuint: uintPtr(3), + Vbool: boolPtr(true), + Vfloat: floatPtr(4.56), + Vdata: interfacePtr([]byte("data")), + Vextra: stringPtr("extra"), + VjsonFloat: floatPtr(1.1), + VjsonInt: intPtr(5), + VjsonNumber: func() *json.Number { n := json.Number(123); return &n }(), }, &Basic{}, &Basic{ - Vstring: "vstring", - Vint: 2, - Vuint: 3, - Vbool: true, - Vfloat: 4.56, - Vdata: []byte("data"), + Vstring: "vstring", + Vint: 2, + Vuint: 3, + Vbool: true, + Vfloat: 4.56, + Vdata: []byte("data"), + Vextra: "extra", + VjsonFloat: 1.1, + VjsonInt: 5, + VjsonNumber: json.Number(123), }, false, }, From 8b645d42c418627804bb8d71f4ac9d72827a452c Mon Sep 17 00:00:00 2001 From: Dmitry Zenovich Date: Thu, 28 Feb 2019 20:44:01 +0300 Subject: [PATCH 2/3] add one more test --- mapstructure_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/mapstructure_test.go b/mapstructure_test.go index 952e7b18..a088a9a0 100644 --- a/mapstructure_test.go +++ b/mapstructure_test.go @@ -317,6 +317,35 @@ func TestNullPointers_WeakDecode(t *testing.T) { } } +func TestNullPointers_DecodeIntoPointers(t *testing.T) { + t.Parallel() + + input := map[string]interface{}{ + "vstring": nil, + "vint": nil, + "Vuint": nil, + "vbool": nil, + "Vfloat": nil, + "vsilent": nil, + "vdata": nil, + "vjsonInt": nil, + "vjsonFloat": nil, + "vjsonNumber": nil, + } + + var result BasicPointer + err := Decode(input, &result) + + if err != nil { + t.Fatalf("got an err: %s", err) + } + + expected := BasicPointer{} + if result != expected { + t.Errorf("result is not a zero structure: %#v", result) + } +} + func TestBasic_IntWithFloat(t *testing.T) { t.Parallel() From 5f275cb00a60611708efdbd462ffb0b8ec05aede Mon Sep 17 00:00:00 2001 From: Dmitry Zenovich Date: Fri, 1 Mar 2019 11:43:30 +0300 Subject: [PATCH 3/3] strictly typed input: disallow nils for structs, but allow them for interfaces --- mapstructure.go | 2 +- mapstructure_test.go | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index d7b9bf9c..52aa2c3e 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -238,7 +238,7 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e if input == nil { // We convert nils into zero values only if WeaklyTypedInput is set to true - if !d.config.WeaklyTypedInput && outVal.Kind() != reflect.Ptr && outVal.Kind() != reflect.Struct { + if !d.config.WeaklyTypedInput && outVal.Kind() != reflect.Ptr && outVal.Kind() != reflect.Interface { return fmt.Errorf("'%s' should not be null (expected type: %s)", name, outVal.Kind()) } diff --git a/mapstructure_test.go b/mapstructure_test.go index a088a9a0..7e51f0f2 100644 --- a/mapstructure_test.go +++ b/mapstructure_test.go @@ -274,8 +274,8 @@ func TestNullPointers(t *testing.T) { if !strings.Contains(err.Error(), "'Vfloat' should not be null (expected type: float64)") { t.Errorf("no error for 'Vfloat' in %#v", err) } - if !strings.Contains(err.Error(), "'Vdata' should not be null (expected type: interface)") { - t.Errorf("no error for 'Vdata' in %#v", err) + if strings.Contains(err.Error(), "Vdata") { + t.Errorf("got error for 'Vdata' in %#v", err) } if !strings.Contains(err.Error(), "'VjsonInt' should not be null (expected type: int)") { t.Errorf("no error for 'VjsonInt' in %#v", err) @@ -716,6 +716,24 @@ func TestDecode_Nil(t *testing.T) { } err := Decode(input, &result) + if err == nil { + t.Fatalf("should be an error") + } + + if !strings.Contains(err.Error(), "'' should not be null (expected type: struct)") { + t.Fatalf("unexpected error message: %s", err) + } +} + +func TestDecode_Nil_Weak(t *testing.T) { + t.Parallel() + + var input interface{} + result := Basic{ + Vstring: "foo", + } + + err := WeakDecode(input, &result) if err != nil { t.Fatalf("err: %s", err) }