diff --git a/lang/types/value.go b/lang/types/value.go index 6331e9b5..4cdf888b 100644 --- a/lang/types/value.go +++ b/lang/types/value.go @@ -18,6 +18,7 @@ package types import ( + "errors" "fmt" "reflect" "sort" @@ -27,6 +28,16 @@ import ( "github.com/purpleidea/mgmt/util/errwrap" ) +var ( + // ErrNilValue is returned when ValueOf() attempts to represent a nil + // pointer as an mcl value. This is not supported in mcl. + ErrNilValue = errors.New("cannot represent a nil Golang value in mcl") + + // ErrInvalidValue is returned when ValueOf() is called on an invalid or + // zero reflect.Value. + ErrInvalidValue = errors.New("cannot represent invalid reflect.Value") +) + // Value represents an interface to get values out of each type. It is similar // to the reflection interfaces used in the golang standard library. type Value interface { @@ -65,12 +76,22 @@ func ValueOfGolang(i interface{}) (Value, error) { // a resource field. This is done with our "elvis" operator. When using this // function, if you pass in something with a nil value, then expect a panic or // an error if you're lucky. -// FIXME: we should make sure we error instead of ever panic-ing on a nil value func ValueOf(v reflect.Value) (Value, error) { + // Gracefully handle invalid values instead of panic(). Invalid + // reflect.Value values can come from nil values, or the zero value. + if !v.IsValid() { + return nil, ErrInvalidValue + } + value := v typ := value.Type() kind := typ.Kind() for kind == reflect.Ptr { + // Prevent panic() if value is a nil pointer and return an error. + if value.IsNil() { + return nil, ErrNilValue + } + typ = typ.Elem() // un-nest one pointer kind = typ.Kind() diff --git a/lang/types/value_test.go b/lang/types/value_test.go index 2b3487b2..1fc57482 100644 --- a/lang/types/value_test.go +++ b/lang/types/value_test.go @@ -20,6 +20,7 @@ package types import ( + "errors" "fmt" "math" "reflect" @@ -694,6 +695,23 @@ func TestValueOf2(t *testing.T) { } } +func TestValueOf3(t *testing.T) { + st := struct { + Ptr *string `lang:"ptr"` + Ptr2 **string `lang:"ptr2"` + }{nil, nil} + + // cannot represent nil pointers, expect an err + val, err := ValueOf(reflect.ValueOf(st)) + if err == nil { + t.Errorf("function ValueOf(%+v) returned a Value when error was expected: %s", st, val) + return + } + if !errors.Is(err, ErrNilValue) { + t.Errorf("function ValueOf(%+v) returned err %s but types.ErrNilValue was expected", st, err) + } +} + func TestValueInto0(t *testing.T) { // converts a Go variable to a types.Value, or panics if any error mustValue := func(v interface{}) Value {