Skip to content

Fix fn::toString quoting #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
seanyeh wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix fn::toString quoting #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions eval/testdata/eval/nested-unknowns-secrets/expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2982,7 +2982,7 @@
}
},
"stringOutputSecret": {
"value": "\"account\"=\"\\\"id\\\"=\\\"accountIdSecret\\\",\\\"name\\\"=\\\"account-name\\\"\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"value": "\"account\"=\"id\"=\"accountIdSecret\",\"name\"=\"account-name\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"secret": true,
"trace": {
"def": {
Expand Down Expand Up @@ -6532,7 +6532,7 @@
}
},
"stringOutputSecret": {
"value": "\"account\"=\"\\\"id\\\"=\\\"accountIdSecret\\\",\\\"name\\\"=\\\"account-name\\\"\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"value": "\"account\"=\"id\"=\"accountIdSecret\",\"name\"=\"account-name\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"secret": true,
"trace": {
"def": {
Expand All @@ -6551,7 +6551,7 @@
}
},
"stringOutputUnknown": {
"value": "\"oneStep\"=\"\\\"twoStep\\\"=\\\"\\\\\\\"threeStep\\\\\\\"=\\\\\\\"\\\\\\\\\\\\\\\"fourStep\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"foo\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"bar\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\"\\\\\\\"\\\"\"",
"value": "\"oneStep\"=\"twoStep\"=\"threeStep\"=\"fourStep\"=\"foo\"=\"bar\"",
"trace": {
"def": {
"environment": "nested-unknowns-secrets",
Expand Down Expand Up @@ -7001,7 +7001,7 @@
},
"stringOutputCombo": "[secret]",
"stringOutputSecret": "[secret]",
"stringOutputUnknown": "\"oneStep\"=\"\\\"twoStep\\\"=\\\"\\\\\\\"threeStep\\\\\\\"=\\\\\\\"\\\\\\\\\\\\\\\"fourStep\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"foo\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"bar\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\"\\\\\\\"\\\"\"",
"stringOutputUnknown": "\"oneStep\"=\"twoStep\"=\"threeStep\"=\"fourStep\"=\"foo\"=\"bar\"",
"top-level-secret": "[secret]"
},
"evalJSONRevealed": {
Expand Down Expand Up @@ -7043,8 +7043,8 @@
"refTopLevelSecret": "top-secret"
},
"stringOutputCombo": "\"cloudflareApi\"=\"apiKeySecret\",\"provider\"=\"bar\"",
"stringOutputSecret": "\"account\"=\"\\\"id\\\"=\\\"accountIdSecret\\\",\\\"name\\\"=\\\"account-name\\\"\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"stringOutputUnknown": "\"oneStep\"=\"\\\"twoStep\\\"=\\\"\\\\\\\"threeStep\\\\\\\"=\\\\\\\"\\\\\\\\\\\\\\\"fourStep\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"foo\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"=\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"bar\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\"\\\\\\\"\\\"\"",
"stringOutputSecret": "\"account\"=\"id\"=\"accountIdSecret\",\"name\"=\"account-name\",\"apiKey\"=\"apiKeySecret\",\"apiToken\"=\"apiTokenSecret\",\"username\"=\"bot@pulumi.com\",\"zoneId\"=\"zone-id-123\"",
"stringOutputUnknown": "\"oneStep\"=\"twoStep\"=\"threeStep\"=\"fourStep\"=\"foo\"=\"bar\"",
"top-level-secret": "top-secret"
}
}
12 changes: 6 additions & 6 deletions eval/testdata/eval/omnibus/expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3342,7 +3342,7 @@
}
},
"interp": {
"value": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\"",
"value": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\"",
"trace": {
"def": {
"environment": "omnibus",
Expand Down Expand Up @@ -3815,7 +3815,7 @@
}
},
"toString": {
"value": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\"",
"value": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\"",
"trace": {
"def": {
"environment": "omnibus",
Expand Down Expand Up @@ -4220,7 +4220,7 @@
],
"foo": "bar"
},
"interp": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\"",
"interp": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\"",
"join": "hello,world",
"open": {
"a": null,
Expand Down Expand Up @@ -4249,7 +4249,7 @@
],
"toBase64": "aGVsbG8sd29ybGQ=",
"toJSON": "{\"a\":null,\"b\":true,\"baz\":\"qux\",\"c\":42,\"d\":[\"hello\"],\"foo\":\"bar\"}",
"toString": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\""
"toString": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\""
},
"evalJSONRevealed": {
"access": "qux",
Expand All @@ -4264,7 +4264,7 @@
],
"foo": "bar"
},
"interp": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\"",
"interp": "hello, \"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\"",
"join": "hello,world",
"open": {
"a": null,
Expand Down Expand Up @@ -4293,6 +4293,6 @@
],
"toBase64": "aGVsbG8sd29ybGQ=",
"toJSON": "{\"a\":null,\"b\":true,\"baz\":\"qux\",\"c\":42,\"d\":[\"hello\"],\"foo\":\"bar\"}",
"toString": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"\\\"hello\\\"\""
"toString": "\"a\"=\"\",\"b\"=\"true\",\"baz\"=\"qux\",\"c\"=\"42\",\"d\"=\"hello\""
}
}
28 changes: 26 additions & 2 deletions eval/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ func (v *value) merge(base *value) {
v.schema = mergedSchema(v.base.schema, v.schema)
}

// isComplex returns true if the value's representation is a complex type (array or map)
// that already includes its own quoting when converted to a string.
func (v *value) isComplex() bool {
if v.unknown {
return false
}
switch v.repr.(type) {
case []*value, map[string]*value:
return true
default:
return false
}
}

// toString returns the string representation of this value, whether the string is known, and whether the string is
// secret.
func (v *value) toString() (str string, unknown bool, secret bool) {
Expand All @@ -302,7 +316,12 @@ func (v *value) toString() (str string, unknown bool, secret bool) {
vals := make([]string, len(repr))
for i, v := range repr {
vs, vunknown, vsecret := v.toString()
vals[i], unknown, secret = strconv.Quote(vs), unknown || vunknown, secret || vsecret
unknown, secret = unknown || vunknown, secret || vsecret
if v.isComplex() {
vals[i] = vs
} else {
vals[i] = strconv.Quote(vs)
}
}
s = strings.Join(vals, ",")
case map[string]*value:
Expand All @@ -312,7 +331,12 @@ func (v *value) toString() (str string, unknown bool, secret bool) {
pairs := make([]string, len(repr))
for i, k := range keys {
vs, vunknown, vsecret := repr[k].toString()
pairs[i], unknown, secret = fmt.Sprintf("%q=%q", k, vs), unknown || vunknown, secret || vsecret
unknown, secret = unknown || vunknown, secret || vsecret
if repr[k].isComplex() {
pairs[i] = fmt.Sprintf("%q=%s", k, vs)
} else {
pairs[i] = fmt.Sprintf("%q=%q", k, vs)
}
}
s = strings.Join(pairs, ",")
}
Expand Down
39 changes: 39 additions & 0 deletions eval/value_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,45 @@ func TestValueToString(t *testing.T) {
secret: true,
expected: `"baz"="42","foo"="bar"`,
},
// Unknown value with complex repr should still be quoted
{
value: &value{repr: []*value{
{unknown: true, repr: map[string]*value{"foo": {repr: "bar"}}},
{repr: "hello"},
}},
unknown: true,
expected: `"[unknown]","hello"`,
},
// Nested structures: map with array value
{
value: &value{repr: map[string]*value{
"hello": {repr: "world"},
"from": {repr: []*value{{repr: "pulumi"}, {repr: "esc"}}},
}},
expected: `"from"="pulumi","esc","hello"="world"`,
},
// Nested structures: array with map value
{
value: &value{repr: []*value{
{repr: map[string]*value{"foo": {repr: "bar"}}},
{repr: "hello"},
}},
expected: `"foo"="bar","hello"`,
},
// Nested structures: map with nested map value
{
value: &value{repr: map[string]*value{
"outer": {repr: map[string]*value{"inner": {repr: "val"}}},
}},
expected: `"outer"="inner"="val"`,
},
// Nested structures: array with nested array value
{
value: &value{repr: []*value{
{repr: []*value{{repr: "a"}, {repr: "b"}}},
}},
expected: `"a","b"`,
},
}
for _, c := range cases {
t.Run(c.expected, func(t *testing.T) {
Expand Down
Loading