From f613c919d2eb0f3ba20d4dfed076743ea4191ea5 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 12 Jun 2020 17:14:00 -0400 Subject: [PATCH 1/2] decode: recursively unslice opaque config Also handle []interface{} in HookWeakDecodeFromSlice Without this change only the top level []map[string]interface{} will be unpacked as a single item. With this change any nested config will be unpacked. --- lib/decode/decode.go | 44 +++++++++++++++++++++++++++++++-------- lib/decode/decode_test.go | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/lib/decode/decode.go b/lib/decode/decode.go index b098e4313..26eab31a5 100644 --- a/lib/decode/decode.go +++ b/lib/decode/decode.go @@ -7,6 +7,8 @@ package decode import ( "reflect" "strings" + + "github.com/mitchellh/reflectwalk" ) // HookTranslateKeys is a mapstructure decode hook which translates keys in a @@ -120,15 +122,39 @@ func HookWeakDecodeFromSlice(from, to reflect.Type, data interface{}) (interface switch d := data.(type) { case []map[string]interface{}: - switch { - case len(d) == 0: - return nil, nil - case len(d) == 1: - return d[0], nil - default: - return data, nil + if len(d) == 1 { + return unSlice(d[0]) + } + // the JSON decoder can apparently decode slices as []interface{} + case []interface{}: + if len(d) == 1 { + return unSlice(d[0]) } - default: - return data, nil } + return data, nil +} + +func unSlice(data interface{}) (interface{}, error) { + err := reflectwalk.Walk(data, &unSliceWalker{}) + return data, err +} + +type unSliceWalker struct{} + +func (u *unSliceWalker) Map(_ reflect.Value) error { + return nil +} + +func (u *unSliceWalker) MapElem(m, k, v reflect.Value) error { + if !v.IsValid() || v.Kind() != reflect.Interface { + return nil + } + + v = v.Elem() // unpack the value from the interface{} + if v.Kind() != reflect.Slice || v.Len() != 1 { + return nil + } + + m.SetMapIndex(k, v.Index(0)) + return nil } diff --git a/lib/decode/decode_test.go b/lib/decode/decode_test.go index cc2ca1bfb..2fcf5448a 100644 --- a/lib/decode/decode_test.go +++ b/lib/decode/decode_test.go @@ -272,3 +272,39 @@ item { } require.Equal(t, target, expected) } + +func TestHookWeakDecodeFromSlice_NestedOpaqueConfig(t *testing.T) { + source := ` +service { + proxy { + config { + envoy_gateway_bind_addresses { + all-interfaces { + address = "0.0.0.0" + port = 8443 + } + } + } + } +}` + + target := map[string]interface{}{} + err := decodeHCLToMapStructure(source, &target) + require.NoError(t, err) + + expected := map[string]interface{}{ + "service": map[string]interface{}{ + "proxy": map[string]interface{}{ + "config": map[string]interface{}{ + "envoy_gateway_bind_addresses": map[string]interface{}{ + "all-interfaces": map[string]interface{}{ + "address": "0.0.0.0", + "port": 8443, + }, + }, + }, + }, + }, + } + require.Equal(t, target, expected) +} From dad8f29d4ee6ff9a63a439144b9630419f77a133 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 15 Jun 2020 12:22:54 -0400 Subject: [PATCH 2/2] decode: Only recursively unslice when the target is an interface{} --- lib/decode/decode.go | 38 +++++++++++++++++++------- lib/decode/decode_test.go | 56 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/lib/decode/decode.go b/lib/decode/decode.go index 26eab31a5..6730d47f8 100644 --- a/lib/decode/decode.go +++ b/lib/decode/decode.go @@ -94,11 +94,13 @@ func canonicalFieldKey(field reflect.StructField) string { return parts[0] } -// HookWeakDecodeFromSlice looks for []map[string]interface{} in the source -// data. If the target is not a slice or array it attempts to unpack 1 item -// out of the slice. If there are more items the source data is left unmodified, -// allowing mapstructure to handle and report the decode error caused by -// mismatched types. +// HookWeakDecodeFromSlice looks for []map[string]interface{} and []interface{} +// in the source data. If the target is not a slice or array it attempts to unpack +// 1 item out of the slice. If there are more items the source data is left +// unmodified, allowing mapstructure to handle and report the decode error caused by +// mismatched types. The []interface{} is handled so that all slice types are +// behave the same way, and for the rare case when a raw structure is re-encoded +// to JSON, which will produce the []interface{}. // // If this hook is being used on a "second pass" decode to decode an opaque // configuration into a type, the DecodeConfig should set WeaklyTypedInput=true, @@ -122,18 +124,31 @@ func HookWeakDecodeFromSlice(from, to reflect.Type, data interface{}) (interface switch d := data.(type) { case []map[string]interface{}: - if len(d) == 1 { + switch { + case len(d) != 1: + return data, nil + case to == typeOfEmptyInterface: return unSlice(d[0]) + default: + return d[0], nil } - // the JSON decoder can apparently decode slices as []interface{} + + // a slice map be encoded as []interface{} in some cases case []interface{}: - if len(d) == 1 { + switch { + case len(d) != 1: + return data, nil + case to == typeOfEmptyInterface: return unSlice(d[0]) + default: + return d[0], nil } } return data, nil } +var typeOfEmptyInterface = reflect.TypeOf((*interface{})(nil)).Elem() + func unSlice(data interface{}) (interface{}, error) { err := reflectwalk.Walk(data, &unSliceWalker{}) return data, err @@ -155,6 +170,11 @@ func (u *unSliceWalker) MapElem(m, k, v reflect.Value) error { return nil } - m.SetMapIndex(k, v.Index(0)) + first := v.Index(0) + // The value should always be assignable, but double check to avoid a panic. + if !first.Type().AssignableTo(m.Type().Elem()) { + return nil + } + m.SetMapIndex(k, first) return nil } diff --git a/lib/decode/decode_test.go b/lib/decode/decode_test.go index 2fcf5448a..72e012b39 100644 --- a/lib/decode/decode_test.go +++ b/lib/decode/decode_test.go @@ -198,9 +198,11 @@ func TestTranslationsForType(t *testing.T) { } type nested struct { - O map[string]interface{} - Slice []Item - Item Item + O map[string]interface{} + Slice []Item + Item Item + OSlice []map[string]interface{} + Sub *nested } type Item struct { @@ -215,6 +217,14 @@ slice { slice { name = "second" } +item { + name = "solo" +} +sub { + oslice { + something = "v1" + } +} ` target := &nested{} err := decodeHCLToMapStructure(source, target) @@ -222,6 +232,12 @@ slice { expected := &nested{ Slice: []Item{{Name: "first"}, {Name: "second"}}, + Item: Item{Name: "solo"}, + Sub: &nested{ + OSlice: []map[string]interface{}{ + {"something": "v1"}, + }, + }, } require.Equal(t, target, expected) } @@ -242,6 +258,40 @@ func decodeHCLToMapStructure(source string, target interface{}) error { return decoder.Decode(&raw) } +func TestHookWeakDecodeFromSlice_DoesNotModifySliceTargetsFromSliceInterface(t *testing.T) { + raw := map[string]interface{}{ + "slice": []interface{}{map[string]interface{}{"name": "first"}}, + "item": []interface{}{map[string]interface{}{"name": "solo"}}, + "sub": []interface{}{ + map[string]interface{}{ + "OSlice": []interface{}{ + map[string]interface{}{"something": "v1"}, + }, + "item": []interface{}{map[string]interface{}{"name": "subitem"}}, + }, + }, + } + target := &nested{} + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: HookWeakDecodeFromSlice, + Result: target, + }) + require.NoError(t, err) + err = decoder.Decode(&raw) + + expected := &nested{ + Slice: []Item{{Name: "first"}}, + Item: Item{Name: "solo"}, + Sub: &nested{ + OSlice: []map[string]interface{}{ + {"something": "v1"}, + }, + Item: Item{Name: "subitem"}, + }, + } + require.Equal(t, target, expected) +} + func TestHookWeakDecodeFromSlice_ErrorsWithMultipleNestedBlocks(t *testing.T) { source := ` item {