Skip to content

Commit

Permalink
YAMLDecodeFunc: handle null values correctly
Browse files Browse the repository at this point in the history
Due to a logic error in the YAMLDecodeFunc implementation, any situation
where the result was literally null the function would return an unknown
value of an unknown type instead.

Now it will correctly return a null of unknown type instead.

This also includes some extra test coverage for other functions to confirm
that they were already handling these situations correctly, and to guard
against possible future regressions for these edge-cases.
  • Loading branch information
apparentlymart committed Nov 2, 2022
1 parent 14b7afb commit c591969
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 12 deletions.
4 changes: 2 additions & 2 deletions converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ func (c *Converter) Marshal(v cty.Value) ([]byte, error) {
// and attempts to convert it into a value conforming to the given type
// constraint.
//
// An error is returned if the given source contains any YAML document
// delimiters.
// An error is returned if the given source contains more than one YAML
// document.
func (c *Converter) Unmarshal(src []byte, ty cty.Type) (cty.Value, error) {
return c.unmarshal(src, ty)
}
3 changes: 0 additions & 3 deletions cty_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ var YAMLDecodeFunc = function.New(&function.Spec{
return Standard.ImpliedType([]byte(args[0].AsString()))
},
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
if retType == cty.DynamicPseudoType {
return cty.DynamicVal, nil
}
return Standard.Unmarshal([]byte(args[0].AsString()), retType)
},
})
Expand Down
36 changes: 29 additions & 7 deletions cty_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,36 @@ import (

func TestYAMLDecodeFunc(t *testing.T) {
// FIXME: This is not a very extensive test.
got, err := YAMLDecodeFunc.Call([]cty.Value{
cty.StringVal("true"),
})
if err != nil {
t.Fatalf("unexpected error: %s", err)
tests := map[string]struct {
input string
want cty.Value
}{
"only document separator": {
`---`,
cty.NullVal(cty.DynamicPseudoType),
},
"null": {
`~`,
cty.NullVal(cty.DynamicPseudoType),
},
"boolean true": {
`true`,
cty.True,
},
}
if want := cty.True; !want.RawEquals(got) {
t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, want)

for name, test := range tests {
t.Run(name, func(t *testing.T) {
got, err := YAMLDecodeFunc.Call([]cty.Value{
cty.StringVal(test.input),
})
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
if want := test.want; !want.RawEquals(got) {
t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, want)
}
})
}
}

Expand Down
28 changes: 28 additions & 0 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,34 @@ func TestUnmarshal(t *testing.T) {
want cty.Value
wantErr string
}{
"only document separator": {
Standard,
`---`,
cty.String,
cty.NullVal(cty.String),
``,
},
"null": {
Standard,
`~`,
cty.String,
cty.NullVal(cty.String),
``,
},
"one document separator followed by value": {
Standard,
"---\ntrue",
cty.Bool,
cty.True,
``,
},
"multiple documents": {
Standard,
"---\ntrue\n---\nfalse",
cty.Bool,
cty.DynamicVal,
`on line 2, column 1: unexpected extra content after value`,
},
"single string doublequote": {
Standard,
`"hello"`,
Expand Down
12 changes: 12 additions & 0 deletions implied_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ func TestImpliedType(t *testing.T) {
want cty.Type
wantErr string
}{
"only document separator": {
Standard,
`---`,
cty.DynamicPseudoType, // would decode as a null value of unknown type
``,
},
"null": {
Standard,
`~`,
cty.DynamicPseudoType, // would decode as a null value of unknown type
``,
},
"single string doublequote": {
Standard,
`"hello"`,
Expand Down

0 comments on commit c591969

Please sign in to comment.