From 37006c584125cc945a48451e4df727bb770baf1c Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 4 Mar 2020 10:46:14 -0500 Subject: [PATCH] lang: Fix non-string key panics in map function The map function assumed that the key arguments were strings, and would panic if they were not. After this commit, calling `map(1, 2)` will result in a map `{"1" = 1}`, and calling `map(null, 1)` will result in a syntax error. Fixes #23346, fixes #23043 --- lang/funcs/collection.go | 8 +++++--- lang/funcs/collection_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index a6eb16fba..dbcc0c715 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -346,12 +346,14 @@ var MapFunc = function.New(&function.Spec{ for i := 0; i < len(args); i += 2 { - key := args[i].AsString() - - err := gocty.FromCtyValue(args[i], &key) + keyVal, err := convert.Convert(args[i], cty.String) if err != nil { return cty.NilVal, err } + if keyVal.IsNull() { + return cty.NilVal, fmt.Errorf("argument %d is a null key", i+1) + } + key := keyVal.AsString() val := args[i+1] diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 76dd1dbf0..fa5368a46 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" ) func TestLength(t *testing.T) { @@ -730,6 +731,19 @@ func TestMap(t *testing.T) { }), false, }, + { // convert number keys to strings + []cty.Value{ + cty.NumberIntVal(1), + cty.StringVal("hello"), + cty.NumberIntVal(2), + cty.StringVal("goodbye"), + }, + cty.MapVal(map[string]cty.Value{ + "1": cty.StringVal("hello"), + "2": cty.StringVal("goodbye"), + }), + false, + }, { // map of lists is okay []cty.Value{ cty.StringVal("hello"), @@ -785,6 +799,14 @@ func TestMap(t *testing.T) { cty.NilVal, true, }, + { // null key returns an error + []cty.Value{ + cty.NullVal(cty.DynamicPseudoType), + cty.NumberIntVal(5), + }, + cty.NilVal, + true, + }, } for _, test := range tests { @@ -794,6 +816,9 @@ func TestMap(t *testing.T) { if err == nil { t.Fatal("succeeded; want error") } + if _, ok := err.(function.PanicError); ok { + t.Fatalf("unexpected panic: %s", err) + } return } else if err != nil { t.Fatalf("unexpected error: %s", err)