From 7ef3c0881f6c476a2e54b9ac726b20198e2b6707 Mon Sep 17 00:00:00 2001
From: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Date: Mon, 21 Jun 2021 11:52:58 -0400
Subject: [PATCH] function/stdlib: Fix setproduct bug with unknowns

If an argument to setproduct is a set with unknown values, its length is
unknown, as the unknown values may be duplicates of others in the set.
This should result in returning an unknown value propagating all marks.

Previously this crashed due to a misuse of `.Mark(marks)` instead of
`.WithMarks(marks)`. This commit also fixes an issue with the previous
logic, which would shortcut return and omit marks from any arguments
after the set with unknown length.

Also includes a couple of tests for setproduct that were in my local
working copy for some reason.
---
 cty/function/stdlib/collection.go      | 10 ++++++++-
 cty/function/stdlib/collection_test.go | 30 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/cty/function/stdlib/collection.go b/cty/function/stdlib/collection.go
index 1f599c85..f50f196e 100644
--- a/cty/function/stdlib/collection.go
+++ b/cty/function/stdlib/collection.go
@@ -948,12 +948,16 @@ var SetProductFunc = function.New(&function.Spec{
 		var retMarks cty.ValueMarks
 
 		total := 1
+		var hasUnknownLength bool
 		for _, arg := range args {
 			arg, marks := arg.Unmark()
 			retMarks = cty.NewValueMarks(retMarks, marks)
 
+			// Continue processing after we find an argument with unknown
+			// length to ensure that we cover all the marks
 			if !arg.Length().IsKnown() {
-				return cty.UnknownVal(retType).Mark(marks), nil
+				hasUnknownLength = true
+				continue
 			}
 
 			// Because of our type checking function, we are guaranteed that
@@ -962,6 +966,10 @@ var SetProductFunc = function.New(&function.Spec{
 			total *= arg.LengthInt()
 		}
 
+		if hasUnknownLength {
+			return cty.UnknownVal(retType).WithMarks(retMarks), nil
+		}
+
 		if total == 0 {
 			// If any of the arguments was an empty collection then our result
 			// is also an empty collection, which we'll short-circuit here.
diff --git a/cty/function/stdlib/collection_test.go b/cty/function/stdlib/collection_test.go
index 4d9a2163..cfafb4af 100644
--- a/cty/function/stdlib/collection_test.go
+++ b/cty/function/stdlib/collection_test.go
@@ -2324,6 +2324,36 @@ func TestSetproduct(t *testing.T) {
 			}).Mark("b"),
 			``,
 		},
+		{
+			// Empty lists with marks should propagate the marks
+			[]cty.Value{
+				cty.ListValEmpty(cty.String).Mark("a"),
+				cty.ListValEmpty(cty.Bool).Mark("b"),
+			},
+			cty.ListValEmpty(cty.Tuple([]cty.Type{cty.String, cty.Bool})).WithMarks(cty.NewValueMarks("a", "b")),
+			``,
+		},
+		{
+			// Empty sets with marks should propagate the marks
+			[]cty.Value{
+				cty.SetValEmpty(cty.String).Mark("a"),
+				cty.SetValEmpty(cty.Bool).Mark("b"),
+			},
+			cty.SetValEmpty(cty.Tuple([]cty.Type{cty.String, cty.Bool})).WithMarks(cty.NewValueMarks("a", "b")),
+			``,
+		},
+		{
+			// Arguments which are sets with partially unknown values results
+			// in unknown length (since the unknown values may already be
+			// present in the set). This gives an unknown result preserving all
+			// marks
+			[]cty.Value{
+				cty.SetVal([]cty.Value{cty.StringVal("x"), cty.UnknownVal(cty.String)}).Mark("a"),
+				cty.SetVal([]cty.Value{cty.True, cty.False}).Mark("b"),
+			},
+			cty.UnknownVal(cty.Set(cty.Tuple([]cty.Type{cty.String, cty.Bool}))).WithMarks(cty.NewValueMarks("a", "b")),
+			``,
+		},
 	}
 
 	for _, test := range tests {