Skip to content

Commit

Permalink
convert: Fix panic when unifying tuple element types
Browse files Browse the repository at this point in the history
The `unifyObjectTypesToMap` function was inadvertently called from the
tuple-list unification function, which would panic since the value has
no attributes.

The given test example is possible speculatively unify, however we don't
have a codepath to recursively unify these yet, so we will just fail to
unify for now. This will fix the crashes in downstream consumers,
allowing the possibility of working around the shortcoming by using more
specific or different combinations of types.
  • Loading branch information
jbardin committed Jun 23, 2022
1 parent 97a685d commit 24404ad
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
5 changes: 2 additions & 3 deletions cty/convert/unify.go
Expand Up @@ -447,7 +447,6 @@ func unifyTupleTypes(types []cty.Type, unsafe bool, hasDynamic bool) (cty.Type,
conversions[i] = GetConversion(ty, retTy)
}
if conversions[i] == nil {
// Shouldn't be reachable, since we were able to unify
return unifyTupleTypesToList(types, unsafe)
}
}
Expand Down Expand Up @@ -483,8 +482,8 @@ func unifyTupleTypesToList(types []cty.Type, unsafe bool) (cty.Type, []Conversio
conversions[i] = GetConversion(ty, retTy)
}
if conversions[i] == nil {
// Shouldn't be reachable, since we were able to unify
return unifyObjectTypesToMap(types, unsafe)
// no conversion was found
return cty.NilType, nil
}
}
return retTy, conversions
Expand Down
17 changes: 17 additions & 0 deletions cty/convert/unify_test.go
Expand Up @@ -163,6 +163,23 @@ func TestUnify(t *testing.T) {
cty.List(cty.Map(cty.String)),
[]bool{true, true},
},
{
// The second tuple value could be anything, so we can't unify
// these as a list.
// FIXME: While a unification is possible, we get a NilType for
// now until we can handle more complex recursive unification.
[]cty.Type{
cty.Tuple([]cty.Type{
cty.Object(map[string]cty.Type{
"a": cty.String,
}),
cty.DynamicPseudoType,
}),
cty.List(cty.DynamicPseudoType),
},
cty.NilType,
nil,
},
{
// unifies to the same result as above, since the only difference
// is the addition of a list
Expand Down

0 comments on commit 24404ad

Please sign in to comment.