Skip to content

Commit

Permalink
KeysFunc: apply top-level marks to result
Browse files Browse the repository at this point in the history
  • Loading branch information
mildwonkey committed Apr 20, 2021
1 parent c65249e commit 5a45c63
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 80 deletions.
52 changes: 14 additions & 38 deletions cty/function/stdlib/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,7 @@ var KeysFunc = function.New(&function.Spec{
}
},
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
// Since map keys cannot be marked, we don't need to preserve the deeply-nested marks.
m, marks := args[0].Unmark()
m, _ = m.UnmarkDeep()
m, marks := args[0].UnmarkDeep()
var keys []cty.Value

switch {
Expand All @@ -579,26 +577,26 @@ var KeysFunc = function.New(&function.Spec{
}
sort.Strings(names) // same ordering guaranteed by cty's ElementIterator
if len(names) == 0 {
return cty.EmptyTupleVal.WithMarks(marks), nil
return cty.EmptyTupleVal, nil
}
keys = make([]cty.Value, len(names))
for i, name := range names {
keys[i] = cty.StringVal(name)
}
return cty.TupleVal(keys).WithMarks(marks), nil
return cty.TupleVal(keys), nil
default:
if !m.IsKnown() {
return cty.UnknownVal(retType).WithMarks(marks), nil
return cty.UnknownVal(retType), nil
}

// cty guarantees that ElementIterator will iterate in lexicographical
// order by key.
for it := m.ElementIterator(); it.Next(); {
for it := args[0].ElementIterator(); it.Next(); {
k, _ := it.Element()
keys = append(keys, k)
}
if len(keys) == 0 {
return cty.ListValEmpty(cty.String).WithMarks(marks), nil
return cty.ListValEmpty(cty.String), nil
}
return cty.ListVal(keys).WithMarks(marks), nil
}
Expand Down Expand Up @@ -1148,14 +1146,12 @@ var ValuesFunc = function.New(&function.Spec{
var ZipmapFunc = function.New(&function.Spec{
Params: []function.Parameter{
{
Name: "keys",
Type: cty.List(cty.String),
AllowMarked: true,
Name: "keys",
Type: cty.List(cty.String),
},
{
Name: "values",
Type: cty.DynamicPseudoType,
AllowMarked: true,
Name: "values",
Type: cty.DynamicPseudoType,
},
},
Type: func(args []cty.Value) (ret cty.Type, err error) {
Expand All @@ -1173,21 +1169,13 @@ var ZipmapFunc = function.New(&function.Spec{
return cty.DynamicPseudoType, nil
}

// NOTE: Marking of the keys list can't be represented in the
// result type, so the tuple type here will disclose the keys.
// This is unfortunate but is a common compromise with dynamic
// return types; the result from Impl will still reflect the marks
// from the keys list, so a mark-using caller should look out for
// that if it's important for their use-case.
keys, _ := keys.Unmark()
keysRaw := keys.AsValueSlice()
valueTypesRaw := valuesTy.TupleElementTypes()
if len(keysRaw) != len(valueTypesRaw) {
return cty.NilType, fmt.Errorf("number of keys (%d) does not match number of values (%d)", len(keysRaw), len(valueTypesRaw))
}
atys := make(map[string]cty.Type, len(valueTypesRaw))
for i, keyVal := range keysRaw {
keyVal, _ = keyVal.Unmark()
if keyVal.IsNull() {
return cty.NilType, fmt.Errorf("keys list has null value at index %d", i)
}
Expand All @@ -1203,17 +1191,11 @@ var ZipmapFunc = function.New(&function.Spec{
Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
keys := args[0]
values := args[1]
keys, keysMarks := keys.Unmark()
values, valuesMarks := values.Unmark()

// All of our return paths must pass through the merged marks from
// both the keys and the values, if any, using .WithMarks(retMarks)
retMarks := cty.NewValueMarks(keysMarks, valuesMarks)

if !keys.IsWhollyKnown() {
// Unknown map keys and object attributes are not supported, so
// our entire result must be unknown in this case.
return cty.UnknownVal(retType).WithMarks(retMarks), nil
return cty.UnknownVal(retType), nil
}

// both keys and values are guaranteed to be shallowly-known here,
Expand All @@ -1227,25 +1209,19 @@ var ZipmapFunc = function.New(&function.Spec{
i := 0
for it := keys.ElementIterator(); it.Next(); {
_, v := it.Element()
v, vMarks := v.Unmark()
val := values.Index(cty.NumberIntVal(int64(i)))
output[v.AsString()] = val

// We also need to accumulate the individual key marks on the
// returned map, because keys can't carry marks on their own.
retMarks = cty.NewValueMarks(retMarks, vMarks)

i++
}

switch {
case retType.IsMapType():
if len(output) == 0 {
return cty.MapValEmpty(retType.ElementType()).WithMarks(retMarks), nil
return cty.MapValEmpty(retType.ElementType()), nil
}
return cty.MapVal(output).WithMarks(retMarks), nil
return cty.MapVal(output), nil
case retType.IsObjectType():
return cty.ObjectVal(output).WithMarks(retMarks), nil
return cty.ObjectVal(output), nil
default:
// Should never happen because the type-check function should've
// caught any other case.
Expand Down
42 changes: 0 additions & 42 deletions cty/function/stdlib/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1328,48 +1328,6 @@ func TestZipMap(t *testing.T) {
}
}

func TestKeys(t *testing.T) {
tests := []struct {
Collection cty.Value
Want cty.Value
Err bool
}{
{
cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world")}),
cty.ListVal([]cty.Value{cty.StringVal("hello")}),
false,
},
{ // The map itself is not marked, just an inner element.
cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world").Mark("a")}),
cty.ListVal([]cty.Value{cty.StringVal("hello")}),
false,
},
{ // The entire map is marked, and we refuse to risk returning sensitive keys.
cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world")}).Mark("a"),
cty.NilVal,
false,
},
}

for _, test := range tests {
t.Run(fmt.Sprintf("Keys(%#v)", test.Collection), func(t *testing.T) {
got, err := Keys(test.Collection)
if test.Err {
if err == nil {
t.Fatal("succeeded; want error")
}
return
} else if err != nil {
t.Fatalf("unexpected error: %s", err)
}

if !got.RawEquals(test.Want) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}

func TestKeys(t *testing.T) {
tests := []struct {
Collection cty.Value
Expand Down

0 comments on commit 5a45c63

Please sign in to comment.