Skip to content

Commit

Permalink
convert: prevent invalid types in dynamicReplace
Browse files Browse the repository at this point in the history
When converting an unknown map into an object with optional attributes,
that object may have attributes which don't match the map element type.
This conversion will be valid, but when building Null or Unknown values
of that type we need to ensure all attributes have a valid type.

The fallthrough path for dynamicReplace when non-primitive types didn't
match contained a cty.NilVal element type, which would result in an
overall invalid type. We can early returns to ensure there are no
uninitialized values, and let the fallback use the original out type.
Since at this point the types appear to not be convertible, we can
assume they are from attributes which were skipped during conversion due
to being optional, otherwise the conversion would not have been valid.
  • Loading branch information
jbardin authored and apparentlymart committed Jan 23, 2024
1 parent d279406 commit 864b88d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 15 deletions.
29 changes: 14 additions & 15 deletions cty/convert/conversion_dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ func dynamicPassthrough(in cty.Value, path cty.Path) (cty.Value, error) {
// perspective, and will panic if it finds that they are not. For example if
// in is an object and out is a map, this function will still attempt to iterate
// through both as if they were the same.
// While the outermost in and out types may be compatible from a Convert
// perspective, inner types may not match when converting between maps and
// objects with optional attributes when the optional attributes don't match
// the map element type. Therefor in the case of a non-primitive type mismatch,
// we have to assume conversion was possible and pass the out type through.
func dynamicReplace(in, out cty.Type) cty.Type {
if in == cty.DynamicPseudoType || in == cty.NilType {
// Short circuit this case, there's no point worrying about this if in
Expand All @@ -56,11 +61,9 @@ func dynamicReplace(in, out cty.Type) cty.Type {
// return it unchanged.
return out
case out.IsMapType():
var elemType cty.Type

// Maps are compatible with other maps or objects.
if in.IsMapType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
return cty.Map(dynamicReplace(in.ElementType(), out.ElementType()))
}

if in.IsObjectType() {
Expand All @@ -69,10 +72,10 @@ func dynamicReplace(in, out cty.Type) cty.Type {
types = append(types, t)
}
unifiedType, _ := unify(types, true)
elemType = dynamicReplace(unifiedType, out.ElementType())
return cty.Map(dynamicReplace(unifiedType, out.ElementType()))
}

return cty.Map(elemType)
return out
case out.IsObjectType():
// Objects are compatible with other objects and maps.
outTypes := map[string]cty.Type{}
Expand All @@ -97,33 +100,29 @@ func dynamicReplace(in, out cty.Type) cty.Type {

return cty.Object(outTypes)
case out.IsSetType():
var elemType cty.Type

// Sets are compatible with other sets, lists, tuples.
if in.IsSetType() || in.IsListType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
return cty.Set(dynamicReplace(in.ElementType(), out.ElementType()))
}

if in.IsTupleType() {
unifiedType, _ := unify(in.TupleElementTypes(), true)
elemType = dynamicReplace(unifiedType, out.ElementType())
return cty.Set(dynamicReplace(unifiedType, out.ElementType()))
}

return cty.Set(elemType)
return out
case out.IsListType():
var elemType cty.Type

// Lists are compatible with other lists, sets, and tuples.
if in.IsSetType() || in.IsListType() {
elemType = dynamicReplace(in.ElementType(), out.ElementType())
return cty.List(dynamicReplace(in.ElementType(), out.ElementType()))
}

if in.IsTupleType() {
unifiedType, _ := unify(in.TupleElementTypes(), true)
elemType = dynamicReplace(unifiedType, out.ElementType())
return cty.List(dynamicReplace(unifiedType, out.ElementType()))
}

return cty.List(elemType)
return out
case out.IsTupleType():
// Tuples are only compatible with other tuples
var types []cty.Type
Expand Down
24 changes: 24 additions & 0 deletions cty/convert/public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,30 @@ func TestConvert(t *testing.T) {
Type: cty.String,
Want: cty.UnknownVal(cty.String).RefineNotNull(),
},

// Make sure we get valid unknown attribute types when converting from
// a map to an object with optional attributes.
{
Value: cty.ObjectVal(map[string]cty.Value{
"TTTattr": cty.UnknownVal(cty.Map(cty.String)),
}),
Type: cty.Object(map[string]cty.Type{
"TTTattr": cty.ObjectWithOptionalAttrs(map[string]cty.Type{
"string": cty.String,
"set": cty.Set(cty.String),
"list": cty.List(cty.String),
"map": cty.Map(cty.String),
}, []string{"set", "list", "map"}),
}),
Want: cty.ObjectVal(map[string]cty.Value{
"TTTattr": cty.UnknownVal(cty.Object(map[string]cty.Type{
"list": cty.List(cty.String),
"map": cty.Map(cty.String),
"set": cty.Set(cty.String),
"string": cty.String,
})),
}),
},
}

for _, test := range tests {
Expand Down

0 comments on commit 864b88d

Please sign in to comment.