From bc8bc37282a240d7cd5008756676a916261c2e35 Mon Sep 17 00:00:00 2001 From: Muhammed Uluyol Date: Tue, 11 Aug 2015 14:59:46 -0700 Subject: [PATCH] Properly handle nil interfaces in DeepCopy. Running reflect.ValueOf(X) where X is a nil interface will return a zero Value. We cannot get the type (because no concrete type is known) and cannot check if the Value is nil later on due to the way reflect.Value works. So we should handle this case by immediately returning nil. We cannot type-assert a nil interface to another interface type (as no concrete type is assigned), so we must add another check to see if the returned interface is nil. --- pkg/api/deep_copy_generated.go | 8 ++++++++ pkg/api/v1/deep_copy_generated.go | 2 ++ pkg/conversion/cloner.go | 7 +++++++ pkg/runtime/deep_copy_generator.go | 6 ++++++ 4 files changed, 23 insertions(+) diff --git a/pkg/api/deep_copy_generated.go b/pkg/api/deep_copy_generated.go index 15ecb80bb5f..fceeec0353a 100644 --- a/pkg/api/deep_copy_generated.go +++ b/pkg/api/deep_copy_generated.go @@ -736,6 +736,8 @@ func deepCopy_api_List(in List, out *List, c *conversion.Cloner) error { for i := range in.Items { if newVal, err := c.DeepCopy(in.Items[i]); err != nil { return err + } else if newVal == nil { + out.Items[i] = nil } else { out.Items[i] = newVal.(runtime.Object) } @@ -758,11 +760,15 @@ func deepCopy_api_ListOptions(in ListOptions, out *ListOptions, c *conversion.Cl } if newVal, err := c.DeepCopy(in.LabelSelector); err != nil { return err + } else if newVal == nil { + out.LabelSelector = nil } else { out.LabelSelector = newVal.(labels.Selector) } if newVal, err := c.DeepCopy(in.FieldSelector); err != nil { return err + } else if newVal == nil { + out.FieldSelector = nil } else { out.FieldSelector = newVal.(fields.Selector) } @@ -2124,6 +2130,8 @@ func deepCopy_resource_Quantity(in resource.Quantity, out *resource.Quantity, c if in.Amount != nil { if newVal, err := c.DeepCopy(in.Amount); err != nil { return err + } else if newVal == nil { + out.Amount = nil } else { out.Amount = newVal.(*inf.Dec) } diff --git a/pkg/api/v1/deep_copy_generated.go b/pkg/api/v1/deep_copy_generated.go index 192b44d48cc..f14cddd763f 100644 --- a/pkg/api/v1/deep_copy_generated.go +++ b/pkg/api/v1/deep_copy_generated.go @@ -31,6 +31,8 @@ func deepCopy_resource_Quantity(in resource.Quantity, out *resource.Quantity, c if in.Amount != nil { if newVal, err := c.DeepCopy(in.Amount); err != nil { return err + } else if newVal == nil { + out.Amount = nil } else { out.Amount = newVal.(*inf.Dec) } diff --git a/pkg/conversion/cloner.go b/pkg/conversion/cloner.go index 43ed27007ed..a8c57471327 100644 --- a/pkg/conversion/cloner.go +++ b/pkg/conversion/cloner.go @@ -117,6 +117,13 @@ func (c *Cloner) RegisterGeneratedDeepCopyFunc(deepCopyFunc interface{}) error { // DeepCopy will perform a deep copy of a given object. func (c *Cloner) DeepCopy(in interface{}) (interface{}, error) { + // Can be invalid if we run DeepCopy(X) where X is a nil interface type. + // For example, we get an invalid value when someone tries to deep-copy + // a nil labels.Selector. + // This does not occur if X is nil and is a pointer to a concrete type. + if in == nil { + return nil, nil + } inValue := reflect.ValueOf(in) outValue, err := c.deepCopy(inValue) if err != nil { diff --git a/pkg/runtime/deep_copy_generator.go b/pkg/runtime/deep_copy_generator.go index 0b05d1470f3..45fe3a01926 100644 --- a/pkg/runtime/deep_copy_generator.go +++ b/pkg/runtime/deep_copy_generator.go @@ -420,6 +420,8 @@ func (g *deepCopyGenerator) writeDeepCopyForPtr(b *buffer, inField reflect.Struc ifStmt := fmt.Sprintf(ifFormat, inField.Name) b.addLine(ifStmt, indent+1) b.addLine("return err\n", indent+2) + b.addLine("} else if newVal == nil {\n", indent+1) + b.addLine(fmt.Sprintf("out.%s = nil\n", inField.Name), indent+2) b.addLine("} else {\n", indent+1) assignFormat := "out.%s = newVal.(%s)\n" assignStmt := fmt.Sprintf(assignFormat, inField.Name, g.typeName(inField.Type)) @@ -467,6 +469,8 @@ func (g *deepCopyGenerator) writeDeepCopyForSlice(b *buffer, inField reflect.Str ifStmt := fmt.Sprintf(ifFormat, inField.Name) b.addLine(ifStmt, indent+2) b.addLine("return err\n", indent+3) + b.addLine("} else if newVal == nil {\n", indent+2) + b.addLine(fmt.Sprintf("out.%s[i] = nil\n", inField.Name), indent+3) b.addLine("} else {\n", indent+2) assignFormat := "out.%s[i] = newVal.(%s)\n" assignStmt := fmt.Sprintf(assignFormat, inField.Name, g.typeName(inField.Type.Elem())) @@ -508,6 +512,8 @@ func (g *deepCopyGenerator) writeDeepCopyForStruct(b *buffer, inType reflect.Typ ifStmt := fmt.Sprintf(ifFormat, inField.Name) b.addLine(ifStmt, indent) b.addLine("return err\n", indent+1) + b.addLine("} else if newVal == nil {\n", indent) + b.addLine(fmt.Sprintf("out.%s = nil\n", inField.Name), indent+1) b.addLine("} else {\n", indent) copyFormat := "out.%s = newVal.(%s)\n" copyStmt := fmt.Sprintf(copyFormat, inField.Name, g.typeName(inField.Type))