Merge pull request #17588 from creack/creack/fix-deepcopy-type

Fix issue with deepcopy returning wrong type causing panic
This commit is contained in:
James Bardin 2018-04-05 09:24:08 -04:00 committed by GitHub
commit 61eae050ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 177 additions and 42 deletions

View File

@ -395,7 +395,7 @@ func (m *schemaMap) DeepCopy() schemaMap {
if err != nil { if err != nil {
panic(err) panic(err)
} }
return copy.(schemaMap) return *copy.(*schemaMap)
} }
// Diff returns the diff for a resource given the schema map, // Diff returns the diff for a resource given the schema map,

View File

@ -156,9 +156,13 @@ func (w *walker) Exit(l reflectwalk.Location) error {
} }
switch l { switch l {
case reflectwalk.Array:
fallthrough
case reflectwalk.Map: case reflectwalk.Map:
fallthrough fallthrough
case reflectwalk.Slice: case reflectwalk.Slice:
w.replacePointerMaybe()
// Pop map off our container // Pop map off our container
w.cs = w.cs[:len(w.cs)-1] w.cs = w.cs[:len(w.cs)-1]
case reflectwalk.MapValue: 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 // or in this case never adds it. We need to create a properly typed
// zero value so that this key can be set. // zero value so that this key can be set.
if !mv.IsValid() { 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: case reflectwalk.SliceElem:
// Pop off the value and the index and set it on the slice // Pop off the value and the index and set it on the slice
v := w.valPop() v := w.valPop()
i := w.valPop().Interface().(int) i := w.valPop().Interface().(int)
if v.IsValid() { if v.IsValid() {
s := w.cs[len(w.cs)-1] s := w.cs[len(w.cs)-1]
se := s.Index(i) se := s.Elem().Index(i)
if se.CanSet() { if se.CanSet() {
se.Set(v) 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 // Create the map. If the map itself is nil, then just make a nil map
var newMap reflect.Value var newMap reflect.Value
if m.IsNil() { if m.IsNil() {
newMap = reflect.Indirect(reflect.New(m.Type())) newMap = reflect.New(m.Type())
} else { } else {
newMap = reflect.MakeMap(m.Type()) newMap = wrapPtr(reflect.MakeMap(m.Type()))
} }
w.cs = append(w.cs, newMap) w.cs = append(w.cs, newMap)
@ -287,9 +302,9 @@ func (w *walker) Slice(s reflect.Value) error {
var newS reflect.Value var newS reflect.Value
if s.IsNil() { if s.IsNil() {
newS = reflect.Indirect(reflect.New(s.Type())) newS = reflect.New(s.Type())
} else { } 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) w.cs = append(w.cs, newS)
@ -309,6 +324,31 @@ func (w *walker) SliceElem(i int, elem reflect.Value) error {
return nil 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 { func (w *walker) Struct(s reflect.Value) error {
if w.ignoring() { if w.ignoring() {
return nil return nil
@ -326,7 +366,10 @@ func (w *walker) Struct(s reflect.Value) error {
return err 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 { } else {
// No copier, we copy ourselves and allow reflectwalk to guide // No copier, we copy ourselves and allow reflectwalk to guide
// us deeper into the structure for copying. // us deeper into the structure for copying.
@ -405,6 +448,23 @@ func (w *walker) replacePointerMaybe() {
} }
v := w.valPop() 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++ { for i := 1; i < w.ps[w.depth]; i++ {
if iType, ok := w.ifaceTypes[ifaceKey(w.ps[w.depth]-i, w.depth)]; ok { if iType, ok := w.ifaceTypes[ifaceKey(w.ps[w.depth]-i, w.depth)]; ok {
iface := reflect.New(iType).Elem() iface := reflect.New(iType).Elem()
@ -475,3 +535,14 @@ func (w *walker) lock(v reflect.Value) {
locker.Lock() locker.Lock()
w.locks[w.depth] = locker 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
}

View File

@ -11,6 +11,8 @@ const (
MapValue MapValue
Slice Slice
SliceElem SliceElem
Array
ArrayElem
Struct Struct
StructField StructField
WalkLoc WalkLoc

View File

@ -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 package reflectwalk
import "fmt" 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 { 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 fmt.Sprintf("Location(%d)", i)
} }
return _Location_name[_Location_index[i]:_Location_index[i+1]] return _Location_name[_Location_index[i]:_Location_index[i+1]]

View File

@ -39,6 +39,13 @@ type SliceWalker interface {
SliceElem(int, reflect.Value) error 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 // StructWalker is an interface that has methods that are called for
// structs when a Walk is done. // structs when a Walk is done.
type StructWalker interface { type StructWalker interface {
@ -65,6 +72,7 @@ type PointerWalker interface {
// SkipEntry can be returned from walk functions to skip walking // SkipEntry can be returned from walk functions to skip walking
// the value of this field. This is only valid in the following functions: // 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 // - StructField: skips walking the struct value
// //
var SkipEntry = errors.New("skip this entry") var SkipEntry = errors.New("skip this entry")
@ -179,6 +187,9 @@ func walk(v reflect.Value, w interface{}) (err error) {
case reflect.Struct: case reflect.Struct:
err = walkStruct(v, w) err = walkStruct(v, w)
return return
case reflect.Array:
err = walkArray(v, w)
return
default: default:
panic("unsupported type: " + k.String()) panic("unsupported type: " + k.String())
} }
@ -286,48 +297,99 @@ func walkSlice(v reflect.Value, w interface{}) (err error) {
return nil 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) { func walkStruct(v reflect.Value, w interface{}) (err error) {
ew, ewok := w.(EnterExitWalker) ew, ewok := w.(EnterExitWalker)
if ewok { if ewok {
ew.Enter(Struct) ew.Enter(Struct)
} }
skip := false
if sw, ok := w.(StructWalker); ok { 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 return
} }
} }
vt := v.Type() if !skip {
for i := 0; i < vt.NumField(); i++ { vt := v.Type()
sf := vt.Field(i) for i := 0; i < vt.NumField(); i++ {
f := v.FieldByIndex([]int{i}) sf := vt.Field(i)
f := v.FieldByIndex([]int{i})
if sw, ok := w.(StructWalker); ok { if sw, ok := w.(StructWalker); ok {
err = sw.StructField(sf, f) err = sw.StructField(sf, f)
// SkipEntry just pretends this field doesn't even exist // SkipEntry just pretends this field doesn't even exist
if err == SkipEntry { if err == SkipEntry {
continue continue
}
if err != nil {
return
}
} }
ew, ok := w.(EnterExitWalker)
if ok {
ew.Enter(StructField)
}
err = walk(f, w)
if err != nil { if err != nil {
return return
} }
}
ew, ok := w.(EnterExitWalker) if ok {
if ok { ew.Exit(StructField)
ew.Enter(StructField) }
}
err = walk(f, w)
if err != nil {
return
}
if ok {
ew.Exit(StructField)
} }
} }

12
vendor/vendor.json vendored
View File

@ -2030,10 +2030,10 @@
"revisionTime": "2015-09-17T21:48:07Z" "revisionTime": "2015-09-17T21:48:07Z"
}, },
{ {
"checksumSHA1": "guxbLo8KHHBeM0rzou4OTzzpDNs=", "checksumSHA1": "+p4JY4wmFQAppCdlrJ8Kxybmht8=",
"path": "github.com/mitchellh/copystructure", "path": "github.com/mitchellh/copystructure",
"revision": "5af94aef99f597e6a9e1f6ac6be6ce0f3c96b49d", "revision": "d23ffcb85de31694d6ccaa23ccb4a03e55c1303f",
"revisionTime": "2016-10-13T19:53:42Z" "revisionTime": "2017-05-25T01:39:02Z"
}, },
{ {
"checksumSHA1": "V/quM7+em2ByJbWBLOsEwnY3j/Q=", "checksumSHA1": "V/quM7+em2ByJbWBLOsEwnY3j/Q=",
@ -2087,10 +2087,10 @@
"revision": "6e6954073784f7ee67b28f2d22749d6479151ed7" "revision": "6e6954073784f7ee67b28f2d22749d6479151ed7"
}, },
{ {
"checksumSHA1": "vBpuqNfSTZcAR/0tP8tNYacySGs=", "checksumSHA1": "AMU63CNOg4XmIhVR/S/Xttt1/f0=",
"path": "github.com/mitchellh/reflectwalk", "path": "github.com/mitchellh/reflectwalk",
"revision": "92573fe8d000a145bfebc03a16bc22b34945867f", "revision": "63d60e9d0dbc60cf9164e6510889b0db6683d98c",
"revisionTime": "2016-10-03T17:45:16Z" "revisionTime": "2017-07-26T20:21:17Z"
}, },
{ {
"checksumSHA1": "gcLub3oB+u4QrOJZcYmk/y2AP4k=", "checksumSHA1": "gcLub3oB+u4QrOJZcYmk/y2AP4k=",