Repository: vmware/govmomi
Pull request #3530 (2024-08-23 18:46:37)

api: Fix panic when encoding nil iface ptrs to JSON


## Description

This patch fixes a panic that occurred when encoding nil interface pointers to JSON using the vimtype JSON encoder.

Closes: `NA`

## Type of change

Please mark options that are relevant:

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Build related change

## How Has This Been Tested?

```
go test -v -count 1 -run '^TestSerialization$/^ConfigSpec$'  ./vim25/types
```

## Checklist:

- [x] My code follows the `CONTRIBUTION` [guidelines] of this project
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged

[guidelines]: https://github.com/vmware/govmomi/blob/main/CONTRIBUTING.md



Patch:
diff --git a/vim25/json/discriminator.go b/vim25/json/discriminator.go
index ce315dd5..eed9a664 100644
--- a/vim25/json/discriminator.go
+++ b/vim25/json/discriminator.go
@@ -354,7 +354,11 @@ func discriminatorInterfaceEncode(e *encodeState, v reflect.Value, opts encOpts)
 		e.discriminatorEncodeTypeName = true
 		newStructEncoder(v.Type())(e, v, opts)
 	case reflect.Ptr:
-		discriminatorInterfaceEncode(e, v, opts)
+		if v.IsZero() {
+			newPtrEncoder(v.Type())(e, v, opts)
+		} else {
+			discriminatorInterfaceEncode(e, v, opts)
+		}
 	default:
 		discriminatorValue := opts.discriminatorValueFn(v.Type())
 		if discriminatorValue == "" {


Test patch:
diff --git a/vim25/types/json_test.go b/vim25/types/json_test.go
index aeeffb4e..a33e8cff 100644
--- a/vim25/types/json_test.go
+++ b/vim25/types/json_test.go
@@ -107,6 +107,109 @@ func TestSerialization(t *testing.T) {
 			assert.JSONEq(t, expected, actual)
 		})
 	}
+
+	t.Run("ConfigSpec", func(t *testing.T) {
+		t.Run("Encode", func(t *testing.T) {
+
+			var testCases = []struct {
+				name        string
+				data        any
+				expected    string
+				expectPanic bool
+			}{
+				{
+					name:     "nil ConfigSpec",
+					data:     (*VirtualMachineConfigSpec)(nil),
+					expected: "null",
+				},
+				{
+					name: "ConfigSpec with nil OptionValue value",
+					data: &VirtualMachineConfigSpec{
+						ExtraConfig: []BaseOptionValue{
+							&OptionValue{
+								Key:   "key1",
+								Value: nil,
+							},
+						},
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec","extraConfig":[{"_typeName":"OptionValue","key":"key1","value":null}]}`,
+				},
+				{
+					name: "ConfigSpec with nil OptionValue interface value",
+					data: &VirtualMachineConfigSpec{
+						ExtraConfig: []BaseOptionValue{
+							&OptionValue{
+								Key:   "key1",
+								Value: (any)(nil),
+							},
+						},
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec","extraConfig":[{"_typeName":"OptionValue","key":"key1","value":null}]}`,
+				},
+				{
+					name: "ConfigSpec with nil element in OptionValues",
+					data: &VirtualMachineConfigSpec{
+						ExtraConfig: []BaseOptionValue{
+							(*OptionValue)(nil),
+						},
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec","extraConfig":[null]}`,
+				},
+				{
+					name: "ConfigSpec with nil ToolsConfigInfo",
+					data: &VirtualMachineConfigSpec{
+						Tools: (*ToolsConfigInfo)(nil),
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec"}`,
+				},
+				{
+					name: "ConfigSpec with nil vAppConfig",
+					data: &VirtualMachineConfigSpec{
+						VAppConfig: nil,
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec"}`,
+				},
+				{
+					name: "ConfigSpec with nil pointer vAppConfig ",
+					data: &VirtualMachineConfigSpec{
+						VAppConfig: (*VmConfigSpec)(nil),
+					},
+					expected: `{"_typeName":"VirtualMachineConfigSpec","vAppConfig":null}`,
+				},
+			}
+
+			for i := range testCases {
+				tc := testCases[i]
+				t.Run(tc.name, func(t *testing.T) {
+					var w bytes.Buffer
+					enc := NewJSONEncoder(&w)
+
+					var panicErr any
+
+					func() {
+						defer func() {
+							panicErr = recover()
+						}()
+						if err := enc.Encode(tc.data); err != nil {
+							t.Fatal(err)
+						}
+					}()
+
+					if tc.expectPanic && panicErr == nil {
+						t.Fatalf("did not panic, w=%v", w.String())
+					} else if tc.expectPanic && panicErr != nil {
+						t.Logf("expected panic occurred: %v\n", panicErr)
+					} else if !tc.expectPanic && panicErr != nil {
+						t.Fatalf("unexpected panic occurred: %v\n", panicErr)
+					} else if a, e := w.String(), tc.expected+"\n"; a != e {
+						t.Fatalf("act=%s != exp=%s", a, e)
+					} else {
+						t.Log(a)
+					}
+				})
+			}
+		})
+	})
 }
 
 func TestOptionValueSerialization(t *testing.T) {