diff --git a/.changelog/99.txt b/.changelog/99.txt new file mode 100644 index 00000000..deb88788 --- /dev/null +++ b/.changelog/99.txt @@ -0,0 +1,3 @@ +```release-note:bug +tftypes: Prevent potential panic unmarshaling null DynamicPseudoType in msgpack +``` diff --git a/tftypes/object.go b/tftypes/object.go index c989e08b..6df9caf8 100644 --- a/tftypes/object.go +++ b/tftypes/object.go @@ -158,15 +158,6 @@ func (o Object) supportedGoTypes() []string { return []string{"map[string]tftypes.Value"} } -func valueCanBeObject(val interface{}) bool { - switch val.(type) { - case map[string]Value: - return true - default: - return false - } -} - func valueFromObject(types map[string]Type, optionalAttrs map[string]struct{}, in interface{}) (Value, error) { switch value := in.(type) { case map[string]Value: diff --git a/tftypes/primitive.go b/tftypes/primitive.go index c4cd73c7..b0a3c8fc 100644 --- a/tftypes/primitive.go +++ b/tftypes/primitive.go @@ -102,30 +102,11 @@ func (p primitive) supportedGoTypes() []string { case Bool.name: return []string{"bool", "*bool"} case DynamicPseudoType.name: - possibleTypes := []Type{ - String, Bool, Number, - Tuple{}, Object{}, - } - results := []string{} - for _, t := range possibleTypes { - results = append(results, t.supportedGoTypes()...) - } - return results + return []string{"nil", "UnknownValue"} } panic(fmt.Sprintf("unknown primitive type %q", p.name)) } -func valueCanBeString(val interface{}) bool { - switch val.(type) { - case string: - return true - case *string: - return true - default: - return false - } -} - func valueFromString(in interface{}) (Value, error) { switch value := in.(type) { case *string: @@ -149,17 +130,6 @@ func valueFromString(in interface{}) (Value, error) { } } -func valueCanBeBool(val interface{}) bool { - switch val.(type) { - case bool: - return true - case *bool: - return true - default: - return false - } -} - func valueFromBool(in interface{}) (Value, error) { switch value := in.(type) { case *bool: @@ -183,25 +153,6 @@ func valueFromBool(in interface{}) (Value, error) { } } -func valueCanBeNumber(val interface{}) bool { - switch val.(type) { - case uint, uint8, uint16, uint32, uint64: - return true - case *uint, *uint8, *uint16, *uint32, *uint64: - return true - case int, int8, int16, int32, int64: - return true - case *int, *int8, *int16, *int32, *int64: - return true - case float64, *float64: - return true - case *big.Float: - return true - default: - return false - } -} - func valueFromNumber(in interface{}) (Value, error) { switch value := in.(type) { case *big.Float: @@ -389,45 +340,3 @@ func valueFromNumber(in interface{}) (Value, error) { return Value{}, fmt.Errorf("tftypes.NewValue can't use %T as a tftypes.Number; expected types are: %s", in, formattedSupportedGoTypes(Number)) } } - -func valueFromDynamicPseudoType(val interface{}) (Value, error) { - switch { - case valueCanBeString(val): - v, err := valueFromString(val) - if err != nil { - return Value{}, err - } - v.typ = DynamicPseudoType - return v, nil - case valueCanBeNumber(val): - v, err := valueFromNumber(val) - if err != nil { - return Value{}, err - } - v.typ = DynamicPseudoType - return v, nil - case valueCanBeBool(val): - v, err := valueFromBool(val) - if err != nil { - return Value{}, err - } - v.typ = DynamicPseudoType - return v, nil - case valueCanBeObject(val): - v, err := valueFromObject(nil, nil, val) - if err != nil { - return Value{}, err - } - v.typ = DynamicPseudoType - return v, nil - case valueCanBeTuple(val): - v, err := valueFromTuple(nil, val) - if err != nil { - return Value{}, err - } - v.typ = DynamicPseudoType - return v, nil - default: - return Value{}, fmt.Errorf("tftypes.NewValue can't use %T as a tftypes.DynamicPseudoType; expected types are: %s", val, formattedSupportedGoTypes(DynamicPseudoType)) - } -} diff --git a/tftypes/tuple.go b/tftypes/tuple.go index dd2ef2cd..601455d2 100644 --- a/tftypes/tuple.go +++ b/tftypes/tuple.go @@ -96,15 +96,6 @@ func (tu Tuple) supportedGoTypes() []string { return []string{"[]tftypes.Value"} } -func valueCanBeTuple(val interface{}) bool { - switch val.(type) { - case []Value: - return true - default: - return false - } -} - func valueFromTuple(types []Type, in interface{}) (Value, error) { switch value := in.(type) { case []Value: diff --git a/tftypes/value.go b/tftypes/value.go index b9308853..0c534f10 100644 --- a/tftypes/value.go +++ b/tftypes/value.go @@ -289,15 +289,17 @@ func ValidateValue(t Type, val interface{}) error { } func newValue(t Type, val interface{}) (Value, error) { - if t.Is(DynamicPseudoType) && val != UnknownValue { - return Value{}, errors.New("cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain unknown values") - } if val == nil || val == UnknownValue { return Value{ typ: t, value: val, }, nil } + + if t.Is(DynamicPseudoType) { + return Value{}, errors.New("cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain null or unknown values") + } + if creator, ok := val.(ValueCreator); ok { var err error val, err = creator.ToTerraform5Value() @@ -307,12 +309,6 @@ func newValue(t Type, val interface{}) (Value, error) { } switch { - case t.Is(DynamicPseudoType): - v, err := valueFromDynamicPseudoType(val) - if err != nil { - return Value{}, err - } - return v, nil case t.Is(String): v, err := valueFromString(val) if err != nil { diff --git a/tftypes/value_dpt_test.go b/tftypes/value_dpt_test.go index 12011990..c7d78acc 100644 --- a/tftypes/value_dpt_test.go +++ b/tftypes/value_dpt_test.go @@ -14,7 +14,20 @@ func Test_newValue_dpt(t *testing.T) { expected Value } tests := map[string]testCase{ - "normal": { + "known": { + typ: DynamicPseudoType, + val: "hello", + err: regexp.MustCompile(`cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain null or unknown values`), + }, + "null": { + typ: DynamicPseudoType, + val: nil, + expected: Value{ + typ: DynamicPseudoType, + value: nil, + }, + }, + "unknown": { typ: DynamicPseudoType, val: UnknownValue, expected: Value{ @@ -23,11 +36,6 @@ func Test_newValue_dpt(t *testing.T) { }, err: nil, }, - "dynamic-known": { - typ: DynamicPseudoType, - val: "hello", - err: regexp.MustCompile(`cannot have DynamicPseudoType with known value, DynamicPseudoType can only contain unknown values`), - }, } for name, test := range tests { name, test := name, test diff --git a/tftypes/value_msgpack.go b/tftypes/value_msgpack.go index 7153460b..deae3997 100644 --- a/tftypes/value_msgpack.go +++ b/tftypes/value_msgpack.go @@ -341,7 +341,7 @@ func msgpackUnmarshalDynamic(dec *msgpack.Decoder, path *AttributePath) (Value, switch { case length == -1: - return NewValue(DynamicPseudoType, nil), nil + return newValue(DynamicPseudoType, nil) case length != 2: return Value{}, path.NewErrorf("expected %d elements in DynamicPseudoType array, got %d", 2, length) } diff --git a/tftypes/value_msgpack_test.go b/tftypes/value_msgpack_test.go index b71338d3..c687b6eb 100644 --- a/tftypes/value_msgpack_test.go +++ b/tftypes/value_msgpack_test.go @@ -442,6 +442,11 @@ func TestValueFromMsgPack(t *testing.T) { value: NewValue(String, nil), typ: DynamicPseudoType, }, + "dynamic-null": { + hex: "c0", + value: NewValue(DynamicPseudoType, nil), + typ: DynamicPseudoType, + }, "dynamic-unknown": { hex: "d40000", value: NewValue(DynamicPseudoType, UnknownValue), diff --git a/tftypes/value_test.go b/tftypes/value_test.go index 215b540e..ef5110c0 100644 --- a/tftypes/value_test.go +++ b/tftypes/value_test.go @@ -470,6 +470,11 @@ func TestValueIsKnown(t *testing.T) { known: false, fullyKnown: false, }, + "dynamic-unknown": { + value: NewValue(DynamicPseudoType, UnknownValue), + known: false, + fullyKnown: false, + }, "list-string-known": { value: NewValue(List{ElementType: String}, []Value{NewValue(String, "hello")}), known: true, @@ -655,6 +660,11 @@ func TestValueIsKnown(t *testing.T) { known: true, fullyKnown: false, }, + "dynamic-null": { + value: NewValue(DynamicPseudoType, nil), + known: true, + fullyKnown: true, + }, "object-null": { value: NewValue(Object{AttributeTypes: map[string]Type{ "foo": String, @@ -1606,6 +1616,10 @@ func TestValueString(t *testing.T) { in: NewValue(Bool, nil), expected: "tftypes.Bool", }, + "dynamic-null": { + in: NewValue(DynamicPseudoType, nil), + expected: "tftypes.DynamicPseudoType", + }, "map": { in: NewValue(Map{AttributeType: String}, map[string]Value{ "hello": NewValue(String, "world"),