Skip to content

Commit

Permalink
cty: Fix path array reuse with UnmarkDeepWithPaths
Browse files Browse the repository at this point in the history
When unmarking a complex value and retaining the marked paths, a
sufficiently deep structure would result in incorrect duplicate path
output. For example, this structure (in HCL syntax):

{
  environment = [
    {
      variables = {
        "x" = 1
        "y" = 2
      }
    }
  ]
}

If the 1 and 2 values are marked, the resulting path value marks from
UnmarkDeepWithPaths would have two entries, as expected. However, both
would have the same Path attribute, like so:

[
  { environment[0].variables["x"], "mark" },
  { environment[0].variables["x"], "mark" },
]

This is caused by calling `append` in the walk transform function with
the same path object repeatedly, which eventually does not result in
array reallocation, and therefore the path object is modified in-place.
  • Loading branch information
alisdair authored and apparentlymart committed Oct 12, 2020
1 parent 19f2b6b commit e9c1706
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
4 changes: 3 additions & 1 deletion cty/marks.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,9 @@ type unmarkTransformer struct {
func (t *unmarkTransformer) Enter(p Path, v Value) (Value, error) {
unmarkedVal, marks := v.Unmark()
if len(marks) > 0 {
t.pvm = append(t.pvm, PathValueMarks{p, marks})
path := make(Path, len(p), len(p)+1)
copy(path, p)
t.pvm = append(t.pvm, PathValueMarks{path, marks})
}
return unmarkedVal, nil
}
Expand Down
26 changes: 26 additions & 0 deletions cty/marks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,32 @@ func TestPathValueMarks(t *testing.T) {
{GetAttrPath("z"), NewValueMarks("f")},
},
},
"path array reuse regression test": {
ObjectVal(map[string]Value{
"environment": ListVal([]Value{
ObjectVal(map[string]Value{
"variables": MapVal(map[string]Value{
"bar": StringVal("secret").Mark("sensitive"),
"foo": StringVal("secret").Mark("sensitive"),
}),
}),
}),
}),
ObjectVal(map[string]Value{
"environment": ListVal([]Value{
ObjectVal(map[string]Value{
"variables": MapVal(map[string]Value{
"bar": StringVal("secret"),
"foo": StringVal("secret"),
}),
}),
}),
}),
[]PathValueMarks{
{GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("bar"), NewValueMarks("sensitive")},
{GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("foo"), NewValueMarks("sensitive")},
},
},
}

for name, tc := range testCases {
Expand Down
3 changes: 3 additions & 0 deletions cty/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error {
// Use Enter when you want to transform a complex value before traversal
// (preorder), and Exit when you want to transform a value after traversal
// (postorder).
//
// The path passed to the given function may not be used after that function
// returns, since its backing array is re-used for other calls.
type Transformer interface {
Enter(Path, Value) (Value, error)
Exit(Path, Value) (Value, error)
Expand Down

0 comments on commit e9c1706

Please # to comment.