From a4f941368b58896af60f9ed52628df477708ad34 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Wed, 14 Mar 2018 19:34:23 -0400 Subject: [PATCH 1/2] Fix issue with deepcopy returning wrong type causing panic Signed-off-by: Guillaume J. Charmes --- helper/schema/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 6773fe584..43d0f0070 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -395,7 +395,7 @@ func (m *schemaMap) DeepCopy() schemaMap { if err != nil { panic(err) } - return copy.(schemaMap) + return *copy.(*schemaMap) } // Diff returns the diff for a resource given the schema map, From bfac8e0eec2a4d8be711b9b1c7a6017e9f4301d9 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Wed, 14 Mar 2018 20:13:36 -0400 Subject: [PATCH 2/2] Update vendor for DeepCopy issue Signed-off-by: Guillaume J. Charmes --- .../mitchellh/copystructure/copystructure.go | 87 ++++++++++++-- .../mitchellh/reflectwalk/location.go | 2 + .../mitchellh/reflectwalk/location_string.go | 8 +- .../mitchellh/reflectwalk/reflectwalk.go | 108 ++++++++++++++---- vendor/vendor.json | 12 +- 5 files changed, 176 insertions(+), 41 deletions(-) diff --git a/vendor/github.com/mitchellh/copystructure/copystructure.go b/vendor/github.com/mitchellh/copystructure/copystructure.go index 0e725ea72..140435255 100644 --- a/vendor/github.com/mitchellh/copystructure/copystructure.go +++ b/vendor/github.com/mitchellh/copystructure/copystructure.go @@ -156,9 +156,13 @@ func (w *walker) Exit(l reflectwalk.Location) error { } switch l { + case reflectwalk.Array: + fallthrough case reflectwalk.Map: fallthrough case reflectwalk.Slice: + w.replacePointerMaybe() + // Pop map off our container w.cs = w.cs[:len(w.cs)-1] case reflectwalk.MapValue: @@ -171,16 +175,27 @@ func (w *walker) Exit(l reflectwalk.Location) error { // or in this case never adds it. We need to create a properly typed // zero value so that this key can be set. if !mv.IsValid() { - mv = reflect.Zero(m.Type().Elem()) + mv = reflect.Zero(m.Elem().Type().Elem()) + } + m.Elem().SetMapIndex(mk, mv) + case reflectwalk.ArrayElem: + // Pop off the value and the index and set it on the array + v := w.valPop() + i := w.valPop().Interface().(int) + if v.IsValid() { + a := w.cs[len(w.cs)-1] + ae := a.Elem().Index(i) // storing array as pointer on stack - so need Elem() call + if ae.CanSet() { + ae.Set(v) + } } - m.SetMapIndex(mk, mv) case reflectwalk.SliceElem: // Pop off the value and the index and set it on the slice v := w.valPop() i := w.valPop().Interface().(int) if v.IsValid() { s := w.cs[len(w.cs)-1] - se := s.Index(i) + se := s.Elem().Index(i) if se.CanSet() { se.Set(v) } @@ -220,9 +235,9 @@ func (w *walker) Map(m reflect.Value) error { // Create the map. If the map itself is nil, then just make a nil map var newMap reflect.Value if m.IsNil() { - newMap = reflect.Indirect(reflect.New(m.Type())) + newMap = reflect.New(m.Type()) } else { - newMap = reflect.MakeMap(m.Type()) + newMap = wrapPtr(reflect.MakeMap(m.Type())) } w.cs = append(w.cs, newMap) @@ -287,9 +302,9 @@ func (w *walker) Slice(s reflect.Value) error { var newS reflect.Value if s.IsNil() { - newS = reflect.Indirect(reflect.New(s.Type())) + newS = reflect.New(s.Type()) } else { - newS = reflect.MakeSlice(s.Type(), s.Len(), s.Cap()) + newS = wrapPtr(reflect.MakeSlice(s.Type(), s.Len(), s.Cap())) } w.cs = append(w.cs, newS) @@ -309,6 +324,31 @@ func (w *walker) SliceElem(i int, elem reflect.Value) error { return nil } +func (w *walker) Array(a reflect.Value) error { + if w.ignoring() { + return nil + } + w.lock(a) + + newA := reflect.New(a.Type()) + + w.cs = append(w.cs, newA) + w.valPush(newA) + return nil +} + +func (w *walker) ArrayElem(i int, elem reflect.Value) error { + if w.ignoring() { + return nil + } + + // We don't write the array here because elem might still be + // arbitrarily complex. Just record the index and continue on. + w.valPush(reflect.ValueOf(i)) + + return nil +} + func (w *walker) Struct(s reflect.Value) error { if w.ignoring() { return nil @@ -326,7 +366,10 @@ func (w *walker) Struct(s reflect.Value) error { return err } - v = reflect.ValueOf(dup) + // We need to put a pointer to the value on the value stack, + // so allocate a new pointer and set it. + v = reflect.New(s.Type()) + reflect.Indirect(v).Set(reflect.ValueOf(dup)) } else { // No copier, we copy ourselves and allow reflectwalk to guide // us deeper into the structure for copying. @@ -405,6 +448,23 @@ func (w *walker) replacePointerMaybe() { } v := w.valPop() + + // If the expected type is a pointer to an interface of any depth, + // such as *interface{}, **interface{}, etc., then we need to convert + // the value "v" from *CONCRETE to *interface{} so types match for + // Set. + // + // Example if v is type *Foo where Foo is a struct, v would become + // *interface{} instead. This only happens if we have an interface expectation + // at this depth. + // + // For more info, see GH-16 + if iType, ok := w.ifaceTypes[ifaceKey(w.ps[w.depth], w.depth)]; ok && iType.Kind() == reflect.Interface { + y := reflect.New(iType) // Create *interface{} + y.Elem().Set(reflect.Indirect(v)) // Assign "Foo" to interface{} (dereferenced) + v = y // v is now typed *interface{} (where *v = Foo) + } + for i := 1; i < w.ps[w.depth]; i++ { if iType, ok := w.ifaceTypes[ifaceKey(w.ps[w.depth]-i, w.depth)]; ok { iface := reflect.New(iType).Elem() @@ -475,3 +535,14 @@ func (w *walker) lock(v reflect.Value) { locker.Lock() w.locks[w.depth] = locker } + +// wrapPtr is a helper that takes v and always make it *v. copystructure +// stores things internally as pointers until the last moment before unwrapping +func wrapPtr(v reflect.Value) reflect.Value { + if !v.IsValid() { + return v + } + vPtr := reflect.New(v.Type()) + vPtr.Elem().Set(v) + return vPtr +} diff --git a/vendor/github.com/mitchellh/reflectwalk/location.go b/vendor/github.com/mitchellh/reflectwalk/location.go index 7c59d764c..6a7f17611 100644 --- a/vendor/github.com/mitchellh/reflectwalk/location.go +++ b/vendor/github.com/mitchellh/reflectwalk/location.go @@ -11,6 +11,8 @@ const ( MapValue Slice SliceElem + Array + ArrayElem Struct StructField WalkLoc diff --git a/vendor/github.com/mitchellh/reflectwalk/location_string.go b/vendor/github.com/mitchellh/reflectwalk/location_string.go index d3cfe8545..70760cf4c 100644 --- a/vendor/github.com/mitchellh/reflectwalk/location_string.go +++ b/vendor/github.com/mitchellh/reflectwalk/location_string.go @@ -1,15 +1,15 @@ -// generated by stringer -type=Location location.go; DO NOT EDIT +// Code generated by "stringer -type=Location location.go"; DO NOT EDIT. package reflectwalk import "fmt" -const _Location_name = "NoneMapMapKeyMapValueSliceSliceElemStructStructFieldWalkLoc" +const _Location_name = "NoneMapMapKeyMapValueSliceSliceElemArrayArrayElemStructStructFieldWalkLoc" -var _Location_index = [...]uint8{0, 4, 7, 13, 21, 26, 35, 41, 52, 59} +var _Location_index = [...]uint8{0, 4, 7, 13, 21, 26, 35, 40, 49, 55, 66, 73} func (i Location) String() string { - if i+1 >= Location(len(_Location_index)) { + if i >= Location(len(_Location_index)-1) { return fmt.Sprintf("Location(%d)", i) } return _Location_name[_Location_index[i]:_Location_index[i+1]] diff --git a/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go b/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go index ec0a62337..d7ab7b6d7 100644 --- a/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go +++ b/vendor/github.com/mitchellh/reflectwalk/reflectwalk.go @@ -39,6 +39,13 @@ type SliceWalker interface { SliceElem(int, reflect.Value) error } +// ArrayWalker implementations are able to handle array elements found +// within complex structures. +type ArrayWalker interface { + Array(reflect.Value) error + ArrayElem(int, reflect.Value) error +} + // StructWalker is an interface that has methods that are called for // structs when a Walk is done. type StructWalker interface { @@ -65,6 +72,7 @@ type PointerWalker interface { // SkipEntry can be returned from walk functions to skip walking // the value of this field. This is only valid in the following functions: // +// - Struct: skips all fields from being walked // - StructField: skips walking the struct value // var SkipEntry = errors.New("skip this entry") @@ -179,6 +187,9 @@ func walk(v reflect.Value, w interface{}) (err error) { case reflect.Struct: err = walkStruct(v, w) return + case reflect.Array: + err = walkArray(v, w) + return default: panic("unsupported type: " + k.String()) } @@ -286,48 +297,99 @@ func walkSlice(v reflect.Value, w interface{}) (err error) { return nil } +func walkArray(v reflect.Value, w interface{}) (err error) { + ew, ok := w.(EnterExitWalker) + if ok { + ew.Enter(Array) + } + + if aw, ok := w.(ArrayWalker); ok { + if err := aw.Array(v); err != nil { + return err + } + } + + for i := 0; i < v.Len(); i++ { + elem := v.Index(i) + + if aw, ok := w.(ArrayWalker); ok { + if err := aw.ArrayElem(i, elem); err != nil { + return err + } + } + + ew, ok := w.(EnterExitWalker) + if ok { + ew.Enter(ArrayElem) + } + + if err := walk(elem, w); err != nil { + return err + } + + if ok { + ew.Exit(ArrayElem) + } + } + + ew, ok = w.(EnterExitWalker) + if ok { + ew.Exit(Array) + } + + return nil +} + func walkStruct(v reflect.Value, w interface{}) (err error) { ew, ewok := w.(EnterExitWalker) if ewok { ew.Enter(Struct) } + skip := false if sw, ok := w.(StructWalker); ok { - if err = sw.Struct(v); err != nil { + err = sw.Struct(v) + if err == SkipEntry { + skip = true + err = nil + } + if err != nil { return } } - vt := v.Type() - for i := 0; i < vt.NumField(); i++ { - sf := vt.Field(i) - f := v.FieldByIndex([]int{i}) + if !skip { + vt := v.Type() + for i := 0; i < vt.NumField(); i++ { + sf := vt.Field(i) + f := v.FieldByIndex([]int{i}) - if sw, ok := w.(StructWalker); ok { - err = sw.StructField(sf, f) + if sw, ok := w.(StructWalker); ok { + err = sw.StructField(sf, f) - // SkipEntry just pretends this field doesn't even exist - if err == SkipEntry { - continue + // SkipEntry just pretends this field doesn't even exist + if err == SkipEntry { + continue + } + + if err != nil { + return + } } + ew, ok := w.(EnterExitWalker) + if ok { + ew.Enter(StructField) + } + + err = walk(f, w) if err != nil { return } - } - ew, ok := w.(EnterExitWalker) - if ok { - ew.Enter(StructField) - } - - err = walk(f, w) - if err != nil { - return - } - - if ok { - ew.Exit(StructField) + if ok { + ew.Exit(StructField) + } } } diff --git a/vendor/vendor.json b/vendor/vendor.json index 9a5623c93..24e2891be 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1964,10 +1964,10 @@ "revisionTime": "2015-09-17T21:48:07Z" }, { - "checksumSHA1": "guxbLo8KHHBeM0rzou4OTzzpDNs=", + "checksumSHA1": "+p4JY4wmFQAppCdlrJ8Kxybmht8=", "path": "github.com/mitchellh/copystructure", - "revision": "5af94aef99f597e6a9e1f6ac6be6ce0f3c96b49d", - "revisionTime": "2016-10-13T19:53:42Z" + "revision": "d23ffcb85de31694d6ccaa23ccb4a03e55c1303f", + "revisionTime": "2017-05-25T01:39:02Z" }, { "checksumSHA1": "V/quM7+em2ByJbWBLOsEwnY3j/Q=", @@ -2021,10 +2021,10 @@ "revision": "6e6954073784f7ee67b28f2d22749d6479151ed7" }, { - "checksumSHA1": "vBpuqNfSTZcAR/0tP8tNYacySGs=", + "checksumSHA1": "AMU63CNOg4XmIhVR/S/Xttt1/f0=", "path": "github.com/mitchellh/reflectwalk", - "revision": "92573fe8d000a145bfebc03a16bc22b34945867f", - "revisionTime": "2016-10-03T17:45:16Z" + "revision": "63d60e9d0dbc60cf9164e6510889b0db6683d98c", + "revisionTime": "2017-07-26T20:21:17Z" }, { "checksumSHA1": "gcLub3oB+u4QrOJZcYmk/y2AP4k=",