Marshal() generate invalid JSON

[denis256]

Hello, during my investigation of a bug in a different project, noticed that exist cases when Marshal() can generate invalid JSON

...
    valMap := map[string]cty.Value{}
    valMap["test"] = cty.NullVal(cty.DynamicPseudoType)
    valMap["test2"] = cty.UnknownVal(cty.DynamicPseudoType)

    valMapAsCty, err := gocty.ToCtyValue(valMap, generateTypeFromValuesMap(valMap))
    if err != nil {
        log.Fatal(err)
    }

    jsonBytes, err := ctyjson.Marshal(valMapAsCty, cty.DynamicPseudoType)
    if err != nil {
        log.Fatal(err)
    }
    log.Println(string(jsonBytes))
...

Output:

2023/09/11 11:51:03 {"value":{"test":null,"test2":,"type":["object",{"test":"dynamic","test2":"dynamic"}]}

I would expect to fail with an error or generate a valid JSON

End-to-end example: https://github.com/denis256/go-tests/blob/master/zclconf-go-cty/main.go

[apparentlymart]

Thanks for reporting this, @denis256!

With the code you showed I would've expected the call to ctyjson.Marshal to fail, because there's no way to serialize an unknown value in JSON. Instead, it seems to have succeeded but just skipped serializing the invalid value.

I can reproduce this behavior in the Go playground: https://go.dev/play/p/Z_ztLVhfE4Q

The correct behavior here would be for ctyjson.Marshal to return an error, and therefore your caller would fail with log.Fatal.

I will try to debug and fix this at some point, but note that it's typically incorrect to pass unknown values into the JSON encoder and so you could potentially avoid this problem today by screening for unknown values first. For example, you could call valMapAsCty.IsWhollyKnown() first and return an error early if it returns false, since once corrected that call would return an error in that situation anyway.

[apparentlymart]

Thanks again for reporting this!

I've just fixed this in 4a34c33ad713db9437a9173c5d5529ca0ed606e3 and intend to make a new patch release containing that fix soon.