Skip to content

Commit

Permalink
hcldec: Handle or forbid cty.DynamicPseudoType attributes in nested b…
Browse files Browse the repository at this point in the history
…locks

Our BlockList, BlockSet, and BlockMap specs all produce cty collection
values, which require all elements to have a homogeneous type. If the
nested spec contained an attribute of type cty.DynamicPseudoType, that
would create the risk of each element having a different type, which would
previously have caused decoding to panic.

Now we either handle this during decode (BlockList, BlockSet) or forbid
it outright (BlockMap) to prevent that crash. BlockMap could _potentially_
also handle this during decode, but that would require a more significant
reorganization of its implementation than I want to take on right now,
and decoding dynamically-typed values inside collections is an edge case
anyway.
  • Loading branch information
apparentlymart committed Aug 22, 2018
1 parent d6049c2 commit b82170e
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 0 deletions.
9 changes: 9 additions & 0 deletions cmd/hcldec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,15 @@ func decodeBlockMapSpec(body hcl.Body, impliedName string) (hcldec.Spec, hcl.Dia
return errSpec, diags
}

if hcldec.ImpliedType(spec).HasDynamicTypes() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid block_map spec",
Detail: "A block_map spec may not contain attributes with type 'any'.",
Subject: body.MissingItemRange().Ptr(),
})
}

return spec, diags
}

Expand Down
86 changes: 86 additions & 0 deletions hcldec/public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,49 @@ b {}
},
{
`
b {
a = true
}
b {
a = 1
}
`,
&BlockListSpec{
TypeName: "b",
Nested: &AttrSpec{
Name: "a",
Type: cty.DynamicPseudoType,
},
},
nil,
cty.DynamicVal,
1, // Unconsistent argument types in b blocks
},
{
`
b {
a = true
}
b {
a = "not a bool"
}
`,
&BlockListSpec{
TypeName: "b",
Nested: &AttrSpec{
Name: "a",
Type: cty.DynamicPseudoType,
},
},
nil,
cty.ListVal([]cty.Value{
cty.StringVal("true"), // type unification generalizes all the values to strings
cty.StringVal("not a bool"),
}),
0,
},
{
`
b {}
b {}
`,
Expand Down Expand Up @@ -465,6 +508,49 @@ b "bar" "baz" {}
},
{
`
b {
a = true
}
b {
a = 1
}
`,
&BlockSetSpec{
TypeName: "b",
Nested: &AttrSpec{
Name: "a",
Type: cty.DynamicPseudoType,
},
},
nil,
cty.DynamicVal,
1, // Unconsistent argument types in b blocks
},
{
`
b {
a = true
}
b {
a = "not a bool"
}
`,
&BlockSetSpec{
TypeName: "b",
Nested: &AttrSpec{
Name: "a",
Type: cty.DynamicPseudoType,
},
},
nil,
cty.SetVal([]cty.Value{
cty.StringVal("true"), // type unification generalizes all the values to strings
cty.StringVal("not a bool"),
}),
0,
},
{
`
b "foo" {}
b "bar" {}
`,
Expand Down
79 changes: 79 additions & 0 deletions hcldec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,44 @@ func (s *BlockListSpec) decode(content *hcl.BodyContent, blockLabels []blockLabe
if len(elems) == 0 {
ret = cty.ListValEmpty(s.Nested.impliedType())
} else {
// Since our target is a list, all of the decoded elements must have the
// same type or cty.ListVal will panic below. Different types can arise
// if there is an attribute spec of type cty.DynamicPseudoType in the
// nested spec; all given values must be convertable to a single type
// in order for the result to be considered valid.
etys := make([]cty.Type, len(elems))
for i, v := range elems {
etys[i] = v.Type()
}
ety, convs := convert.UnifyUnsafe(etys)
if ety == cty.NilType {
// FIXME: This is a pretty terrible error message.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Unconsistent argument types in %s blocks", s.TypeName),
Detail: "Corresponding attributes in all blocks of this type must be the same.",
Subject: &sourceRanges[0],
})
return cty.DynamicVal, diags
}
for i, v := range elems {
if convs[i] != nil {
newV, err := convs[i](v)
if err != nil {
// FIXME: This is a pretty terrible error message.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Unconsistent argument types in %s blocks", s.TypeName),
Detail: fmt.Sprintf("Block with index %d has inconsistent argument types: %s.", i, err),
Subject: &sourceRanges[i],
})
// Bail early here so we won't panic below in cty.ListVal
return cty.DynamicVal, diags
}
elems[i] = newV
}
}

ret = cty.ListVal(elems)
}

Expand Down Expand Up @@ -593,6 +631,44 @@ func (s *BlockSetSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel
if len(elems) == 0 {
ret = cty.SetValEmpty(s.Nested.impliedType())
} else {
// Since our target is a set, all of the decoded elements must have the
// same type or cty.SetVal will panic below. Different types can arise
// if there is an attribute spec of type cty.DynamicPseudoType in the
// nested spec; all given values must be convertable to a single type
// in order for the result to be considered valid.
etys := make([]cty.Type, len(elems))
for i, v := range elems {
etys[i] = v.Type()
}
ety, convs := convert.UnifyUnsafe(etys)
if ety == cty.NilType {
// FIXME: This is a pretty terrible error message.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Unconsistent argument types in %s blocks", s.TypeName),
Detail: "Corresponding attributes in all blocks of this type must be the same.",
Subject: &sourceRanges[0],
})
return cty.DynamicVal, diags
}
for i, v := range elems {
if convs[i] != nil {
newV, err := convs[i](v)
if err != nil {
// FIXME: This is a pretty terrible error message.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: fmt.Sprintf("Unconsistent argument types in %s blocks", s.TypeName),
Detail: fmt.Sprintf("Block with index %d has inconsistent argument types: %s.", i, err),
Subject: &sourceRanges[i],
})
// Bail early here so we won't panic below in cty.ListVal
return cty.DynamicVal, diags
}
elems[i] = newV
}
}

ret = cty.SetVal(elems)
}

Expand Down Expand Up @@ -675,6 +751,9 @@ func (s *BlockMapSpec) decode(content *hcl.BodyContent, blockLabels []blockLabel
if s.Nested == nil {
panic("BlockSetSpec with no Nested Spec")
}
if ImpliedType(s).HasDynamicTypes() {
panic("cty.DynamicPseudoType attributes may not be used inside a BlockMapSpec")
}

elems := map[string]interface{}{}
for _, childBlock := range content.Blocks {
Expand Down

0 comments on commit b82170e

Please sign in to comment.