Skip to content

Commit

Permalink
hclsyntax: Detect and reject invalid nested splat result
Browse files Browse the repository at this point in the history
Unfortunately two of the weird splat rules we included to mimic Terraform
v0.11's bizarre splat behavior interact in a broken way when combined using
nested splat expressions:

1. Applying splat to a list or set value always returns a list value,
   rather than a tuple value, because that allows returning a more precise
   unknown value type when the source value is unknown.
2. Applying a splat to a scalar value returns a tuple of either zero or
   one elements, depending on whether the scalar is null. This returns a
   tuple rather than a list again because that allows returning more
   precise information in some unknown value cases.

When both of these rules are combined using a nested splat -- rule 2 inside
rule 1 -- the second rule causes the nested results to have varying types
based on the scalar _value_ rather than only on the list element type,
which therefore violates the assumption made by rule 1 that because all
elements of a list or set have the same type therefore all splat results
on those elements must have the same type.

This previously caused a crash due to asking cty to construct an invalid
list. We'll now handle this as a normal error diagnostic instead of a
crash.

Ideally we'd make this situation work and generate a valid result of a
different type, such as a list of lists rather than an invalid list of
different tuple types. However, it's not clear that we can make that change
retroactively without breaking existing code, and combining splat
expressions in this way seems to be an edge case anyway -- it took many
years before someone actually tried this combination and found the bug.
Perhaps a future change will find a way to resolve this without an error,
but for now we'll compromise on treating this as invalid since we know it
wasn't working before anyway but at least this avoids crashing the host
program completely.
  • Loading branch information
apparentlymart committed Jan 3, 2025
1 parent 923b06b commit 8c8dab2
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
16 changes: 16 additions & 0 deletions hclsyntax/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -1938,6 +1938,22 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
diags = append(diags, tyDiags...)
return cty.ListValEmpty(ty.ElementType()).WithMarks(marks), diags
}
// Unfortunately it's possible for a nested splat on scalar values to
// generate non-homogenously-typed vals, and we discovered this bad
// interaction after the two conflicting behaviors were both
// well-established so it isn't clear how to change them without
// breaking existing code. Therefore we just make that an error for
// now, to avoid crashing trying to constuct an impossible list.
if !cty.CanListVal(vals) {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid nested splat expressions",
Detail: "The second level of splat expression produced elements of different types, so it isn't possible to construct a valid list to represent the top-level result.\n\nConsider using a for expression instead, to produce a tuple-typed result which can therefore have non-homogenous element types.",
Subject: e.Each.Range().Ptr(),
Context: e.Range().Ptr(), // encourage a diagnostic renderer to also include the "source" part of the expression in its code snippet
})
return cty.DynamicVal, diags
}
return cty.ListVal(vals).WithMarks(marks), diags
default:
return cty.TupleVal(vals).WithMarks(marks), diags
Expand Down
48 changes: 48 additions & 0 deletions hclsyntax/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,54 @@ upper(
cty.DynamicVal,
1, // splat cannot be applied to null sequence
},
{
`listofobj[*].scalar[*]`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"listofobj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"scalar": cty.StringVal("foo"),
}),
cty.ObjectVal(map[string]cty.Value{
"scalar": cty.StringVal("bar"),
}),
}),
},
},
cty.ListVal([]cty.Value{
// The second-level splat promotes the scalars to single-element tuples.
cty.TupleVal([]cty.Value{cty.StringVal("foo")}),
cty.TupleVal([]cty.Value{cty.StringVal("bar")}),
}),
0,
},
{
// This is a particularly tricky case where two splat rules interact in
// a sub-optimal way:
// 1. The top-level splat is applied to a list and so it wants to return a list.
// 2. The nested splat is applied to a scalar, and so it wants to return different tuple types depending on the nullness.
// Rule 2 breaks rule 1, because we can't make a list with elements of different types.
// For now we're treating this as an error because we didn't learn of this bad
// interaction until long after both of these rules were in separate wide use,
// and so it isn't clear how to make this work without potentially breaking other
// behavior. Perhaps this can become valid in future if we find a viable way to
// do it.
`listofobj[*].scalar[*]`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"listofobj": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"scalar": cty.NullVal(cty.String),
}),
cty.ObjectVal(map[string]cty.Value{
"scalar": cty.StringVal("bar"),
}),
}),
},
},
cty.DynamicVal,
1, // nested splat produces non-homogenously-typed results in this case, so cannot produce a valid list
},
{
`["hello", "goodbye"].*`,
nil,
Expand Down

0 comments on commit 8c8dab2

Please sign in to comment.